Skip to content

GH-95589: Dont crash when subclassing extension classes with multiple inheritance#96028

Merged
markshannon merged 7 commits into
python:mainfrom
faster-cpython:dont-crash-multiple-inheritance
Aug 17, 2022
Merged

GH-95589: Dont crash when subclassing extension classes with multiple inheritance#96028
markshannon merged 7 commits into
python:mainfrom
faster-cpython:dont-crash-multiple-inheritance

Conversation

@markshannon

@markshannon markshannon commented Aug 16, 2022

Copy link
Copy Markdown
Member
  • Fixes crashes.
  • Documents change in behavior and recommends use of Py_TPFLAGS_MANAGED_DICT and Py_TPFLAGS_MANAGED_WEAKREF

Comment thread 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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Copy Markdown
Member Author

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.

Comment thread Lib/test/test_capi.py
pass
class B2(Both1, cls):
class B2(C4, cls):
pass

Copy link
Copy Markdown
Contributor

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?

Copy link
Copy Markdown
Member Author

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

Comment thread Doc/whatsnew/3.12.rst
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.

Copy link
Copy Markdown
Contributor

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...)

Copy link
Copy Markdown
Member Author

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.

@markshannon markshannon merged commit b73e3b6 into python:main Aug 17, 2022
@markshannon markshannon deleted the dont-crash-multiple-inheritance branch September 26, 2023 12:56
wsfulton added a commit to swig/swig that referenced this pull request Jun 11, 2025
Fix when using -builtin and -DSWIG_HEAPTYPES in these tests:

constructor_copy_non_const
contract
default_constructor
director_basic
minherit
using_composition
using_extend
using_member_multiple_inherit
cpp11_variadic_templates

Fix required for python <= 3.11.
Python 3.12 does not have the TypeError due to
python/cpython#96028.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants