[mypyc] Preserve inherited attribute defaults under separate=True (#21547)

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:

```
    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.

This PR implements the fix discussed in the linked issue.

Author: georgesittas

mypyc/codegen/emitclass.py

@@ -29,6 +29,7 @@
     BITMAP_BITS,
     BITMAP_TYPE,
     CPYFUNCTION_NAME,
+    MYPYC_DEFAULTS_SETUP,
     NATIVE_PREFIX,
     PREFIX,
     REG_PREFIX,
@@ -285,7 +286,7 @@ def emit_line() -> None:
 
     # If the class has a method to initialize default attribute
     # values, we need to call it during initialization.
-    defaults_fn = cl.get_method("__mypyc_defaults_setup")
+    defaults_fn = cl.get_method(MYPYC_DEFAULTS_SETUP)
 
     # If there is a __init__ method, we'll use it in the native constructor.
     init_fn = cl.get_method("__init__")

mypyc/common.py

@@ -24,6 +24,7 @@
 LAMBDA_NAME: Final = "__mypyc_lambda__"
 PROPSET_PREFIX: Final = "__mypyc_setter__"
 SELF_NAME: Final = "__mypyc_self__"
+MYPYC_DEFAULTS_SETUP: Final = "__mypyc_defaults_setup"
 GENERATOR_ATTRIBUTE_PREFIX: Final = "__mypyc_generator_attribute__"
 CPYFUNCTION_NAME = "__cpyfunction__"
 

mypyc/irbuild/classdef.py

@@ -7,6 +7,7 @@
 from typing import Final
 
 from mypy.nodes import (
+    ARG_POS,
     EXCLUDED_ENUM_ATTRIBUTES,
     TYPE_VAR_TUPLE_KIND,
     AssignmentStmt,
@@ -21,15 +22,14 @@
     NameExpr,
     OverloadedFuncDef,
     PassStmt,
-    RefExpr,
     StrExpr,
     TempNode,
     TypeInfo,
     TypeParam,
     is_class_var,
 )
 from mypy.types import Instance, UnboundType, get_proper_type
-from mypyc.common import PROPSET_PREFIX
+from mypyc.common import MYPYC_DEFAULTS_SETUP, PROPSET_PREFIX
 from mypyc.ir.class_ir import ClassIR, NonExtClassInfo
 from mypyc.ir.func_ir import FuncDecl, FuncSignature
 from mypyc.ir.ops import (
@@ -48,15 +48,7 @@
     TupleSet,
     Value,
 )
-from mypyc.ir.rtypes import (
-    RType,
-    bool_rprimitive,
-    dict_rprimitive,
-    is_none_rprimitive,
-    is_object_rprimitive,
-    is_optional_type,
-    object_rprimitive,
-)
+from mypyc.ir.rtypes import RType, bool_rprimitive, dict_rprimitive, object_rprimitive
 from mypyc.irbuild.builder import IRBuilder, create_type_params
 from mypyc.irbuild.function import (
     gen_property_getter_ir,
@@ -66,7 +58,13 @@
     load_type,
 )
 from mypyc.irbuild.prepare import GENERATOR_HELPER_NAME
-from mypyc.irbuild.util import dataclass_type, get_func_def, is_constant, is_dataclass_decorator
+from mypyc.irbuild.util import (
+    dataclass_type,
+    default_attr_name,
+    get_func_def,
+    is_constant,
+    is_dataclass_decorator,
+)
 from mypyc.primitives.dict_ops import dict_new_op, exact_dict_set_item_op
 from mypyc.primitives.generic_ops import (
     iter_op,
@@ -322,10 +320,6 @@ def __init__(self, builder: IRBuilder, cdef: ClassDef) -> None:
     def class_body_obj(self) -> Value | None:
         return self.type_obj
 
-    def skip_attr_default(self, name: str, stmt: AssignmentStmt) -> bool:
-        """Controls whether to skip generating a default for an attribute."""
-        return False
-
     def add_method(self, fdef: FuncDef) -> None:
         handle_ext_method(self.builder, self.cdef, fdef)
 
@@ -348,11 +342,18 @@ def finalize(self, ir: ClassIR) -> None:
         # Call __init_subclass__ after class attributes have been set
         self.builder.call_c(py_init_subclass_op, [self.type_obj], self.cdef.line)
 
-        attrs_with_defaults, default_assignments = find_attr_initializers(
-            self.builder, self.cdef, self.skip_attr_default
-        )
-        ir.attrs_with_defaults.update(attrs_with_defaults)
-        generate_attr_defaults_init(self.builder, self.cdef, default_assignments)
+        # Under separate compilation, prepare.py pre-registers the decl iff
+        # the class has its own default attribute assignments to emit, so we
+        # can skip the body walk entirely when it isn't present. Without
+        # separate compilation, find_attr_initializers walks the MRO so that
+        # inherited defaults are reflected in ir.attrs_with_defaults (relied
+        # on by the attribute-definedness analysis), so we always run it.
+        if not self.builder.options.separate or MYPYC_DEFAULTS_SETUP in ir.method_decls:
+            attrs_with_defaults, default_assignments = find_attr_initializers(
+                self.builder, self.cdef
+            )
+            ir.attrs_with_defaults.update(attrs_with_defaults)
+            generate_attr_defaults_init(self.builder, self.cdef, default_assignments)
         create_ne_from_eq(self.builder, self.cdef)
 
 
@@ -380,9 +381,6 @@ def create_non_ext_info(self) -> NonExtClassInfo:
             self.builder.add(LoadAddress(type_object_op.type, type_object_op.src, self.cdef.line)),
         )
 
-    def skip_attr_default(self, name: str, stmt: AssignmentStmt) -> bool:
-        return stmt.type is not None
-
     def get_type_annotation(self, stmt: AssignmentStmt) -> TypeInfo | None:
         # We populate __annotations__ because dataclasses uses it to determine
         # which attributes to compute on.
@@ -445,9 +443,6 @@ class AttrsClassBuilder(DataClassBuilder):
 
     add_annotations_to_dict = False
 
-    def skip_attr_default(self, name: str, stmt: AssignmentStmt) -> bool:
-        return True
-
     def get_type_annotation(self, stmt: AssignmentStmt) -> TypeInfo | None:
         if isinstance(stmt.rvalue, CallExpr):
             # find the type arg in `attr.ib(type=str)`
@@ -741,74 +736,100 @@ def add_non_ext_class_attr(
 
 
 def find_attr_initializers(
-    builder: IRBuilder, cdef: ClassDef, skip: Callable[[str, AssignmentStmt], bool] | None = None
+    builder: IRBuilder, cdef: ClassDef
 ) -> tuple[set[str], list[tuple[AssignmentStmt, str]]]:
     """Find initializers of attributes in a class body.
 
-    If provided, the skip arg should be a callable which will return whether
-    to skip generating a default for an attribute.  It will be passed the name of
-    the attribute and the corresponding AssignmentStmt.
+    Under separate compilation, only this class's own body is walked, and
+    generate_attr_defaults_init emits a runtime call to the parent's
+    __mypyc_defaults_setup so inherited defaults are produced by chaining,
+    not by inlining. Walking the MRO here would break under separate=True
+    with mypy's incremental cache: a base class loaded from the cache has
+    an empty ClassDef.defs.body (mypy/nodes.py::ClassDef.serialize doesn't
+    serialize the class body), so inherited assignments would be silently
+    dropped and the subclass's __mypyc_defaults_setup would leave inherited
+    slots in the "undefined" state at runtime.
+
+    Without separate compilation, all modules are parsed in the same pass
+    and the MRO walk is safe; we keep the original inline-all behavior
+    there as an optimization (no chain call needed for instance creation).
     """
     cls = builder.mapper.type_to_ir[cdef.info]
     if cls.builtin_base:
         return set(), []
 
-    attrs_with_defaults = set()
+    cls_type = dataclass_type(cdef)
+    attrs_with_defaults: set[str] = set()
+    default_assignments: list[tuple[AssignmentStmt, str]] = []
 
-    # Pull out all assignments in classes in the mro so we can initialize them
     # TODO: Support nested statements
-    default_assignments: list[tuple[AssignmentStmt, str]] = []
-    for info in reversed(cdef.info.mro):
-        if info not in builder.mapper.type_to_ir:
+    if builder.options.separate:
+        infos: list[TypeInfo] = [cdef.info]
+    else:
+        infos = list(reversed(cdef.info.mro))
+
+    for info in infos:
+        info_ir = builder.mapper.type_to_ir.get(info)
+        if info_ir is None:
             continue
         for stmt in info.defn.defs.body:
-            if (
-                isinstance(stmt, AssignmentStmt)
-                and isinstance(stmt.lvalues[0], NameExpr)
-                and not is_class_var(stmt.lvalues[0])
-                and not isinstance(stmt.rvalue, TempNode)
-            ):
-                name = stmt.lvalues[0].name
-                if name == "__slots__":
-                    continue
-
-                if name == "__deletable__":
-                    check_deletable_declaration(builder, cls, stmt.line)
-                    continue
-
-                if skip is not None and skip(name, stmt):
-                    continue
-
-                attr_type = cls.attr_type(name)
-
-                # If the attribute is initialized to None and type isn't optional,
-                # doesn't initialize it to anything (special case for "# type:" comments).
-                if isinstance(stmt.rvalue, RefExpr) and stmt.rvalue.fullname == "builtins.None":
-                    if (
-                        not is_optional_type(attr_type)
-                        and not is_object_rprimitive(attr_type)
-                        and not is_none_rprimitive(attr_type)
-                    ):
-                        continue
-
-                attrs_with_defaults.add(name)
-                default_assignments.append((stmt, info.module_name))
+            if not isinstance(stmt, AssignmentStmt):
+                continue
+            name = default_attr_name(stmt, info_ir, cls_type)
+            if name is None:
+                continue
+            attrs_with_defaults.add(name)
+            default_assignments.append((stmt, info.module_name))
 
     return attrs_with_defaults, default_assignments
 
 
 def generate_attr_defaults_init(
     builder: IRBuilder, cdef: ClassDef, default_assignments: list[tuple[AssignmentStmt, str]]
 ) -> None:
-    """Generate an initialization method for default attr values (from class vars)."""
-    if not default_assignments:
-        return
+    """Generate an initialization method for default attr values (from class vars).
+
+    Under separate compilation, the emitted __mypyc_defaults_setup chains to
+    the nearest ancestor that has the method (Python __init__ style), then
+    sets only this class's own defaults; inherited defaults are produced by
+    the chain at runtime. The ancestor lookup uses cls.mro[1:] and relies on
+    prepare.py having registered the FuncDecl on every class that needs one
+    before any IR build runs. IR build within a compilation group proceeds
+    in filename order, so this class may be IR-built before its base, and a
+    method_decls lookup that depended on the base having been IR-built first
+    would miss. Without separate compilation, find_attr_initializers has
+    already collected the full MRO's defaults into default_assignments, so
+    we inline them all as before.
+    """
     cls = builder.mapper.type_to_ir[cdef.info]
     if cls.builtin_base:
         return
 
-    with builder.enter_method(cls, "__mypyc_defaults_setup", bool_rprimitive):
+    parent_with_defaults: ClassIR | None = None
+    if builder.options.separate:
+        for ancestor in cls.mro[1:]:
+            if MYPYC_DEFAULTS_SETUP in ancestor.method_decls:
+                parent_with_defaults = ancestor
+                break
+
+    if not default_assignments and parent_with_defaults is None:
+        return
+
+    with builder.enter_method(cls, MYPYC_DEFAULTS_SETUP, bool_rprimitive):
         self_var = builder.self()
+
+        # Chain to parent's setup so inherited defaults run first; propagate
+        # its False return so a parent default that raised still aborts
+        # instance creation rather than being silently swallowed here.
+        if parent_with_defaults is not None:
+            decl = parent_with_defaults.method_decl(MYPYC_DEFAULTS_SETUP)
+            parent_ok = builder.builder.call(decl, [self_var], [ARG_POS], [None], cdef.line)
+            fail_block, continue_block = BasicBlock(), BasicBlock()
+            builder.add(Branch(parent_ok, continue_block, fail_block, Branch.BOOL))
+            builder.activate_block(fail_block)
+            builder.add(Return(builder.false()))
+            builder.activate_block(continue_block)
+
         for stmt, origin_module in default_assignments:
             lvalue = stmt.lvalues[0]
             assert isinstance(lvalue, NameExpr), lvalue
@@ -833,26 +854,6 @@ def generate_attr_defaults_init(
         builder.add(Return(builder.true()))
 
 
-def check_deletable_declaration(builder: IRBuilder, cl: ClassIR, line: int) -> None:
-    for attr in cl.deletable:
-        if attr not in cl.attributes:
-            if not cl.has_attr(attr):
-                builder.error(f'Attribute "{attr}" not defined', line)
-                continue
-            for base in cl.mro:
-                if attr in base.property_types:
-                    builder.error(f'Cannot make property "{attr}" deletable', line)
-                    break
-            else:
-                _, base = cl.attr_details(attr)
-                builder.error(
-                    ('Attribute "{}" not defined in "{}" ' + '(defined in "{}")').format(
-                        attr, cl.name, base.name
-                    ),
-                    line,
-                )
-
-
 def create_ne_from_eq(builder: IRBuilder, cdef: ClassDef) -> None:
     """Create a "__ne__" method from a "__eq__" method (if only latter exists)."""
     cls = builder.mapper.type_to_ir[cdef.info]