[mypyc] Fix AttributeError in async try/finally with mixed return paths (#19361)

Async functions with try/finally blocks were raising AttributeError
when:

* Some paths in the try block return while others don't
* The non-return path is executed at runtime
* No further await calls are needed

This occurred because mypyc's IR requires all control flow paths to
assign
to spill targets. The non-return path assigns NULL to maintain this
invariant, but reading NULL attributes raises AttributeError in Python.

Modified the GetAttr IR operation to support reading NULL attributes
without raising AttributeError through a new allow_null parameter. This
parameter is used specifically in try/finally resolution when reading
spill targets.

* Added allow_null: bool = False parameter to GetAttr.init in
mypyc/ir/ops.py
* When allow_null=True, sets error_kind=ERR_NEVER to prevent
AttributeError
* Modified read_nullable_attr in IRBuilder to create GetAttr with
allow_null=True
* Modified try_finally_resolve_control in statement.py to use
read_nullable_attr
  only for spill targets (attributes starting with 'mypyc_temp')
* Updated C code generation in emitfunc.py:
* visit_get_attr checks for allow_null and delegates to
get_attr_with_allow_null
* get_attr_with_allow_null reads attributes without NULL checks and only
    increments reference count if not NULL

Design decisions:

* Targeted fix: Only applied to spill targets in try/finally resolution,
not a general replacement for GetAttr. This minimizes risk and maintains
  existing behavior for all other attribute access.

* No initialization changes: Initially tried initializing spill targets
to Py_None instead of NULL, but this would incorrectly make try/finally
  blocks return None instead of falling through to subsequent code.

Added two test cases to mypyc/test-data/run-async.test:

* testAsyncTryFinallyMixedReturn: Tests the basic issue with async
  try/finally blocks containing mixed return/non-return paths.

* testAsyncWithMixedReturn: Tests async with statements (which use
  try/finally under the hood) to ensure the fix works for this common
  pattern as well.

Both tests verify that the AttributeError no longer occurs when taking
the non-return path through the try block.

See mypyc/mypyc#1115

Author: Chainfire

mypyc/codegen/emitfunc.py

@@ -358,6 +358,9 @@ def get_attr_expr(self, obj: str, op: GetAttr | SetAttr, decl_cl: ClassIR) -> st
             return f"({cast}{obj})->{self.emitter.attr(op.attr)}"
 
     def visit_get_attr(self, op: GetAttr) -> None:
+        if op.allow_null:
+            self.get_attr_with_allow_null(op)
+            return
         dest = self.reg(op)
         obj = self.reg(op.obj)
         rtype = op.class_type
@@ -426,6 +429,24 @@ def visit_get_attr(self, op: GetAttr) -> None:
             elif not always_defined:
                 self.emitter.emit_line("}")
 
+    def get_attr_with_allow_null(self, op: GetAttr) -> None:
+        """Handle GetAttr with allow_null=True which allows NULL without raising AttributeError."""
+        dest = self.reg(op)
+        obj = self.reg(op.obj)
+        rtype = op.class_type
+        cl = rtype.class_ir
+        attr_rtype, decl_cl = cl.attr_details(op.attr)
+
+        # Direct struct access without NULL check
+        attr_expr = self.get_attr_expr(obj, op, decl_cl)
+        self.emitter.emit_line(f"{dest} = {attr_expr};")
+
+        # Only emit inc_ref if not NULL
+        if attr_rtype.is_refcounted and not op.is_borrowed:
+            self.emitter.emit_line(f"if ({dest} != NULL) {{")
+            self.emitter.emit_inc_ref(dest, attr_rtype)
+            self.emitter.emit_line("}")
+
     def next_branch(self) -> Branch | None:
         if self.op_index + 1 < len(self.ops):
             next_op = self.ops[self.op_index + 1]

mypyc/ir/ops.py

@@ -777,15 +777,20 @@ class GetAttr(RegisterOp):
 
     error_kind = ERR_MAGIC
 
-    def __init__(self, obj: Value, attr: str, line: int, *, borrow: bool = False) -> None:
+    def __init__(
+        self, obj: Value, attr: str, line: int, *, borrow: bool = False, allow_null: bool = False
+    ) -> None:
         super().__init__(line)
         self.obj = obj
         self.attr = attr
+        self.allow_null = allow_null
         assert isinstance(obj.type, RInstance), "Attribute access not supported: %s" % obj.type
         self.class_type = obj.type
         attr_type = obj.type.attr_type(attr)
         self.type = attr_type
-        if attr_type.error_overlap:
+        if allow_null:
+            self.error_kind = ERR_NEVER
+        elif attr_type.error_overlap:
             self.error_kind = ERR_MAGIC_OVERLAPPING
         self.is_borrowed = borrow and attr_type.is_refcounted
 

mypyc/irbuild/builder.py

@@ -708,6 +708,15 @@ def read(
 
         assert False, "Unsupported lvalue: %r" % target
 
+    def read_nullable_attr(self, obj: Value, attr: str, line: int = -1) -> Value:
+        """Read an attribute that might be NULL without raising AttributeError.
+
+        This is used for reading spill targets in try/finally blocks where NULL
+        indicates the non-return path was taken.
+        """
+        assert isinstance(obj.type, RInstance) and obj.type.class_ir.is_ext_class
+        return self.add(GetAttr(obj, attr, line, allow_null=True))
+
     def assign(self, target: Register | AssignmentTarget, rvalue_reg: Value, line: int) -> None:
         if isinstance(target, Register):
             self.add(Assign(target, self.coerce_rvalue(rvalue_reg, target.type, line)))

mypyc/irbuild/statement.py

@@ -46,6 +46,7 @@
     YieldExpr,
     YieldFromExpr,
 )
+from mypyc.common import TEMP_ATTR_NAME
 from mypyc.ir.ops import (
     NAMESPACE_MODULE,
     NO_TRACEBACK_LINE_NO,
@@ -653,10 +654,15 @@ def try_finally_resolve_control(
     if ret_reg:
         builder.activate_block(rest)
         return_block, rest = BasicBlock(), BasicBlock()
-        builder.add(Branch(builder.read(ret_reg), rest, return_block, Branch.IS_ERROR))
+        # For spill targets in try/finally, use nullable read to avoid AttributeError
+        if isinstance(ret_reg, AssignmentTargetAttr) and ret_reg.attr.startswith(TEMP_ATTR_NAME):
+            ret_val = builder.read_nullable_attr(ret_reg.obj, ret_reg.attr, -1)
+        else:
+            ret_val = builder.read(ret_reg)
+        builder.add(Branch(ret_val, rest, return_block, Branch.IS_ERROR))
 
         builder.activate_block(return_block)
-        builder.nonlocal_control[-1].gen_return(builder, builder.read(ret_reg), -1)
+        builder.nonlocal_control[-1].gen_return(builder, ret_val, -1)
 
     # TODO: handle break/continue
     builder.activate_block(rest)