Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork32.1k
gh-105020: Share tp_bases and tp_mro Between Interpreters For All Static Builtin Types#105115
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Do we have a meaningful place to put a regression test to assert that tp_bases and tp_mro are never NULL in the situations user code was encountering that in 3.12beta1?
Include/internal/pycore_typeobject.h Outdated
| @@ -49,8 +49,6 @@ typedef struct { | |||
| // XXX tp_dict, tp_bases, and tp_mro can probably be statically | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
should this comment (added by the original PR) be updated now that bases and mro don't exist here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
fixed
I'll add a test in |
I've added a test. Thanks for the suggestion. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This looks good to me.
I do wonder if anything is going to be tripped up by thetp_dict as well, but that seems like a separate category of change given I believe it could've been NULL in the past - if it comes up it can be addressed separately.
you might want a NEWS entry fwiw... as people do read the changelog between beta1 and beta2. |
Thanks@ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
Sorry@ericsnowcurrently, I had trouble checking out the |
Thanks@ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
…ll Static Builtin Types (pythongh-105115)Inpythongh-103912 we added tp_bases and tp_mro to each PyInterpreterState.types.builtins entry. However, doing so ignored the fact that both PyTypeObject fields are public API, and not documented as internal (as opposed to tp_subclasses). We address that here by reverting back to shared objects, making them immortal in the process.(cherry picked from commit7be667d)Co-authored-by: Eric Snow <ericsnowcurrently@gmail.com>
bedevere-bot commentedMay 31, 2023
GH-105124 is a backport of this pull request to the3.12 branch. |
…All Static Builtin Types (gh-105115) (gh-105124)Ingh-103912 we added tp_bases and tp_mro to each PyInterpreterState.types.builtins entry. However, doing so ignored the fact that both PyTypeObject fields are public API, and not documented as internal (as opposed to tp_subclasses). We address that here by reverting back to shared objects, making them immortal in the process.(cherry picked from commit7be667d)Co-authored-by: Eric Snow ericsnowcurrently@gmail.com
Uh oh!
There was an error while loading.Please reload this page.
Ingh-103912 we added tp_bases and tp_mro to each
PyInterpreterState.types.builtinsentry. However, doing so ignored the fact that bothPyTypeObjectfields are public API, and not documented as internal (as opposed to tp_subclasses). We address that here by reverting back to shared objects, making them immortal in the process.tp_basesofobjectisNULL: undocumented or unintentional behavior change in 3.12? #105020