-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
GH-95589: Dont crash when subclassing extension classes with multiple inheritance #96028
GH-95589: Dont crash when subclassing extension classes with multiple inheritance #96028
Conversation
Doc/whatsnew/3.12.rst
Outdated
:const:`Py_TPFLAGS_MANAGED_WEAKREF` instead of ``tp_dictoffset`` and | ||
``tp_weaklistoffset``, respectively. | ||
The use of ``tp_dictoffset`` and ``tp_weaklistoffset`` is still | ||
supported, but will not fully support multiple inheritance |
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.
supported, but will not fully support multiple inheritance | |
supported, but does not fully support multiple inheritance |
I was a little surprised by the tense here, but I also may be wrong.
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.
You are right.
Because this doesn't apply right now, I unthinkingly use the future tense.
pass | ||
class B2(Both1, cls): | ||
class B2(C4, cls): | ||
pass |
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 we also assert that the mro
of these classes looks correct?
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 won't change the mro. That's determined by the C3 algorithm. https://en.wikipedia.org/wiki/C3_linearization
Classes declaring :const:`Py_TPFLAGS_MANAGED_DICT` should call | ||
:c:func:`_PyObject_VisitManagedDict` and :c:func:`_PyObject_ClearManagedDict` | ||
to traverse and clear their instance's dictionaries. | ||
To clear weakrefs, call :c:func:`PyObject_ClearWeakRefs`, as before. |
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 the extension docs be updated to provide examples of using the new API? (Probably as a separate PR...)
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.
Yes, they should. I also plan to add a new file documenting the new object layout.
Py_TPFLAGS_MANAGED_DICT
andPy_TPFLAGS_MANAGED_WEAKREF
tp_dictoffset
in inheritance. #95589