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-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VERSION_TAG no-op #27260

Merged
merged 3 commits into from
Jul 23, 2021
Merged

bpo-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VERSION_TAG no-op #27260

merged 3 commits into from
Jul 23, 2021

Conversation

encukou
Copy link
Member

@encukou encukou commented Jul 20, 2021 •
edited by miss-islington
Loading

  • Remove code that checks Py_TPFLAGS_HAVE_VERSION_TAG

    The field is always present in the type struct, as explained
    in the added comment.

  • Remove Py_TPFLAGS_HAVE_AM_SEND

    The flag is not needed, and since it was added in 3.10 it can be removed now.

https://bugs.python.org/issue42747

Automerge-Triggered-By: GH:encukou

encukou added 3 commits July 20, 2021 16:34
@encukou
Remove code that checks Py_TPFLAGS_HAVE_VERSION_TAG
bdad9e8 
The field is always present in the type struct, as explained
in the added comment.
@encukou
Remove Py_TPFLAGS_HAVE_AM_SEND
91ccfc4 
The flag is not needed.
@encukou
Add a blurb
7a19100
@encukou encukou added the needs backport to 3.10 only security fixes label Jul 20, 2021
@encukou encukou requested a review from markshannon as a code owner July 20, 2021 14:40
@encukou encukou force-pushed the remove-unnecessary-tpflags branch from f3ea135 to 7a19100 Compare July 20, 2021 14:40
@pablogsal
Copy link
Member

How this flag was added and when it was turned into a no-op?

@encukou
Copy link
Member Author

encukou commented Jul 20, 2021 •
edited
Loading

Py_TPFLAGS_HAVE_* flags have been historically added whenever a new field was added to the type struct, to make it possible to use extensions compiled for earlier Python versions (where the fields aren't present).
But extensions need to be recompiled for each feature version (as long I can remember, and definitely in Python 3.0+), or use the stable ABI (where types use the interpreter's memory layout).

This PR turns both into no-ops, as they should be.
One of the removed code paths probably has an issue, see bpo-44588 -- but I didn't investigate that deeper yet.

shihai1991
shihai1991 reviewed Jul 20, 2021
View reviewed changes
Misc/NEWS.d/next/C API/2021-07-20-16-21-06.bpo-42747.rRxjUY.rst
@@ -0,0 +1,4 @@
The ``Py_TPFLAGS_HAVE_VERSION_TAG`` type flag now does nothing. The
``Py_TPFLAGS_HAVE_AM_SEND`` flag (which was added in 3.10) is removed. Both
Copy link
Member

Choose a reason for hiding this comment

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

Py_TPFLAGS_HAVE_AM_SEND flag (which was added in 3.10) is removed.

there aren't many users use it?

Copy link
Member Author

Choose a reason for hiding this comment

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

If there any users of it, they are using the 3.10 betas. Requiring that they remove the flag is fair game.

@miss-islington miss-islington merged commit a4760cc into python:main Jul 23, 2021
@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

GH-27306 is a backport of this pull request to the 3.10 branch.

@encukou encukou deleted the remove-unnecessary-tpflags branch July 23, 2021 13:21
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Jul 23, 2021
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jul 23, 2021
@encukou @miss-islington
bpo-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VE…
ab38497 
…RSION_TAG no-op (pythonGH-27260)

* Remove code that checks Py_TPFLAGS_HAVE_VERSION_TAG

    The field is always present in the type struct, as explained
    in the added comment.

* Remove Py_TPFLAGS_HAVE_AM_SEND

    The flag is not needed, and since it was added in 3.10 it can be removed now.
(cherry picked from commit a4760cc)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
encukou added a commit that referenced this pull request Jul 23, 2021
@miss-islington @encukou
bpo-42747: Remove Py_TPFLAGS_HAVE_AM_SEND and make Py_TPFLAGS_HAVE_VE…
632e8a6 
…RSION_TAG no-op (GH-27260) (GH-27306)

* Remove code that checks Py_TPFLAGS_HAVE_VERSION_TAG

    The field is always present in the type struct, as explained
    in the added comment.

* Remove Py_TPFLAGS_HAVE_AM_SEND

    The flag is not needed, and since it was added in 3.10 it can be removed now.
(cherry picked from commit a4760cc)

Co-authored-by: Petr Viktorin <encukou@gmail.com>
@vstinner
Copy link
Member

vstinner commented Aug 9, 2021

Thanks for this fix @encukou. The rationale for Py_TPFLAGS_HAVE_AM_SEND was unclear. Now it's well defined, it's useless :-D

@Wesley-Jzy Wesley-Jzy mannequin mentioned this pull request Jul 21, 2022
Closed
@colesbury colesbury mentioned this pull request Dec 9, 2024
Open
@encukou encukou mentioned this pull request Dec 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corona10 corona10 corona10 approved these changes

@shihai1991 shihai1991 shihai1991 approved these changes

@pablogsal pablogsal pablogsal approved these changes

@vstinner vstinner Awaiting requested review from vstinner

@1st1 1st1 Awaiting requested review from 1st1 1st1 is a code owner

@asvetlov asvetlov Awaiting requested review from asvetlov asvetlov is a code owner

@serhiy-storchaka serhiy-storchaka Awaiting requested review from serhiy-storchaka

@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

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

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