drop ref-counting on C attribute swaps

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 2dcecf2e16e0e3473e164f4c4bb278c5ffee6a55..9e44753f971ae39ddfb7ad10159a7a58b8e1d8b9 100644 (file)
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -3098,7 +3098,7 @@ class AttributeNode(ExprNode):
                 self.put_nonecheck(code)
 
             select_code = self.result()
-            if self.type.is_pyobject:
+            if self.type.is_pyobject and self.use_managed_ref:
                 rhs.make_owned_reference(code)
                 code.put_giveref(rhs.py_result())
                 code.put_gotref(select_code)

diff --git a/Cython/Compiler/Optimize.py b/Cython/Compiler/Optimize.py
index 4f12fa0f38e6c1b9e7fe463f421eb17c4f60cba7..0cff21b97a52bd3949e19004e0f2a4a40828917d 100644 (file)
--- a/Cython/Compiler/Optimize.py
+++ b/Cython/Compiler/Optimize.py
@@ -634,8 +634,8 @@ class DropRefcountingTransform(Visitor.VisitorTransform):
 
         if left_names or right_names:
             # lhs/rhs names must be a non-redundant permutation
-            lnames = [n.name for n in left_names]
-            rnames = [n.name for n in right_names]
+            lnames = [ path for path, n in left_names ]
+            rnames = [ path for path, n in right_names ]
             if set(lnames) != set(rnames):
                 return node
             if len(set(lnames)) != len(right_names):
@@ -670,7 +670,7 @@ class DropRefcountingTransform(Visitor.VisitorTransform):
         for temp in temps:
             temp.use_managed_ref = False
 
-        for name_node in left_names + right_names:
+        for _, name_node in left_names + right_names:
             if name_node not in temp_args:
                 name_node.use_managed_ref = False
 
@@ -686,10 +686,16 @@ class DropRefcountingTransform(Visitor.VisitorTransform):
         if isinstance(node, ExprNodes.CoerceToTempNode):
             temps.append(node)
             node = node.arg
-        if isinstance(node, ExprNodes.NameNode):
-            if node.entry.is_builtin or node.entry.is_pyglobal:
+        name_path = []
+        obj_node = node
+        while isinstance(obj_node, ExprNodes.AttributeNode):
+            if obj_node.is_py_attr:
                 return False
-            names.append(node)
+            name_path.append(obj_node.member)
+            obj_node = obj_node.obj
+        if isinstance(obj_node, ExprNodes.NameNode):
+            name_path.append(obj_node.name)
+            names.append( ('.'.join(name_path[::-1]), node) )
         elif isinstance(node, ExprNodes.IndexNode):
             if node.base.type != Builtin.list_type:
                 return False

diff --git a/tests/run/parallel_swap_assign_T425.pyx b/tests/run/parallel_swap_assign_T425.pyx
index a337e935a410cddfb4006a38fc315d94968f0d1d..18db9c0dc6bc0e02f0f4b0becb29c3d0aea7757a 100644 (file)
--- a/tests/run/parallel_swap_assign_T425.pyx
+++ b/tests/run/parallel_swap_assign_T425.pyx
@@ -68,10 +68,43 @@ def swap_cmp5(a,b,c,d,e):
     "//ParallelAssignmentNode/SingleAssignmentNode//CoerceToTempNode[@use_managed_ref=False]",
     )
 def swap_py(a,b):
+    """
+    >>> swap_py(1,2)
+    (1, 2)
+    """
     a,a = b,a
     return a,b
 
 
+cdef class A:
+    cdef readonly object x
+    cdef readonly object y
+    def __init__(self, x, y):
+        self.x, self.y = x, y
+
+@cython.test_assert_path_exists(
+    "//ParallelAssignmentNode",
+    "//ParallelAssignmentNode/SingleAssignmentNode",
+    "//ParallelAssignmentNode/SingleAssignmentNode//AttributeNode/NameNode",
+    "//ParallelAssignmentNode/SingleAssignmentNode//AttributeNode[@use_managed_ref=False]/NameNode",
+    )
+@cython.test_fail_if_path_exists(
+#    "//ParallelAssignmentNode/SingleAssignmentNode//AttributeNode[@use_managed_ref=True]",
+    )
+def swap_attr_values(A a, A b):
+    """
+    >>> a, b = A(1,2), A(3,4)
+    >>> a.x, a.y, b.x, b.y
+    (1, 2, 3, 4)
+    >>> swap_attr_values(a,b)
+    >>> a.x, a.y, b.x, b.y
+    (3, 2, 1, 4)
+    """
+    a.x, a.y, b.x, b.y = a.y, b.x, b.y, a.x # shift by one
+    a.x, a.y, b.x, b.y = b.x, b.y, a.x, a.y # shift by two
+    a.x, a.y, b.x, b.y = b.y, b.x, a.y, a.x # reverse
+
+
 @cython.test_assert_path_exists(
 #    "//ParallelAssignmentNode",
 #    "//ParallelAssignmentNode/SingleAssignmentNode",