[mypyc] Record MRO ancestor modules as deps of subclass-defining modules#21743
[mypyc] Record MRO ancestor modules as deps of subclass-defining modules#21743geooo109 wants to merge 4 commits into
Conversation
Fixes python#21742 Mypy's incremental system only recompiles a module when a recorded dependency changes, and a dependency edge to an ancestor's module is created in cases such as an explicit import or a checked expression whose type references that ancestor. A bare subclass definition produces neither, so the transitive ancestors never enter the module's dependency graph. The goal of this PR is to make the dependency graph reflect what the generated C actually depends on. When compiling with mypyc, a module that defines a class now records the modules defining every ancestor in that class's MRO as indirect dependencies, so an ancestor's interface change reaches the subclass-defining module directly.
5f4468f to
c3b4e51
Compare
This comment has been minimized.
This comment has been minimized.
| def compiled_class_ancestor_refs(self) -> set[str]: | ||
| """Modules defining MRO ancestors of classes defined in this module. | ||
|
|
||
| Only used for mypyc-compiled modules: the generated C for a class | ||
| (vtable arrays, getter/setter tables, object struct) references | ||
| every inherited method/attribute of every ancestor, including | ||
| ancestors defined in modules this module does not import directly. | ||
| Recording them as indirect dependencies makes an ancestor's | ||
| interface change re-trigger type checking (and hence C regeneration) | ||
| of this module. Only top-level classes are scanned: mypyc rejects | ||
| nested class definitions. | ||
| """ | ||
| assert self.tree is not None | ||
| mods: set[str] = set() | ||
| for sym in self.tree.names.values(): | ||
| node = sym.node | ||
| if isinstance(node, TypeInfo) and node.module_name == self.id: | ||
| for ancestor in node.mro[1:]: | ||
| mods.add(ancestor.module_name) | ||
| return mods |
There was a problem hiding this comment.
could this be moved to get_additional_deps in mypyc/codegen/emitmodule.py? if the dependencies are only needed by mypyc then it would be better to keep this out of the top-level mypy logic, since mypyc functions as a plugin.
There was a problem hiding this comment.
I did a refactor. I think we can't add this inside the get_additional_deps because we want to apply it after the analysis. So, I created a new method for this. Does this approach look good, or is there a simpler solution?
| """ | ||
| return [] | ||
|
|
||
| def get_additional_indirect_deps(self, file: MypyFile) -> set[str]: |
There was a problem hiding this comment.
Is this naming ok? or We should make it more precise on which phase the method is being called.
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
Fixes #21742
Mypy's incremental system only recompiles a module when a recorded dependency changes, and a dependency edge to an ancestor's module is created in cases such as an explicit import or a checked expression whose type references that ancestor. A bare subclass definition produces neither, so the transitive ancestors never enter the module's dependency graph.
The goal of this PR is to make the dependency graph reflect what the generated C actually depends on. When compiling with mypyc, a module that defines a class now records the modules defining every ancestor in that class's MRO as indirect dependencies, so an ancestor's interface change reaches the subclass-defining module directly.