[mypyc] Preserve inherited attribute defaults under separate=True#21547
[mypyc] Preserve inherited attribute defaults under separate=True#21547georgesittas wants to merge 1 commit into
Conversation
Under `separate=True`, when a subclass is recompiled while its parent
is loaded from mypy's incremental cache, parent default-attribute
assignments are silently dropped from the subclass's
`__mypyc_defaults_setup`. The first read of an inherited default-attr
then raises:
AttributeError: attribute '<name>' of '<Parent>' undefined
`find_attr_initializers` walks `cdef.info.mro` and reads
`info.defn.defs.body` for `AssignmentStmt`s. `ClassDef.serialize`
(mypy/nodes.py) does not serialize `defs`, so a cache-loaded parent
has `defs = Block([])`; the MRO walk collects no parent assignments
and the subclass's emitted setup leaves inherited slots in the
undefined-sentinel state.
Fix: under `separate=True`, scope `find_attr_initializers` to the
subclass's own body and have `generate_attr_defaults_init` emit a
chained call to the nearest ancestor with `__mypyc_defaults_setup`
before setting own defaults. Each class's setup is responsible only
for its own attributes; the chain runs ancestors first, mirroring
the `__init__` chain. The ancestor's return value is propagated so
a parent default that raised still aborts instance creation. Without
`separate=True`, the MRO AST walk is unaffected (all modules parsed
in the same pass), preserving the existing inline-all behavior.
Adds `testIncrementalCrossModuleInheritedAttrDefaultsWithOverride`
to `run-multimodule.test`, which triggers the cache-load path: clean
build of a two-module Parent/Child hierarchy, then a `.2` revision of
the subclass module to force an incremental rebuild while the parent
is served from the cache. The child overrides one inherited default
so it emits its own `__mypyc_defaults_setup` (the case the chain
composition is required for). The test fails on master without this
patch.
| if "__mypyc_defaults_setup" in ancestor.method_decls: | ||
| parent_with_defaults = ancestor | ||
| break |
There was a problem hiding this comment.
this makes the compilation results dependent on the order in which files are compiled, because __mypyc_defaults_setup will only be present in method_decls after it has been added when this function is called for the base class. so if the subclass is compiled before the base class then it won't find it. assuming the base and subclasses are defined in different files like in the test.
in the test this is not hit because the three different files are each in their own compilation groups, and the dependency analysis finds that they are dependent on each other so the compilation order is always the same: other_b.py (base class) -> other_a.py (subclass) -> native.py (instantiation).
but if we put other_b.py and other_a.py in the same compilation group with this comment:
[case testIncrementalCrossModuleInheritedAttrDefaultsWithOverride]
# separate: [(["native.py"], "grp1"), (["other_a.py", "other_b.py"], "grp2")]
...
then the compilation order changes because the dependency analysis treats all files in a given group as dependent on each other, and the compilation order within a group is determined by filenames. so other_a.py (subclass) will be compiled before other_b.py (base class) because its name comes first when sorting.
to make the compilation independent on the order, we have a preparation phase in prepare.py that runs before the IR build and sets up attributes that might be needed when compiling across different files.
so i think we should move the analysis of whether __mypyc_defaults_setup needs to be generated to prepare.py, and add the method declaration there if so. then here we only generate the body and we can safely make this check.
There was a problem hiding this comment.
I see, this is very helpful, appreciate the detailed explanation and it makes sense. Let me give your suggestion a whirl. I'll make sure to include a regression test covering this path as well.
2 participants
Fixes #21542
Under
separate=True, when a subclass is recompiled while its parent is loaded from mypy's incremental cache, parent default-attribute assignments are silently dropped from the subclass's__mypyc_defaults_setup. The first read of an inherited default-attr then raises:find_attr_initializerswalkscdef.info.mroand readsinfo.defn.defs.bodyforAssignmentStmts.ClassDef.serialize(mypy/nodes.py) does not serializedefs, so a cache-loaded parent hasdefs = Block([]); the MRO walk collects no parent assignments and the subclass's emitted setup leaves inherited slots in the undefined-sentinel state.This PR implements the fix discussed in the linked issue.