[mypyc] Fix allow_interpreted_subclasses not seeing subclass attrs#21013
[mypyc] Fix allow_interpreted_subclasses not seeing subclass attrs#21013VaggelisD wants to merge 2 commits intopython:masterfrom
Conversation
…e overrides When a compiled class with allow_interpreted_subclasses=True has methods that access self.ATTR via direct C struct slots, interpreted subclasses that override ATTR in their class __dict__ are ignored — the compiled method always reads the base class default from the slot. Fix: in visit_get_attr for non-property attribute access, check if the instance is a mypyc-compiled type (via a new CPy_TPFLAGS_MYPYC_COMPILED tp_flags bit). If not, fall back to PyObject_GenericGetAttr which respects the MRO and finds the subclass override. Using tp_flags rather than an exact type check ensures compiled subclasses retain fast direct struct access, while only interpreted subclasses hit the GenericGetAttr slow path. For unboxed types (bool, int), the PyObject* result is unboxed to the expected C type.
for more information, see https://pre-commit.ci
| # Test that compiled subclasses of a class with allow_interpreted_subclasses=True | ||
| # can correctly access parent instance attributes via direct struct access | ||
| # (not falling back to PyObject_GenericGetAttr). |
There was a problem hiding this comment.
the run test doesn't prove that it's using direct struct access because it would work either way, just one way is slower. i don't think there's a way to test this well so maybe just reword the comment.
i guess one way to confirm it's still using the fast path is to benchmark the accesses on master and with this change. could you do that and add the results to the description?
| // Flag bit set on all mypyc-compiled types. Used to distinguish compiled | ||
| // subclasses (safe for direct struct access) from interpreted subclasses | ||
| // (need PyObject_GenericGetAttr fallback) in allow_interpreted_subclasses mode. | ||
| #define CPy_TPFLAGS_MYPYC_COMPILED (1UL << 20) |
There was a problem hiding this comment.
this bit is already used by cpython https://github.com/python/cpython/blob/main/Include/object.h#L611. from what i saw the flag is only used by object_new which isn't used by mypyc-compiled types but i think it would be better to pick a bit that's not used at all in case there's compatibility issues later.
When a compiled class with
allow_interpreted_subclasses=Truehas methods that accessself.ATTRvia direct C struct slots, interpreted subclasses that overrideATTRin their class__dict__are ignored; the compiled method always reads the base class default from the slot.The fix: In
visit_get_attrfor non-property attribute access, check if the instance is a mypyc-compiled type (via a newCPy_TPFLAGS_MYPYC_COMPILEDtp_flagsbit). If not, fall back toPyObject_GenericGetAttrwhich respects the MRO and finds the subclass override.Using
tp_flagsrather than an exact type check ensures compiled subclasses retain fast direct struct access, while only interpreted subclasses hit theGenericGetAttrslow path.For unboxed types (bool, int), the
PyObject*result is unboxed to the expected C type.