Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 1 commit into from
May 28, 2020
Merged

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

merged 1 commit into from
May 28, 2020

Conversation

encukou
Copy link
Member

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

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 added the needs backport to 3.9 only security fixes label May 28, 2020
@encukou encukou requested review from pganssle and corona10 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
Modules/_datetimemodule.c
@@ -3325,7 +3325,7 @@ static PyTypeObject PyDateTime_IsoCalendarDateType = {
.tp_doc = iso_calendar_date__doc__,
.tp_methods = iso_calendar_date_methods,
.tp_getset = iso_calendar_date_getset,
.tp_base = &PyTuple_Type,
// .tp_base = &PyTuple_Type, // filled in PyInit__datetime
Copy link
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.

Copy link
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
Copy link
Member

@pganssle pganssle left a comment

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
Copy link
Member Author

encukou commented May 28, 2020

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
Copy link
Contributor

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

@encukou
Copy link
Member Author

encukou commented May 28, 2020

Thank you for the reviews!

@encukou encukou deleted the bpo-40777 branch May 28, 2020 16:15
@bedevere-bot
Copy link

GH-20495 is a backport of this pull request to the 3.9 branch.

@bedevere-bot bedevere-bot removed the needs backport to 3.9 only security fixes label May 28, 2020
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
Reviewers

@pganssle pganssle pganssle approved these changes

@corona10 corona10 corona10 approved these changes

@abalkin abalkin Awaiting requested review from abalkin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@encukou @miss-islington @bedevere-bot @pganssle @corona10 @the-knights-who-say-ni