PR feedback 1 (p-sawicki)

Author: geooo109

mypy/build.py

@@ -118,7 +118,6 @@
     MypyFile,
     OverloadedFuncDef,
     SymbolTable,
-    TypeInfo,
 )
 from mypy.options import OPTIONS_AFFECTING_CACHE_NO_PLATFORM
 from mypy.partially_defined import PossiblyUndefinedVariableVisitor
@@ -3480,20 +3479,21 @@ def finish_passes(self) -> None:
             if options.export_types:
                 manager.all_types.update(self.type_map())
 
-            # Possible sources of indirect dependencies:
-            # * Symbols not directly imported in this module but accessed via an attribute
-            #   or via a re-export (vast majority of these recorded in semantic analysis).
-            # * For each expression type we need to record definitions of type components
-            #   since "meaning" of the type may be updated when definitions are updated.
-            # * For mypyc-compiled modules only: modules defining MRO ancestors of
-            #   classes defined here, since the generated C embeds each ancestor's
-            #   method/attribute layout.
-            indirect_refs = self.tree.module_refs | self.type_checker().module_refs
-            if self.options.mypyc:
-                indirect_refs |= self.compiled_class_ancestor_refs()
             # We should always patch indirect dependencies, even in full (non-incremental) builds,
             # because the cache still may be written, and it must be correct.
-            self.patch_indirect_dependencies(indirect_refs, set(self.type_map().values()))
+            self.patch_indirect_dependencies(
+                # Three possible sources of indirect dependencies:
+                # * Symbols not directly imported in this module but accessed via an attribute
+                #   or via a re-export (vast majority of these recorded in semantic analysis).
+                # * For each expression type we need to record definitions of type components
+                #   since "meaning" of the type may be updated when definitions are updated.
+                # * Additional dependencies reported by plugins (e.g. mypyc, see
+                #   MypycPlugin.get_additional_indirect_deps).
+                self.tree.module_refs
+                | self.type_checker().module_refs
+                | manager.plugin.get_additional_indirect_deps(self.tree),
+                set(self.type_map().values()),
+            )
 
             if self.options.dump_inference_stats:
                 dump_type_stats(
@@ -3535,27 +3535,6 @@ def patch_indirect_dependencies(self, module_refs: set[str], types: set[Type]) -
             self.add_dependency(dep)
             self.priorities[dep] = PRI_INDIRECT
 
-    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
-
     def compute_fine_grained_deps(self) -> dict[str, set[str]]:
         assert self.tree is not None
         if self.id in ("builtins", "typing", "types", "sys", "_typeshed"):

mypy/plugin.py

@@ -587,6 +587,17 @@ def get_additional_deps(self, file: MypyFile) -> list[tuple[int, str, int]]:
         """
         return []
 
+    def get_additional_indirect_deps(self, file: MypyFile) -> set[str]:
+        """Customize indirect dependencies for a module.
+
+        Unlike get_additional_deps(), this hook is called after the module
+        has been type checked, so analyzed information (such as class MROs)
+        is available. The returned module names are recorded as indirect
+        dependencies: a change to their interfaces will invalidate this
+        module's cache, but they are not treated as imports.
+        """
+        return set()
+
     def get_type_analyze_hook(self, fullname: str) -> Callable[[AnalyzeTypeContext], Type] | None:
         """Customize behaviour of the type analyzer for given full names.
 
@@ -846,6 +857,12 @@ def get_additional_deps(self, file: MypyFile) -> list[tuple[int, str, int]]:
             deps.extend(plugin.get_additional_deps(file))
         return deps
 
+    def get_additional_indirect_deps(self, file: MypyFile) -> set[str]:
+        deps: set[str] = set()
+        for plugin in self._plugins:
+            deps |= plugin.get_additional_indirect_deps(file)
+        return deps
+
     def get_type_analyze_hook(self, fullname: str) -> Callable[[AnalyzeTypeContext], Type] | None:
         # Micro-optimization: Inline iteration over plugins
         for plugin in self._plugins:

mypyc/codegen/emitmodule.py

@@ -23,7 +23,7 @@
 )
 from mypy.errors import CompileError
 from mypy.fscache import FileSystemCache
-from mypy.nodes import MypyFile
+from mypy.nodes import MypyFile, TypeInfo
 from mypy.options import Options
 from mypy.plugin import Plugin, ReportConfigContext
 from mypy.util import hash_digest, json_dumps
@@ -200,6 +200,28 @@ def get_additional_deps(self, file: MypyFile) -> list[tuple[int, str, int]]:
         # Report dependency on modules in the module's group
         return [(10, id, -1) for id in self.group_map.get(file.fullname, (None, []))[1]]
 
+    def get_additional_indirect_deps(self, file: MypyFile) -> set[str]:
+        """Modules defining MRO ancestors of classes defined in this module.
+
+        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.
+        """
+        if file.fullname not in self.group_map:
+            return set()
+
+        mods: set[str] = set()
+        for sym in file.names.values():
+            node = sym.node
+            if isinstance(node, TypeInfo) and node.module_name == file.fullname:
+                for ancestor in node.mro[1:]:
+                    mods.add(ancestor.module_name)
+        return mods
+
 
 def parse_and_typecheck(
     sources: list[BuildSource],

mypyc/test-data/run-multimodule.test

@@ -1901,18 +1901,8 @@ from native import test
 test()
 
 [case testIncrementalCrossGroupInheritedMethodRemoved]
-# Regression: under separate=True, a module whose only content is a subclass
-# definition (`class Leaf(Mid): pass`) produces no expression types, so it
-# recorded no dependency on the modules defining its transitive bases. When a
-# Base method two hops up the inheritance chain was removed, staleness
-# propagation stopped at other_mid (its own interface is unchanged) and
-# other_leaf stayed fresh: its stale generated C still referenced the method
-# in Leaf's vtable via exports_other_base.CPyDef_..., which no longer exists
-# in the regenerated export table, and the build failed.
-# compiled_class_ancestor_refs must record the MRO ancestors' modules as
-# indirect deps so an ancestor's interface change recompiles the defining
-# module too. (removed_method is declared after existing_method so the
-# surviving call site's vtable slot is unaffected.)
+# Regression: updating a transitive base class triggers recompilation of the
+# subclass-defining module (mypy#21742).
 import other_leaf
 
 def test() -> int:
@@ -1955,12 +1945,8 @@ print(test())
 6
 
 [case testIncrementalCrossGroupInheritedMethodRemovedNonEmptyLeaf]
-# Same regression as testIncrementalCrossGroupInheritedMethodRemoved, but the
-# subclass body is NOT empty: a class constant and a method that never uses
-# self. Neither produces an expression whose type reaches Leaf's MRO, so the
-# pre-existing dependency sources still record nothing for other_leaf, while
-# its generated vtable references the ancestors regardless of body shape.
-# Guards against narrowing compiled_class_ancestor_refs to pass-only bodies.
+# Regression: updating a transitive base class triggers recompilation of the
+# module defining a subclass with a non-empty body (mypy#21742).
 import other_leaf
 
 def test() -> int: