◐ Shell
reader mode source ↗
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

@vstinner vstinner commented Jul 18, 2023 •
edited by bedevere-bot
Loading

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)

@bedevere-bot bedevere-bot mentioned this pull request Jul 18, 2023
Closed
@vstinner

vstinner commented Jul 18, 2023

Copy link
Copy Markdown
Member Author

I created this PR with this upgrade_structmember.py script:

#!/usr/bin/python3
import sys
import re

encoding = "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)
            for pattern, replace in patterns]
patterns.append((re.compile('#include "structmember.h".*$', re.MULTILINE), ''))

def update(filename):
    with open(filename, encoding=encoding) as fp:
        content = fp.read()
    old_content = content
    for pattern, replace in patterns:
        content = pattern.sub(replace, content)

    if content != old_content:
        print(f"Update {filename}")
        with open(filename, "w", encoding=encoding) as fp:
            fp.write(content)

def main():
    if len(sys.argv) < 2:
        print("usage: update.py filename [filename2 ...]")
        sys.exit(1)

    for filename in sys.argv[1:]:
        update(filename)

if __name__ == "__main__":
    main()

And the shell command:

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

serhiy-storchaka
serhiy-storchaka reviewed Jul 19, 2023
View reviewed changes

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

Hide comment

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

vstinner commented Jul 19, 2023

Copy link
Copy Markdown
Member Author

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

vstinner commented Jul 19, 2023

Copy link
Copy Markdown
Member Author

Oh, macOS fails to build:

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

@vstinner

vstinner commented Jul 19, 2023

Copy link
Copy Markdown
Member Author

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 vstinner force-pushed the remove_structmember branch 2 times, most recently from 11bef5d to 0c91ecc Compare July 25, 2023 12:09
@vstinner

vstinner commented Jul 25, 2023

Copy link
Copy Markdown
Member Author

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

@vstinner vstinner marked this pull request as ready for review July 25, 2023 12:19
@vstinner
pythongh-106869: Use new PyMemberDef constant names …
8d13210 
* 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 vstinner force-pushed the remove_structmember branch from 0c91ecc to 8d13210 Compare July 25, 2023 12:22
@vstinner

vstinner commented Jul 25, 2023

Copy link
Copy Markdown
Member Author

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

@vstinner

vstinner commented Jul 25, 2023

Copy link
Copy Markdown
Member Author

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:main Jul 25, 2023
@vstinner vstinner deleted the remove_structmember branch July 25, 2023 13:28
@bedevere-bot bedevere-bot removed the label Jul 25, 2023
@gvanrossum gvanrossum mentioned this pull request Jul 25, 2023
Closed
@shailshouryya shailshouryya mentioned this pull request Jul 26, 2023
Merged
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

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

@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

@erlend-aasland erlend-aasland Awaiting requested review from erlend-aasland erlend-aasland is a code owner

@isidentical isidentical Awaiting requested review from isidentical

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

@abalkin abalkin Awaiting requested review from abalkin

@tiran tiran Awaiting requested review from tiran

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

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

@1st1 1st1 Awaiting requested review from 1st1

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

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

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

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

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vstinner @serhiy-storchaka @bedevere-bot