GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — bpo-40777: Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time by encukou · Pull Request #20493 · python/cpython

Conversation

encukou commented May 28, 2020 •

edited by miss-islington

Loading

Uh oh!

There was an error while loading. Please reload this page.

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions


          Initialize PyDateTime_IsoCalendarDateType.tp_base at run-time

833bc41

encukou added the needs backport to 3.9 label

May 28, 2020

encukou requested review from corona10 and pganssle

May 28, 2020 15:47

encukou requested a review from abalkin as a code owner

May 28, 2020 15:47

the-knights-who-say-ni added the CLA signed label

May 28, 2020

bedevere-bot added the awaiting core review label

May 28, 2020

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 May 28, 2020

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

encukou May 28, 2020

Copy link

Copy Markdown

Member Author

There was a problem hiding this comment.

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?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

pganssle approved these changes

May 28, 2020

View reviewed changes

pganssle left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this 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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

bedevere-bot added awaiting merge and removed awaiting core review labels

May 28, 2020

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

corona10 approved these changes

May 28, 2020

View reviewed changes

encukou added the 🤖 automerge label

May 28, 2020

miss-islington merged commit 459acc5 into python:master

May 28, 2020

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.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the awaiting merge label

May 28, 2020

encukou commented May 28, 2020

Copy link

Copy Markdown

Member Author

Thank you for the reviews!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

encukou deleted the bpo-40777 branch

May 28, 2020 16:15

bedevere-bot commented May 28, 2020

Copy link

Copy Markdown

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

bedevere-bot removed the needs backport to 3.9 label

May 28, 2020

miss-islington added a commit that referenced this pull request

May 28, 2020


          [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 <[email protected]>

CuriousLearner added a commit to CuriousLearner/cpython that referenced this pull request

May 30, 2020


          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)
  ...

Reviewers

pganssle

pganssle approved these changes

corona10

corona10 approved these changes

abalkin

Awaiting requested review from abalkin

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.

GeezerVison

Conversation

encukou commented May 28, 2020 • edited by miss-islington Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pganssle May 28, 2020

Choose a reason for hiding this comment

Uh oh!

encukou May 28, 2020

Choose a reason for hiding this comment

Uh oh!

pganssle left a comment

Choose a reason for hiding this comment

Uh oh!

encukou commented May 28, 2020

Uh oh!

miss-islington commented May 28, 2020

Uh oh!

encukou commented May 28, 2020

Uh oh!

bedevere-bot commented May 28, 2020

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

encukou commented May 28, 2020 •

edited by miss-islington

Loading

Uh oh!

There was an error while loading. Please reload this page.