Repository navigation
[mypyc] Regenerate dependent modules when weakrefs, serializable or acyclic is toggled on a base class - #22467
Open
Jahongir-Qurbonov wants to merge 2 commits into
Conversation
…se class @mypyc_attr(weakrefs=True) (added in python#22387) adds a weak reference list field to the object struct of the class, which moves the attributes of all native subclasses. With separate compilation, toggling it on a base class didn't regenerate subclasses in other groups, since mypy doesn't record class decorators and the interface hash of the base module stayed the same. The subclass kept its stale layout, and accessing its attributes or creating a weak reference could segfault. Include the weakrefs flag in class_kinds(), which MypycPlugin reports as part of a module's interface. When writing the cache it's derived from the tree like prepare_class_def sets ClassIR._weakrefs, and when checking the cache it's read from the cached IR.
…toggled Like weakrefs, the serializable and acyclic flags of `@mypyc_attr` affect the generated code of subclasses in other modules, but mypy ignores the decorator, so toggling them didn't change the interface of the class's module. With separate compilation, the subclasses were then left stale: * `serializable` is inherited, and the tp_new of a serializable class doesn't call `__init__`. After making a base class serializable, a stale subclass still called `__init__` without arguments from tp_new, so `copy.copy()` and `pickle` failed with a `TypeError` about missing arguments to `__init__`. * A class can only be acyclic if all its base classes are. After making a base class non-acyclic, the error for an acyclic subclass wasn't reported. CPython then gives the subclass the GC flag and tp_traverse of the base, but the subclass's tp_dealloc doesn't untrack instances, so a collection triggered while an instance was being freed (e.g. from a `__del__` of one of its attributes) crashed. Add both flags to the class kinds that `MypycPlugin.report_config_data` makes part of a module's interface, using the `_serializable` and `is_acyclic` fields of the cached IR on the check path.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
@mypyc_attr(weakrefs=True)(added in #22387) adds a weak reference list field to the object struct of the class, right after the vtable pointer, so the attributes of every native subclass move andtp_basicsizegrows. With separate compilation, toggling it on a base class didn't regenerate subclasses in other groups. Mypy doesn't record class decorators, so the base module's interface hash didn't change, and the subclass module kept its old C code with the old layout.Example, compiled with
mypycify(["base_mod.py", "sub_mod.py"], separate=True):After the second build
B.__basicsize__was 56 instead of 64, and accessing attributes of aBinstance and creating a weak reference to it segfaulted. Expected:sub_modis recompiled and everything works.The kinds of a module's classes are already part of its interface through
MypycPlugin.report_config_data, fornative_class=Falseand@trait(#22136). This adds theweakrefsflag to the tupleclass_kinds()returns. When the cache is written it's derived from the tree the same wayprepare_class_defsetsClassIR._weakrefs(only for native classes that aren't traits), and when the cache is checked it comes from_weakrefsin the cached IR, so the two agree when nothing changed.I added
testIncrementalCompilationTogglingWeakrefsto run-multimodule.test, following the existing tests fornative_classand@trait: it turnsweakrefson and off on a base class inother.py, and in the middle step it changes onlynative.pyto check thatotherstays fresh. Without the fix, the separate compilation variant crashes in step 2.The same gap existed for two more
@mypyc_attrflags, fixed in a second commit.serializableis inherited, and a serializable class'stp_newdoesn't call__init__. After making a base class serializable, a stale subclass in another group still called__init__without arguments, socopy.copy()/pickleof the subclass failed with aTypeErrorabout missing arguments.acyclicis only allowed when all base classes are acyclic. After making a base class non-acyclic, the error for an acyclic subclass wasn't reported. CPython then made the subclass a GC type with the base'stp_traverse, but itstp_deallocdoesn't untrack instances, which can segfault if a collection runs while an instance is freed. Both flags are now part of the class kinds (_serializableandis_acyclicon the cached-IR path), with a new run-multimodule test for each.allow_interpreted_subclassesdoesn't need this: a subclass's generated code doesn't depend on its base having the flag. The only stale case is a base losing it while a subclass still uses it, and then the build fails with a C compile error instead of mypyc's error message.