From 9f512ea368108470ffed6d1fc4fe8fe92fff88f3 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Jul 2025 16:47:38 +0100 Subject: [PATCH 1/4] [mypyc] Fix error value check for GetAttr that allows nullable values Handle tuples and other types with non-pointer representations. --- mypyc/codegen/emitfunc.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 00c7fd56b899..9b4f4ce5e9c3 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -209,6 +209,16 @@ def visit_goto(self, op: Goto) -> None: if op.label is not self.next_block: self.emit_line("goto %s;" % self.label(op.label)) + def error_value_check(self, value: Value, compare: str) -> str: + typ = value.type + if isinstance(typ, RTuple): + # TODO: What about empty tuple? + return self.emitter.tuple_undefined_check_cond( + typ, self.reg(value), self.c_error_value, compare + ) + else: + return f"{self.reg(value)} {compare} {self.c_error_value(typ)}" + def visit_branch(self, op: Branch) -> None: true, false = op.true, op.false negated = op.negated @@ -225,15 +235,8 @@ def visit_branch(self, op: Branch) -> None: expr_result = self.reg(op.value) cond = f"{neg}{expr_result}" elif op.op == Branch.IS_ERROR: - typ = op.value.type compare = "!=" if negated else "==" - if isinstance(typ, RTuple): - # TODO: What about empty tuple? - cond = self.emitter.tuple_undefined_check_cond( - typ, self.reg(op.value), self.c_error_value, compare - ) - else: - cond = f"{self.reg(op.value)} {compare} {self.c_error_value(typ)}" + cond = self.error_value_check(op.value, compare) else: assert False, "Invalid branch" @@ -443,7 +446,8 @@ def get_attr_with_allow_null(self, op: GetAttr) -> None: # 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) {{") + check = self.error_value_check(op, "!=") + self.emitter.emit_line(f"if ({check}) {{") self.emitter.emit_inc_ref(dest, attr_rtype) self.emitter.emit_line("}") From 81b4d2d567b3006c8ad6089be19b7fca35bf9f33 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Jul 2025 16:55:04 +0100 Subject: [PATCH 2/4] Add test case --- mypyc/test/test_emitfunc.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/mypyc/test/test_emitfunc.py b/mypyc/test/test_emitfunc.py index 275e8c383a4b..2de62c9558f5 100644 --- a/mypyc/test/test_emitfunc.py +++ b/mypyc/test/test_emitfunc.py @@ -111,6 +111,7 @@ def add_local(name: str, rtype: RType) -> Register: "y": int_rprimitive, "i1": int64_rprimitive, "i2": int32_rprimitive, + "t": RTuple([object_rprimitive, object_rprimitive]), } ir.bitmap_attrs = ["i1", "i2"] compute_vtable(ir) @@ -418,6 +419,17 @@ def test_get_attr_with_bitmap(self) -> None: """, ) + def test_get_attr_nullable_with_tuple(self) -> None: + self.assert_emit( + GetAttr(self.r, "t", 1, allow_null=True), + """cpy_r_r0 = ((mod___AObject *)cpy_r_r)->_t; + if (cpy_r_r0.f0 != NULL) { + CPy_INCREF(cpy_r_r0.f0); + CPy_INCREF(cpy_r_r0.f1); + } + """, + ) + def test_set_attr(self) -> None: self.assert_emit( SetAttr(self.r, "y", self.m, 1), From 4170a166d3ab20cd10eea6a962d9166d6a9d855c Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Jul 2025 16:57:59 +0100 Subject: [PATCH 3/4] Rename allow_null -> allow_error_value --- mypyc/codegen/emitfunc.py | 11 +++++++---- mypyc/ir/ops.py | 6 +++--- mypyc/irbuild/builder.py | 8 ++------ mypyc/test/test_emitfunc.py | 2 +- 4 files changed, 13 insertions(+), 14 deletions(-) diff --git a/mypyc/codegen/emitfunc.py b/mypyc/codegen/emitfunc.py index 9b4f4ce5e9c3..e81b119b24d6 100644 --- a/mypyc/codegen/emitfunc.py +++ b/mypyc/codegen/emitfunc.py @@ -361,8 +361,8 @@ 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) + if op.allow_error_value: + self.get_attr_with_allow_error_value(op) return dest = self.reg(op) obj = self.reg(op.obj) @@ -432,8 +432,11 @@ 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.""" + def get_attr_with_allow_error_value(self, op: GetAttr) -> None: + """Handle GetAttr with allow_error_value=True. + + This allows NULL or other error value without raising AttributeError. + """ dest = self.reg(op) obj = self.reg(op.obj) rtype = op.class_type diff --git a/mypyc/ir/ops.py b/mypyc/ir/ops.py index 1cb3df916ac9..08e725992aa6 100644 --- a/mypyc/ir/ops.py +++ b/mypyc/ir/ops.py @@ -811,17 +811,17 @@ class GetAttr(RegisterOp): error_kind = ERR_MAGIC def __init__( - self, obj: Value, attr: str, line: int, *, borrow: bool = False, allow_null: bool = False + self, obj: Value, attr: str, line: int, *, borrow: bool = False, allow_error_value: bool = False ) -> None: super().__init__(line) self.obj = obj self.attr = attr - self.allow_null = allow_null + self.allow_error_value = allow_error_value 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 allow_null: + if allow_error_value: self.error_kind = ERR_NEVER elif attr_type.error_overlap: self.error_kind = ERR_MAGIC_OVERLAPPING diff --git a/mypyc/irbuild/builder.py b/mypyc/irbuild/builder.py index 878c5e76df3d..323450f7c340 100644 --- a/mypyc/irbuild/builder.py +++ b/mypyc/irbuild/builder.py @@ -709,13 +709,9 @@ 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. - """ + """Read an attribute that might have an error value without raising AttributeError.""" assert isinstance(obj.type, RInstance) and obj.type.class_ir.is_ext_class - return self.add(GetAttr(obj, attr, line, allow_null=True)) + return self.add(GetAttr(obj, attr, line, allow_error_value=True)) def assign(self, target: Register | AssignmentTarget, rvalue_reg: Value, line: int) -> None: if isinstance(target, Register): diff --git a/mypyc/test/test_emitfunc.py b/mypyc/test/test_emitfunc.py index 2de62c9558f5..0a696e9e3380 100644 --- a/mypyc/test/test_emitfunc.py +++ b/mypyc/test/test_emitfunc.py @@ -421,7 +421,7 @@ def test_get_attr_with_bitmap(self) -> None: def test_get_attr_nullable_with_tuple(self) -> None: self.assert_emit( - GetAttr(self.r, "t", 1, allow_null=True), + GetAttr(self.r, "t", 1, allow_error_value=True), """cpy_r_r0 = ((mod___AObject *)cpy_r_r)->_t; if (cpy_r_r0.f0 != NULL) { CPy_INCREF(cpy_r_r0.f0); From 566cc1fc051d55cb0db12ad05062a602bb4ad260 Mon Sep 17 00:00:00 2001 From: Jukka Lehtosalo Date: Fri, 4 Jul 2025 16:58:43 +0100 Subject: [PATCH 4/4] Format --- mypyc/ir/ops.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/mypyc/ir/ops.py b/mypyc/ir/ops.py index 08e725992aa6..668e3097ce30 100644 --- a/mypyc/ir/ops.py +++ b/mypyc/ir/ops.py @@ -811,7 +811,13 @@ class GetAttr(RegisterOp): error_kind = ERR_MAGIC def __init__( - self, obj: Value, attr: str, line: int, *, borrow: bool = False, allow_error_value: bool = False + self, + obj: Value, + attr: str, + line: int, + *, + borrow: bool = False, + allow_error_value: bool = False, ) -> None: super().__init__(line) self.obj = obj