Address review comments

Author: p-sawicki

mypyc/irbuild/expression.py

@@ -481,17 +481,17 @@ def translate_super_method_call(builder: IRBuilder, expr: CallExpr, callee: Supe
             result = translate_object_new(builder, expr, MemberExpr(callee.call, "__new__"))
             if result:
                 return result
+        elif callee.name == "__setattr__" and ir.builtin_base is None and not ir.inherits_python:
+            result = translate_object_setattr(
+                builder, expr, MemberExpr(callee.call, "__setattr__")
+            )
+            if result:
+                return result
         if ir.is_ext_class and ir.builtin_base is None and not ir.inherits_python:
             if callee.name == "__init__" and len(expr.args) == 0:
                 # Call translates to object.__init__(self), which is a
                 # no-op, so omit the call.
                 return builder.none()
-            elif callee.name == "__setattr__":
-                result = translate_object_setattr(
-                    builder, expr, MemberExpr(callee.call, "__setattr__")
-                )
-                if result:
-                    return result
         return translate_call(builder, expr, callee)
 
     decl = base.method_decl(callee.name)

mypyc/irbuild/specialize.py

@@ -1010,9 +1010,7 @@ def translate_ord(builder: IRBuilder, expr: CallExpr, callee: RefExpr) -> Value
 
 
 def is_object(callee: RefExpr) -> bool:
-    """
-    Returns True for object.<name> calls
-    """
+    """Returns True for object.<name> calls."""
     return (
         isinstance(callee, MemberExpr)
         and isinstance(callee.expr, NameExpr)
@@ -1021,9 +1019,7 @@ def is_object(callee: RefExpr) -> bool:
 
 
 def is_super_or_object(expr: CallExpr, callee: RefExpr) -> bool:
-    """
-    Returns True for super().<name> or object.<name> calls.
-    """
+    """Returns True for super().<name> or object.<name> calls."""
     return isinstance(expr.callee, SuperExpr) or is_object(callee)
 
 

mypyc/test-data/irbuild-classes.test

@@ -2373,6 +2373,8 @@ L0:
     return 1
 
 [case testUntransformedSetAttr_64bit]
+from mypy_extensions import mypyc_attr
+
 class SetAttr:
     def super_missing_args(self):
         super().__setattr__()
@@ -2383,6 +2385,18 @@ class SetAttr:
         object.__setattr__(self)
         object.__setattr__(self, "attr")
 
+@mypyc_attr(native_class=False)
+class NonNative:
+    pass
+
+class InheritsPython(NonNative):
+    def super_setattr(self, key: str, val: object) -> None:
+        super().__setattr__(key, val)
+
+class BuiltInBase(dict):
+    def super_setattr(self, key: str, val: object) -> None:
+        super().__setattr__(key, val)
+
 [typing fixtures/typing-full.pyi]
 [out]
 def SetAttr.super_missing_args(self):
@@ -2484,3 +2498,63 @@ L0:
     keep_alive r16, self, r17
     r22 = box(None, 1)
     return r22
+def InheritsPython.super_setattr(self, key, val):
+    self :: __main__.InheritsPython
+    key :: str
+    val, r0 :: object
+    r1 :: str
+    r2, r3 :: object
+    r4 :: object[2]
+    r5 :: object_ptr
+    r6 :: object
+    r7 :: str
+    r8 :: object
+    r9 :: object[2]
+    r10 :: object_ptr
+    r11 :: object
+L0:
+    r0 = builtins :: module
+    r1 = 'super'
+    r2 = CPyObject_GetAttr(r0, r1)
+    r3 = __main__.InheritsPython :: type
+    r4 = [r3, self]
+    r5 = load_address r4
+    r6 = PyObject_Vectorcall(r2, r5, 2, 0)
+    keep_alive r3, self
+    r7 = '__setattr__'
+    r8 = CPyObject_GetAttr(r6, r7)
+    r9 = [key, val]
+    r10 = load_address r9
+    r11 = PyObject_Vectorcall(r8, r10, 2, 0)
+    keep_alive key, val
+    return 1
+def BuiltInBase.super_setattr(self, key, val):
+    self :: dict
+    key :: str
+    val, r0 :: object
+    r1 :: str
+    r2, r3 :: object
+    r4 :: object[2]
+    r5 :: object_ptr
+    r6 :: object
+    r7 :: str
+    r8 :: object
+    r9 :: object[2]
+    r10 :: object_ptr
+    r11 :: object
+L0:
+    r0 = builtins :: module
+    r1 = 'super'
+    r2 = CPyObject_GetAttr(r0, r1)
+    r3 = __main__.BuiltInBase :: type
+    r4 = [r3, self]
+    r5 = load_address r4
+    r6 = PyObject_Vectorcall(r2, r5, 2, 0)
+    keep_alive r3, self
+    r7 = '__setattr__'
+    r8 = CPyObject_GetAttr(r6, r7)
+    r9 = [key, val]
+    r10 = load_address r9
+    r11 = PyObject_Vectorcall(r8, r10, 2, 0)
+    keep_alive key, val
+    return 1

mypyc/test-data/run-classes.test

@@ -4598,6 +4598,20 @@ class NoSetAttr:
     def super_setattr(self, attr: str, val: object) -> None:
         super().__setattr__(attr, val)
 
+@mypyc_attr(native_class=False)
+class NoSetAttrNonNative:
+    def __init__(self, attr: int) -> None:
+        self.attr = attr
+
+    def object_setattr(self, attr: str, val: object) -> None:
+        object.__setattr__(self, attr, val)
+
+    def super_setattr(self, attr: str, val: object) -> None:
+        super().__setattr__(attr, val)
+
+    def __getattr__(self, attr: str) -> object:
+        pass
+
 def test_setattr() -> None:
     i = SetAttr(99, {"one": 1})
     assert i.class_var == "x"
@@ -4772,6 +4786,26 @@ def test_no_setattr() -> None:
     with assertRaises(AttributeError):
         object.__setattr__(i, "not_attr", 102)
 
+def test_no_setattr_nonnative() -> None:
+    i = NoSetAttrNonNative(99)
+    i.super_setattr("attr", 100)
+    assert i.attr == 100
+
+    i.object_setattr("attr", 101)
+    assert i.attr == 101
+
+    object.__setattr__(i, "attr", 102)
+    assert i.attr == 102
+
+    i.super_setattr("one", 100)
+    assert i.one == 100
+
+    i.object_setattr("two", 101)
+    assert i.two == 101
+
+    object.__setattr__(i, "three", 102)
+    assert i.three == 102
+
 [typing fixtures/typing-full.pyi]
 
 [case testDunderSetAttrInterpreted]
@@ -4851,8 +4885,22 @@ class NoSetAttr:
     def super_setattr(self, attr: str, val: object) -> None:
         super().__setattr__(attr, val)
 
+@mypyc_attr(native_class=False)
+class NoSetAttrNonNative:
+    def __init__(self, attr: int) -> None:
+        self.attr = attr
+
+    def object_setattr(self, attr: str, val: object) -> None:
+        object.__setattr__(self, attr, val)
+
+    def super_setattr(self, attr: str, val: object) -> None:
+        super().__setattr__(attr, val)
+
+    def __getattr__(self, attr: str) -> object:
+        pass
+
 [file driver.py]
-from native import SetAttr, SetAttrInherited, SetAttrOverridden, SetAttrNonNative, NoSetAttr
+from native import SetAttr, SetAttrInherited, SetAttrOverridden, SetAttrNonNative, NoSetAttr, NoSetAttrNonNative
 from testutil import assertRaises
 
 def test_setattr() -> None:
@@ -5029,10 +5077,31 @@ def test_no_setattr() -> None:
     with assertRaises(AttributeError):
         object.__setattr__(i, "not_attr", 102)
 
+def test_no_setattr_nonnative() -> None:
+    i = NoSetAttrNonNative(99)
+    i.super_setattr("attr", 100)
+    assert i.attr == 100
+
+    i.object_setattr("attr", 101)
+    assert i.attr == 101
+
+    object.__setattr__(i, "attr", 102)
+    assert i.attr == 102
+
+    i.super_setattr("one", 100)
+    assert i.one == 100
+
+    i.object_setattr("two", 101)
+    assert i.two == 101
+
+    object.__setattr__(i, "three", 102)
+    assert i.three == 102
+
 test_setattr()
 test_setattr_inherited()
 test_setattr_overridden()
 test_setattr_nonnative()
 test_no_setattr()
+test_no_setattr_nonnative()
 
 [typing fixtures/typing-full.pyi]