[3.13] gh-117482: Simplify the Fix For Builtin Types Slot Wrappers #121932
Conversation
There was a problem hiding this comment.
Would it make sense to set a flag for each slot that is explicitly overridden when we first see the PyTypeObject struct. That way, we can refer to that during subsequent calls to know whether the field was set by the author or by the VM?
We only need about 80 bits per type
|
Thinking about this so more, I think the problem is that we are attempting to initial static builtin types more than once. |
I agree, but this would probably be too much of a change for 3.13 and 3.12. |
|
Does this fix the issue? The change itself is a bit ad-hoc, but it looks fine. |
The existing test does verify that the underlying is still fixed. I'll add a test for the embedding case. |
|
Thanks @ericsnowcurrently for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
ericsnowcurrently
deleted the
simplify-builtin-static-type-add-operators-3.13
branch
…ers (pythongh-121932) In pythongh-121602, I applied a fix to a builtin types initialization bug. That fix made sense in the context of some broader future changes, but introduced a little bit of extra complexity. For earlier versions those future changes are not relevant; we can avoid the extra complexity. Thus we can revert that earlier change and replace it with this one, which is more focused and conceptually simpler. This is essentially the implementation of an idea that @markshannon pointed out to me. Note that this change would be much smaller if we didn't have to deal with repr compatibility for builtin types that explicitly inherit tp slots (see expect_manually_inherited()). The alternative is to stop *explicitly* inheriting tp slots in static PyTypeObject values, which is churn that we can do separately. (cherry picked from commit 716c677, AKA pythongh-121932)
…h-122241) In gh-121602, I applied a fix to a builtin types initialization bug. That fix made sense in the context of some broader future changes, but introduced a little bit of extra complexity. For earlier versions those future changes are not relevant; we can avoid the extra complexity. Thus we can revert that earlier change and replace it with this one, which is more focused and conceptually simpler. This is essentially the implementation of an idea that @markshannon pointed out to me. Note that this change would be much smaller if we didn't have to deal with repr compatibility for builtin types that explicitly inherit tp slots (see expect_manually_inherited()). The alternative is to stop *explicitly* inheriting tp slots in static PyTypeObject values, which is churn that we can do separately. (cherry picked from commit 716c677, AKA gh-121932)
In gh-121602, I applied a fix to a builtin types initialization bug. That fix made sense in the context of some broader future changes, but introduced a little bit of extra complexity. For earlier versions those future changes are not relevant; we can avoid the extra complexity. Thus we can revert that earlier change and replace it with one that is more focused and conceptually simpler.
That's this PR. It's essentially the implementation of an idea that @markshannon pointed out to me. Note that this change would be much smaller if we didn't have to deal with compatibility for builtin types that explicitly inherit tp slots (see
expect_manually_inherited()).int.__str__behaviour inside sub-interpreters #117482