Skip to content

bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time - #20493

Merged
miss-islington merged 1 commit into
python:masterfrom
encukou:bpo-40777
May 28, 2020
Merged

bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time#20493
miss-islington merged 1 commit into
python:masterfrom
encukou:bpo-40777

Conversation

@encukou

@encukou encukou commented May 28, 2020 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Member

Recent changes to _datetimemodule broke compilation on mingw; see the comments in this change for details.

FWIW, @corona10: this issue is why PyType_FromModuleAndSpec & friends take the bases argument at run time.

https://bugs.python.org/issue40777

Automerge-Triggered-By: @encukou
@encukou
encukou requested review from corona10 and pganssle May 28, 2020 15:47
@encukou
encukou requested a review from abalkin as a code owner May 28, 2020 15:47
pganssle
pganssle reviewed May 28, 2020
View reviewed changes
Comment thread Modules/_datetimemodule.c
.tp_methods = iso_calendar_date_methods,
.tp_getset = iso_calendar_date_getset,
.tp_base = &PyTuple_Type,
// .tp_base = &PyTuple_Type, // filled in PyInit__datetime

@pganssle pganssle May 28, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The xxsubtype.c module does this weird thing with a macro called DEFERRED_ADDRESS that always returns 0:

cpython/Modules/xxsubtype.c

Line 134 in 6039851

DEFERRED_ADDRESS(&PyList_Type), /* tp_base */

Not sure exactly why it's done that way, though.

@encukou encukou May 28, 2020

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's just documentation:

cpython/Modules/xxsubtype.c

Lines 10 to 17 in 6039851

/* We link this module statically for convenience. If compiled as a shared
library instead, some compilers don't allow addresses of Python objects
defined in other libraries to be used in static initializers here. The
DEFERRED_ADDRESS macro is used to tag the slots where such addresses
appear; the module init function must fill in the tagged slots at runtime.
The argument is for documentation -- the macro ignores it.
*/
#define DEFERRED_ADDRESS(ADDR) 0

Would you like to use DEFERRED_ADDRESS instead?
pganssle
pganssle approved these changes May 28, 2020
View reviewed changes

@pganssle pganssle left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another option is to use the DEFERRED_ADDRESS macro that xxsubtype uses, for documentation purposes, but I'm basically on the fence about that unless someone can come up with a clever way to use it for compile-time enforcement that those addresses actually get resolved — people might think it does something clever when it's just essentially a commenting convention.
@encukou

encukou commented May 28, 2020

Copy link
Copy Markdown
Member Author

I'm not much of a fan of DEFERRED_ADDRESS; as you say, it looks like it should do more than it does.
@miss-islington
miss-islington merged commit 459acc5 into python:master May 28, 2020
@miss-islington

miss-islington commented May 28, 2020

Copy link
Copy Markdown
Contributor

Thanks @encukou for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖
@encukou

encukou commented May 28, 2020

Copy link
Copy Markdown
Member Author

Thank you for the reviews!
@encukou
encukou deleted the bpo-40777 branch May 28, 2020 16:15
@bedevere-bot

bedevere-bot commented May 28, 2020

Copy link
Copy Markdown

GH-20495 is a backport of this pull request to the 3.9 branch.
miss-islington added a commit that referenced this pull request May 28, 2020
@miss-islington @encukou
[3.9] bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at…
eceee54 
… run-time (GH-20493) (GH-20495)

Recent changes to _datetimemodule broke compilation on mingw; see the comments in this change for details.

FWIW, @corona10: this issue is why `PyType_FromModuleAndSpec` & friends take the `bases` argument at run time.
(cherry picked from commit 459acc5)


Co-authored-by: Petr Viktorin <encukou@gmail.com>
CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request May 30, 2020
@CuriousLearner
Merge branch 'master' of github.com:python/cpython into bpo-26543
d3d7100 
* 'master' of github.com:python/cpython: (497 commits)
  bpo-40061: Fix a possible refleak in _asynciomodule.c (pythonGH-19748)
  bpo-40798: Generate a different message for already removed elements (pythonGH-20483)
  closes bpo-29017: Update the bindings for Qt information with PySide2 (pythonGH-20149)
  bpo-39885: Make IDLE context menu cut and copy work again (pythonGH-18951)
  bpo-29882: Add an efficient popcount method for integers (python#771)
  Further de-linting of zoneinfo module (python#20499)
  bpo-40780: Fix failure of _Py_dg_dtoa to remove trailing zeros (pythonGH-20435)
  Indicate that abs() method accept argument that implement __abs__(), just like call() method in the docs (pythonGH-20509)
  bpo-39040: Fix parsing of email mime headers with whitespace between encoded-words. (pythongh-17620)
  bpo-40784: Fix sqlite3 deterministic test (pythonGH-20448)
  bpo-30064: Properly skip unstable loop.sock_connect() racing test (pythonGH-20494)
  Note the output ordering of combinatoric functions (pythonGH-19732)
  bpo-40474: Updated coverage.yml to better report coverage stats (python#19851)
  bpo-40806: Clarify that itertools.product immediately consumes its inpt (pythonGH-20492)
  bpo-1294959: Try to clarify the meaning of platlibdir (pythonGH-20332)
  bpo-37878: PyThreadState_DeleteCurrent() was not removed (pythonGH-20489)
  bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time (pythonGH-20493)
  bpo-40755: Add missing multiset operations to Counter() (pythonGH-20339)
  bpo-25920: Remove socket.getaddrinfo() lock on macOS (pythonGH-20177)
  bpo-40275: Fix test.support.threading_helper (pythonGH-20488)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants

@encukou @miss-islington @bedevere-bot @pganssle @corona10 @the-knights-who-say-ni