GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — [3.10] bpo-43760: Ensure that older Cython generated code compiles under 3.10. by markshannon · Pull Request #28474 · python/cpython

Conversation

@markshannon

@markshannon markshannon commented Sep 20, 2021 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

https://bugs.python.org/issue43760

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@markshannon
Add tstate.use_tracing as a copy of tstate.cframe->use_tracing to min…
f9c1b77
…imize breakage in Cython generated modules.
@markshannon markshannon assigned pablogsal Sep 20, 2021
@the-knights-who-say-ni the-knights-who-say-ni added the CLA signed label Sep 20, 2021
@bedevere-bot bedevere-bot added the awaiting core review label Sep 20, 2021
@markshannon

markshannon commented Sep 20, 2021

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Sep 20, 2021 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Sep 20, 2021

Copy link
Copy Markdown
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)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Sep 20, 2021

Copy link
Copy Markdown
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.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@markshannon
Move new field to end of struct for better binary compatibility.
6eb2c12
@pablogsal

pablogsal commented Sep 21, 2021

Copy link
Copy Markdown
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)

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal pablogsal added the skip news label Sep 21, 2021
@pablogsal

pablogsal commented Sep 21, 2021

Copy link
Copy Markdown
Member

lel

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal

pablogsal commented Sep 21, 2021

Copy link
Copy Markdown
Member

Doesn't matter, I have created #28498

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pablogsal pablogsal closed this Sep 21, 2021
ameily added a commit to ameily/cpython that referenced this pull request Oct 2, 2021
@ameily
properly handle unsigned long win32 error codes
887f876
fixes python#28474
@markshannon
markshannon deleted the semi-restore-use-tracing branch January 6, 2022 15:27
@markshannon markshannon mentioned this pull request Sep 13, 2022
The DISPATCH() macro is not as efficient as it could be (move PyThreadState.use_tracing) #87926
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@pablogsal pablogsal

Labels

awaiting core review skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@markshannon @pablogsal @the-knights-who-say-ni @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.