◐ Shell
reader mode source ↗
Skip to content

gh-47146: Soft-deprecate structmember.h, expose its contents via Python.h#99014

Merged
encukou merged 14 commits into
python:mainfrom
encukou:structmember-public
Nov 22, 2022
Merged

gh-47146: Soft-deprecate structmember.h, expose its contents via Python.h#99014
encukou merged 14 commits into
python:mainfrom
encukou:structmember-public

Conversation

@encukou

@encukou encukou commented Nov 2, 2022 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

The structmember.h header is deprecated, though it continues to be available and there are no plans to remove it. There are no deprecation warnings. Old code can stay unchanged (unless the extra include and non-namespaced macros bother you greatly). Specifically, no uses in CPython are updated -- that would just be unnecessary churn.

The header's contents are now available just by including Python.h, with a PY_ prefix added if it was missing:

  • PyMemberDef, PyMember_GetOne, PyMember_SetOne
  • Type macros like PY_T_INT, PY_T_DOUBLE, etc. (previously T_INT, T_DOUBLE, etc.)
  • The flags PY_READONLY (previously READONLY) and PY_AUDIT_READ (name unchanged)

Several items are not exposed from Python.h:

  • T_OBJECT (use PY_T_OBJECT_EX)
  • T_NONE (previously undocumented, and pretty quirky)
  • The macro WRITE_RESTRICTED which does nothing.
  • The macros RESTRICTED and READ_RESTRICTED, equivalents of PY_AUDIT_READ.
  • In some configurations, <stddef.h> is not included from Python.h.
    It should be included manually when using offsetof().

The PY_T_*, PY_READONLY and PY_AUDIT_READ macros are added to the stable API manifest. This is just a clerical change, really -- Stable ABI extensions in the wild use structmember.h, and PyMemberDef & Py_tp_members are already listed.

There is discussion on the issue to rename T_PYSSIZET to PY_T_SSIZE or similar. I chose not to do that -- users will probably copy/paste that with any spelling, and not renaming it makes migration docs simpler.

Co-Authored-By: Alexander Belopolsky abalkin@users.noreply.github.com
Co-Authored-By: Matthias Braun MatzeB@users.noreply.github.com

@encukou
Move structmember.h definitions to descrobject.h, adding PY_ prefixes …
33e614a 
Old names are kept as aliases.
@encukou
Add to limited API
388c6ef
@encukou
All the docs …
498365d 
These docs were ripe for an overhaul, so I did one when touching the code.
@encukou
NEWS & What's New
714bd57
@bedevere-bot bedevere-bot mentioned this pull request Nov 2, 2022
Closed
@encukou encukou mentioned this pull request Nov 2, 2022
Closed
@encukou

encukou commented Nov 7, 2022

Copy link
Copy Markdown
Member Author

Today I noticed the style guide says macros should have the Py_ prefix, not PY_. A minor point, but since this is renaming a bunch of macros, I'll switch to the preferred form.

@encukou encukou marked this pull request as draft November 7, 2022 12:45
@encukou encukou marked this pull request as ready for review November 8, 2022 14:10
@encukou encukou marked this pull request as draft November 8, 2022 16:33
@encukou

encukou commented Nov 8, 2022

Copy link
Copy Markdown
Member Author

The PY_T_*, PY_READONLY and PY_AUDIT_READ macros are added to the stable API manifest. This is just a clerical change, really -- Stable ABI extensions in the wild use structmember.h, and PyMemberDef & Py_tp_members are already listed.

And by the same token, PyMember_GetOne and PyMember_SetOne should be added too.

@encukou encukou marked this pull request as ready for review November 10, 2022 13:19
@encukou encukou requested a review from a team as a code owner November 15, 2022 14:17
@encukou

encukou commented Nov 16, 2022

Copy link
Copy Markdown
Member Author

And here are tests.
I'd appreciate a review, if anyone can spare the time.

@encukou encukou added the Test PR w/ buildbots; report in status section label Nov 16, 2022
@bedevere-bot

bedevere-bot commented Nov 16, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @encukou for commit e44b7ca 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Nov 16, 2022
erlend-aasland
erlend-aasland approved these changes Nov 16, 2022
View reviewed changes

@erlend-aasland erlend-aasland left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hide comment

Looks good. I did some nitpicking.

@encukou

encukou commented Nov 21, 2022

Copy link
Copy Markdown
Member Author

Thank you! I did the changes, and will merge once everything is green.

@encukou encukou merged commit 4d82f62 into python:main Nov 22, 2022
@encukou encukou deleted the structmember-public branch November 22, 2022 07:25
@vstinner

vstinner commented Nov 22, 2022

Copy link
Copy Markdown
Member

Nice work!

@encukou encukou mentioned this pull request Dec 6, 2022
Merged
@iritkatriel iritkatriel mentioned this pull request Feb 18, 2023
Closed
@serhiy-storchaka

serhiy-storchaka commented Jul 25, 2023

Copy link
Copy Markdown
Member

Isn't it strange that the only public constant for object has an _EX suffix? Why not rename Py_T_OBJECT_EX to Py_T_OBJECT?

@vstinner

vstinner commented Jul 25, 2023

Copy link
Copy Markdown
Member

I agree that _Py_T_OBJECT is surprising: returning None if an attribute doesn't exist, so it's not possible to know if the attribute value is None or if the attribute doesn't exist.

Py_T_OBJECT_EX behavior of raising an AttributeError if the attribute value is NULL doesn't better. I agree with @serhiy-storchaka, it should just be Py_T_OBJECT.

By the way, how was _Py_T_NONE used in the past? It's documented as:

Deprecated. Value is always None.

@serhiy-storchaka

serhiy-storchaka commented Jul 25, 2023

Copy link
Copy Markdown
Member

Both _Py_T_OBJECT and Py_T_OBJECT_EX correspond to behavior of attributes of instances of Python classes. Just the latter is the default behavior, and the former is for the case when an instance attribute shadows the class attribute set to None. See an example in #107253.

T_NONE was introduced in 50e9fb9. I have not found a single case of its use.

@vstinner

vstinner commented Jul 25, 2023

Copy link
Copy Markdown
Member

T_NONE was introduced in 50e9fb9. I have not found a single case of its use.

Maybe it's not worth it to add _Py_NONE in this case. I don't know.

@encukou

encukou commented Aug 2, 2023

Copy link
Copy Markdown
Member Author

Isn't it strange that the only public constant for object has an _EX suffix? Why not rename Py_T_OBJECT_EX to Py_T_OBJECT?

T_OBJECT is still public API, it's just discouraged. (Probably not properly “soft-deprecated” in the new sense though?)
Since all the others add Py_, I thought it would be confusing to also drop the suffix in this case.

@XuehaiPan XuehaiPan mentioned this pull request Oct 2, 2023
Merged
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka left review comments

@erlend-aasland erlend-aasland erlend-aasland approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@encukou @bedevere-bot @vstinner @serhiy-storchaka @erlend-aasland