Skip to content

gh-106869: Use new PyMemberDef constant names - #106871

Merged
vstinner merged 1 commit into
python:mainfrom
vstinner:remove_structmember
Jul 25, 2023
Merged

gh-106869: Use new PyMemberDef constant names#106871
vstinner merged 1 commit into
python:mainfrom
vstinner:remove_structmember

Conversation

@vstinner

@vstinnervstinner commented Jul 18, 2023

Copy link
Copy Markdown
Member
  • Remove '#include "structmember.h"'.

  • If needed, add <stddef.h> to get offsetof() function.

  • Replace:

    • T_SHORT => Py_T_SHORT
    • T_INT => Py_T_INT
    • T_LONG => Py_T_LONG
    • T_FLOAT => Py_T_FLOAT
    • T_DOUBLE => Py_T_DOUBLE
    • T_STRING => Py_T_STRING
    • T_OBJECT => _Py_T_OBJECT
    • T_CHAR => Py_T_CHAR
    • T_BYTE => Py_T_BYTE
    • T_UBYTE => Py_T_UBYTE
    • T_USHORT => Py_T_USHORT
    • T_UINT => Py_T_UINT
    • T_ULONG => Py_T_ULONG
    • T_STRING_INPLACE => Py_T_STRING_INPLACE
    • T_BOOL => Py_T_BOOL
    • T_OBJECT_EX => Py_T_OBJECT_EX
    • T_LONGLONG => Py_T_LONGLONG
    • T_ULONGLONG => Py_T_ULONGLONG
    • T_PYSSIZET => Py_T_PYSSIZET
    • T_NONE => _Py_T_NONE
    • READONLY => Py_READONLY
    • PY_AUDIT_READ => Py_AUDIT_READ
    • READ_RESTRICTED => Py_AUDIT_READ
    • PY_WRITE_RESTRICTED => _Py_WRITE_RESTRICTED
    • RESTRICTED => (READ_RESTRICTED | PY_WRITE_RESTRICTED)

@vstinner

Copy link
Copy Markdown
MemberAuthor

I created this PR with this upgrade_structmember.py script:

#!/usr/bin/python3importsysimportreencoding="utf8"patterns= (
('T_SHORT', 'Py_T_SHORT'),
('T_INT', 'Py_T_INT'),
('T_LONG', 'Py_T_LONG'),
('T_FLOAT', 'Py_T_FLOAT'),
('T_DOUBLE', 'Py_T_DOUBLE'),
('T_STRING', 'Py_T_STRING'),
('T_OBJECT', '_Py_T_OBJECT'),
('T_CHAR', 'Py_T_CHAR'),
('T_BYTE', 'Py_T_BYTE'),
('T_UBYTE', 'Py_T_UBYTE'),
('T_USHORT', 'Py_T_USHORT'),
('T_UINT', 'Py_T_UINT'),
('T_ULONG', 'Py_T_ULONG'),
('T_STRING_INPLACE', 'Py_T_STRING_INPLACE'),
('T_BOOL', 'Py_T_BOOL'),
('T_OBJECT_EX', 'Py_T_OBJECT_EX'),
('T_LONGLONG', 'Py_T_LONGLONG'),
('T_ULONGLONG', 'Py_T_ULONGLONG'),
('T_PYSSIZET', 'Py_T_PYSSIZET'),
('T_NONE', '_Py_T_NONE'),
('READONLY', 'Py_READONLY'),
('PY_AUDIT_READ', 'Py_AUDIT_READ'),
('READ_RESTRICTED', 'Py_AUDIT_READ'),
('PY_WRITE_RESTRICTED', '_Py_WRITE_RESTRICTED'),
('RESTRICTED', '(Py_AUDIT_READ | _Py_WRITE_RESTRICTED)'),
)
patterns= [(re.compile(fr'\b{pattern}\b'), replace)
forpattern, replaceinpatterns]
patterns.append((re.compile('#include "structmember.h".*$', re.MULTILINE), ''))
defupdate(filename):
withopen(filename, encoding=encoding) asfp:
content=fp.read()
old_content=contentforpattern, replaceinpatterns:
content=pattern.sub(replace, content)
ifcontent!=old_content:
print(f"Update {filename}")
withopen(filename, "w", encoding=encoding) asfp:
fp.write(content)
defmain():
iflen(sys.argv) <2:
print("usage: update.py filename [filename2 ...]")
sys.exit(1)
forfilenameinsys.argv[1:]:
update(filename)
if__name__=="__main__":
main()

And the shell command:

find -name "*.c" -print0|xargs -0 ./upgrade_structmember.py

@serhiy-storchakaserhiy-storchaka 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.

If it was created by a script, there is no need to read every line for accepting this PR.

Before committing these changes I have questions:

  • Why _Py_T_OBJECT is underscored?
  • Why is prefix Py_ used, but not PY_?

@vstinner

Copy link
Copy Markdown
MemberAuthor

Why Py_T_OBJECT is underscored?
Why is prefix Py
used, but not PY_?

@encukou designed this API in Python 3.12. If if needs to be changed, maybe it's not too late to change it.

I'm fine with Py prefix for everything: the fact that constants are defined as macros or variables doesn't matter to me.

But I'm also curious about the fact that _Py_T_OBJECT is private and the only public constant for object has an EX suffix: maybe it could be renamed Py_T_OBJECT and become the default?

I would prefer to not switch from T_OBJECT (return None) to Py_T_OBJECT_EX (raise AttributeError) in this mechanical change. Maybe it's the same for other people migrating to Python 3.12 API?

By the way, I'm considering to expose these new Py_T_xxx constants to pythoncapi_compat.h to help projects migrating to the new API without losing support for old Python versions.

@vstinner

Copy link
Copy Markdown
MemberAuthor

Oh, macOS fails to build:

Modules/selectmodule.c:1839:41: error: implicit declaration of function 'offsetof' is invalid in C99

@vstinner

Copy link
Copy Markdown
MemberAuthor

If it was created by a script, there is no need to read every line for accepting this PR.

I just added stddef.h includes to fix build issues 😁

@vstinner
vstinnerforce-pushed the remove_structmember branch 2 times, most recently from 11bef5d to 0c91eccCompareJuly 25, 2023 12:09
@vstinner

Copy link
Copy Markdown
MemberAuthor

PR rebased on the main branch to fix merge conflicts. I also added the missing #include for macOS in the select module.

* Remove '#include "structmember.h"'.
* If needed, add <stddef.h> to get offsetof() function.
* Replace:
* T_SHORT => Py_T_SHORT
* T_INT => Py_T_INT
* T_LONG => Py_T_LONG
* T_FLOAT => Py_T_FLOAT
* T_DOUBLE => Py_T_DOUBLE
* T_STRING => Py_T_STRING
* T_OBJECT => _Py_T_OBJECT
* T_CHAR => Py_T_CHAR
* T_BYTE => Py_T_BYTE
* T_UBYTE => Py_T_UBYTE
* T_USHORT => Py_T_USHORT
* T_UINT => Py_T_UINT
* T_ULONG => Py_T_ULONG
* T_STRING_INPLACE => Py_T_STRING_INPLACE
* T_BOOL => Py_T_BOOL
* T_OBJECT_EX => Py_T_OBJECT_EX
* T_LONGLONG => Py_T_LONGLONG
* T_ULONGLONG => Py_T_ULONGLONG
* T_PYSSIZET => Py_T_PYSSIZET
* T_NONE => _Py_T_NONE
* READONLY => Py_READONLY
* PY_AUDIT_READ => Py_AUDIT_READ
* READ_RESTRICTED => Py_AUDIT_READ
* PY_WRITE_RESTRICTED => _Py_WRITE_RESTRICTED
* RESTRICTED => (READ_RESTRICTED | _Py_WRITE_RESTRICTED)
@vstinner
vstinnerforce-pushed the remove_structmember branch from 0c91ecc to 8d13210CompareJuly 25, 2023 12:22
@vstinner

Copy link
Copy Markdown
MemberAuthor

Oops, I reverted my changes on Modules/_testcapi/structmember.c: this C file tests the old constants.

@vstinner

Copy link
Copy Markdown
MemberAuthor

I also updated Parser/asdl_c.py to update Python/Python-ast.c.

@rhettinger
rhettinger removed their request for review July 25, 2023 13:18
@vstinner
vstinner merged commit 1a3faba into python:mainJul 25, 2023
@vstinner
vstinner deleted the remove_structmember branch July 25, 2023 13:28
jtcave pushed a commit to jtcave/cpython that referenced this pull request Jul 27, 2023
* Remove '#include "structmember.h"'.
* If needed, add <stddef.h> to get offsetof() function.
* Update Parser/asdl_c.py to regenerate Python/Python-ast.c.
* Replace:
* T_SHORT => Py_T_SHORT
* T_INT => Py_T_INT
* T_LONG => Py_T_LONG
* T_FLOAT => Py_T_FLOAT
* T_DOUBLE => Py_T_DOUBLE
* T_STRING => Py_T_STRING
* T_OBJECT => _Py_T_OBJECT
* T_CHAR => Py_T_CHAR
* T_BYTE => Py_T_BYTE
* T_UBYTE => Py_T_UBYTE
* T_USHORT => Py_T_USHORT
* T_UINT => Py_T_UINT
* T_ULONG => Py_T_ULONG
* T_STRING_INPLACE => Py_T_STRING_INPLACE
* T_BOOL => Py_T_BOOL
* T_OBJECT_EX => Py_T_OBJECT_EX
* T_LONGLONG => Py_T_LONGLONG
* T_ULONGLONG => Py_T_ULONGLONG
* T_PYSSIZET => Py_T_PYSSIZET
* T_NONE => _Py_T_NONE
* READONLY => Py_READONLY
* PY_AUDIT_READ => Py_AUDIT_READ
* READ_RESTRICTED => Py_AUDIT_READ
* PY_WRITE_RESTRICTED => _Py_WRITE_RESTRICTED
* RESTRICTED => (READ_RESTRICTED | _Py_WRITE_RESTRICTED)
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner@serhiy-storchaka@bedevere-bot