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.

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

[3.10] bpo-43760: Ensure that older Cython generated code compiles under 3.10. #28474

Closed

Conversation

markshannon
Copy link
Member

@markshannon markshannon commented Sep 20, 2021

Not sure what sort of NEWS entry this needs, if any.

https://bugs.python.org/issue43760

@markshannon
Copy link
Member Author

Why does the ABI checker claim this breaks the ABI?
The size of the PyThreadStatestruct is not part of the ABI. It is never passed by value, only be pointer.

@pablogsal
Copy link
Member

pablogsal commented Sep 20, 2021

The size of the PyThreadStatestruct is not part of the ABI. It is never passed by value, only be pointer.

Because code can dereference the pointer and if you know the full definition the compiler needs the offsets to optimize member access.

@pablogsal
Copy link
Member

full definition the compiler needs the offsets to optimize member access.

For instance, this is this particular case:

  [C]'function void _PyErr_Clear(PyThreadState*)' at errors.c:453:1 has some indirect sub-type changes:
    parameter 1 of type 'PyThreadState*' has sub-type changes:
      in pointed to type 'typedef PyThreadState' at pystate.h:20:1:
        underlying type 'struct _ts' at pystate.h:62:1 changed:
          type size changed from 2240 to 2304 (in bits)
          1 data member insertion:
            'int _ts::use_tracing', at offset 384 (in bits) at pystate.h:79:1
          6 data member changes (22 filtered):
           'unsigned long int _ts::thread_id' offset changed from 1408 to 1472 (in bits) (by +64 bits)
           'int _ts::trash_delete_nesting' offset changed from 1472 to 1536 (in bits) (by +64 bits)
           'PyObject* _ts::trash_delete_later' offset changed from 1536 to 1600 (in bits) (by +64 bits)
           'void (void*)* _ts::on_delete' offset changed from 1600 to 1664 (in bits) (by +64 bits)
           'void* _ts::on_delete_data' offset changed from 1664 to 1728 (in bits) (by +64 bits)
           'CFrame _ts::root_cframe' offset changed from 2112 to 2176 (in bits) (by +64 bits)

@pablogsal
Copy link
Member

Don't worry too much, as the fields are not public we can just regenerate the ABI. The check is there so you don't change it unadvertedly.

@pablogsal
Copy link
Member

Seems I cannot push to the fork you are using. Can you make sure the checkbox allowing pushes from maintaners is checked on this PR (on the right of the PR page, at the bottom IIRC)

@pablogsal
Copy link
Member

lel

@pablogsal
Copy link
Member

Doesn't matter, I have created #28498

@pablogsal pablogsal closed this Sep 21, 2021
ameily added a commit to ameily/cpython that referenced this pull request Oct 2, 2021
@markshannon markshannon deleted the semi-restore-use-tracing branch January 6, 2022 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants