Refactor cypclass reference counting code generation

Cython/Compiler/Code.py

@@ -2103,30 +2103,8 @@ class CCodeWriter(object):
             return ''
         return '%s ' % ' '.join([mapper(m,m) for m in modifiers])
 
-    # CyObjects reference counting
 
-    def put_cygotref(self, cname):
-        self.putln("Cy_GOTREF(%s);" % cname)
-
-    def put_cygiveref(self, cname):
-        self.putln("Cy_GIVEREF(%s);" % cname)
-
-    def put_cyxgiveref(self, cname):
-        self.putln("Cy_XGIVEREF(%s);" % cname)
-
-    def put_cyxgotref(self, cname):
-        self.putln("Cy_XGOTREF(%s);" % cname)
-
-    def put_cyincref(self, cname):
-        self.putln("Cy_INCREF(%s);" % cname)
-
-    def put_cydecref(self, cname):
-        self.putln("Cy_DECREF(%s);" % cname)
-
-    def put_cyxdecref(self, cname):
-        self.putln("Cy_XDECREF(%s);" % cname)
-
-    # Python objects and reference counting
+    # Reference counting
 
     def entry_as_pyobject(self, entry):
         type = entry.type

Cython/Compiler/ExprNodes.py

@@ -782,8 +782,9 @@ class ExprNode(Node):
         If the result is in a temp, it is already a new reference.
         """
         if not self.result_in_temp():
-            if self.type.is_cyp_class and "NULL" not in self.result():
-                code.put_cyincref(self.result())
+            # FIXME: is this verification really necessary ?
+            if self.type.is_cyp_class and "NULL" in self.result():
+                pass
             else:
                 code.put_incref(self.result(), self.ctype())
 
@@ -827,9 +828,6 @@ class ExprNode(Node):
                 self.generate_subexpr_disposal_code(code)
                 self.free_subexpr_temps(code)
             if self.result():
-                if self.type.is_cyp_class:
-                    code.put_cyxdecref(self.result())
-                else:
                 code.put_decref_clear(self.result(), self.ctype(),
                                         have_gil=not self.in_nogil_context)
             if self.has_temp_moved:
@@ -1851,11 +1849,9 @@ class ImagNode(AtomicExprNode):
                     self.result(),
                     float(self.value),
                     code.error_goto_if_null(self.result(), self.pos)))
-            if self.type.is_cyp_class:
-                code.put_cygotref(self.result())
-            else:
             self.generate_gotref(code)
-
+        elif self.type.is_cyp_class:
+            self.generate_gotref(code)
 
 class NewExprNode(AtomicExprNode):
 
@@ -2336,7 +2332,7 @@ class NameNode(AtomicExprNode):
             self.generate_gotref(code)
 
         elif entry.type.is_cyp_class:
-            code.put_cygotref(self.result())
+            self.generate_gotref(code)
             #pass
             # code.putln(entry.cname)
         elif entry.is_local or entry.in_closure or entry.from_closure or entry.type.is_memoryviewslice:
@@ -2438,7 +2434,7 @@ class NameNode(AtomicExprNode):
             elif self.type.is_cyp_class:
                 if self.use_managed_ref:
                     rhs.make_owned_reference(code)
-                code.put_cyxdecref(self.result())
+                code.put_xdecref(self.result(), self.type)
             if not self.type.is_memoryviewslice:
                 if not assigned:
                     if overloaded_assignment:
@@ -2536,7 +2532,7 @@ class NameNode(AtomicExprNode):
                     '}' % (del_code, code.error_goto(self.pos)))
             else:
                 code.put_error_if_neg(self.pos, del_code)
-        elif self.entry.type.is_pyobject or self.entry.type.is_memoryviewslice:
+        elif self.entry.type.is_pyobject or self.entry.type.is_memoryviewslice or self.entry.type.is_cyp_class:
             if not self.cf_is_null:
                 if self.cf_maybe_null and not ignore_nonexisting:
                     code.put_error_if_unbound(self.pos, self.entry)
@@ -2550,22 +2546,6 @@ class NameNode(AtomicExprNode):
                 else:
                     code.put_decref_clear(self.result(), self.ctype(),
                                           have_gil=not self.nogil)
-        elif self.entry.type.is_cyp_class:
-            if not self.cf_is_null:
-                if self.cf_maybe_null and not ignore_nonexisting:
-                    code.put_error_if_unbound(self.pos, self.entry)
-
-                if self.entry.in_closure:
-                    # generator
-                    if ignore_nonexisting and self.cf_maybe_null:
-                        code.put_cyxgotref(self.result())
-                    else:
-                        code.put_cygotref(self.result())
-                if ignore_nonexisting and self.cf_maybe_null:
-                    code.put_cyxdecref(self.result())
-                else:
-                    code.put_cydecref(self.result())
-                code.putln('%s = NULL;' % self.result())
         else:
             error(self.pos, "Deletion of C names not supported")
 
@@ -3034,7 +3014,7 @@ class NextNode(AtomicExprNode):
     def generate_result_code(self, code):
         self.iterator.generate_iter_next_result_code(self.result(), code)
         if self.type.is_cyp_class:
-            code.put_cyincref(self.result())
+            code.put_incref(self.result(), self.type)
 
 
 class AsyncIteratorNode(ExprNode):
@@ -4295,7 +4275,7 @@ class IndexNode(_IndexingBaseNode):
         if self.type.is_pyobject:
             self.generate_gotref(code)
         elif self.type.is_cyp_class:
-            code.put_cygotref(self.result())
+            code.put_gotref(self.result(), self.type)
 
     def generate_setitem_code(self, value_code, code):
         if self.index.type.is_int:
@@ -6339,7 +6319,7 @@ class SimpleCallNode(CallNode):
                 if self.type.is_pyobject and self.result():
                     self.generate_gotref(code)
                 elif self.type.is_cyp_class and self.result():
-                    code.put_cygotref(self.result())
+                    self.generate_gotref(code)
             if self.has_optional_args:
                 code.funcstate.release_temp(self.opt_arg_struct)
 
@@ -7624,9 +7604,9 @@ class AttributeNode(ExprNode):
                         select_code, rhs, rhs.result(), self.type, code)
             elif self.type.is_cyp_class:
                 rhs.make_owned_reference(code)
-                code.put_cygiveref(rhs.result())
-                code.put_cygotref(select_code)
-                code.put_cyxdecref(select_code)
+                rhs.generate_giveref(code)
+                code.put_gotref(select_code, self.type)
+                code.put_xdecref(select_code, self.type)
 
             if not self.type.is_memoryviewslice:
                 code.putln(
@@ -10908,7 +10888,7 @@ class TypecastNode(ExprNode):
                             self.result(),
                             self.type.declaration_code(''),
                             operand_result))
-                code.put_cyincref(self.result())
+                code.put_incref(self.result(), self.type)
 
 
 ERR_START = "Start may not be given"
@@ -14014,9 +13994,6 @@ class CoerceToTempNode(CoercionNode):
             self.result(), self.arg.result_as(self.ctype())))
         if self.use_managed_ref:
             if not self.type.is_memoryviewslice:
-                if self.type.is_cyp_class:
-                    code.put_cyincref(self.result())
-                else:
                 code.put_incref(self.result(), self.ctype())
             else:
                 code.put_incref_memoryviewslice(self.result(), self.type,

Cython/Compiler/Nodes.py

@@ -965,9 +965,6 @@ class CArgDeclNode(Node):
         default.make_owned_reference(code)
         result = default.result() if overloaded_assignment else default.result_as(self.type)
         code.putln("%s = %s;" % (target, result))
-        if self.type.is_cyp_class:
-            code.put_cygiveref(default.result())
-        else:
         code.put_giveref(default.result(), self.type)
         default.generate_post_assignment_code(code)
         default.free_temps(code)
@@ -2127,6 +2124,9 @@ class FuncDefNode(StatNode, BlockNode):
         for entry in lenv.arg_entries:
             if not entry.type.is_memoryviewslice:
                 if (acquire_gil or entry.cf_is_reassigned) and not entry.in_closure:
+                    if entry.type.is_cyp_class and entry.is_self_arg:
+                        pass
+                    else:
                         code.put_var_incref(entry)
             # Note: defaults are always incref-ed. For def functions, we
             #       we acquire arguments from object conversion, so we have
@@ -2135,9 +2135,6 @@ class FuncDefNode(StatNode, BlockNode):
             elif is_cdef and entry.cf_is_reassigned:
                 code.put_var_incref_memoryviewslice(entry,
                                     have_gil=code.funcstate.gil_owned)
-            # We have to Cy_INCREF the nogil classes (ccdef'ed ones)
-            elif entry.type.is_cyp_class and len(entry.cf_assignments) > 1 and not entry.is_self_arg:
-                code.put_cyincref(entry.cname)
         for entry in lenv.var_entries:
             if entry.is_arg and entry.cf_is_reassigned and not entry.in_closure:
                 if entry.xdecref_cleanup:
@@ -2325,9 +2322,6 @@ class FuncDefNode(StatNode, BlockNode):
                     continue
             if entry.type.needs_refcounting:
                 assure_gil('success')
-            if entry.type.is_cyp_class:
-                code.put_cyxdecref(entry.cname)
-            else:
             # FIXME ideally use entry.xdecref_cleanup but this currently isn't reliable
             code.put_var_xdecref(entry, have_gil=gil_owned['success'])
 
@@ -2345,10 +2339,8 @@ class FuncDefNode(StatNode, BlockNode):
                     continue
                 if entry.type.needs_refcounting:
                     assure_gil('success')
-            if entry.type.is_cyp_class and not entry.is_self_arg:
-                # We must check for NULL because it is possible to have
-                # NULL as a valid cypclass (with a typecast)
-                code.put_cyxdecref(entry.cname)
+            if entry.type.is_cyp_class and entry.is_self_arg:
+                pass
             else:
                 # FIXME use entry.xdecref_cleanup - del arg seems to be the problem
                 code.put_var_xdecref(entry, have_gil=gil_owned['success'])
@@ -2358,15 +2350,13 @@ class FuncDefNode(StatNode, BlockNode):
 
         # ----- Return
         # This code is duplicated in ModuleNode.generate_module_init_func
-        if not lenv.nogil:
+        # We can always return a CythonExtensionType as it is nogil-compliant
+        if not lenv.nogil or self.return_type.is_cyp_class:
             default_retval = return_type.default_value
             err_val = self.error_value()
             if err_val is None and default_retval:
                 err_val = default_retval  # FIXME: why is err_val not used?
             code.put_xgiveref(Naming.retval_cname, return_type)
-        # We can always return a CythonExtensionType as it is nogil-compliant
-        if self.return_type.is_cyp_class:
-            code.put_cyxgiveref(Naming.retval_cname)
 
         if self.entry.is_special and self.entry.name == "__hash__":
             # Returning -1 for __hash__ is supposed to signal an error
@@ -3712,20 +3702,22 @@ class DefNodeWrapper(FuncDefNode):
         # ----- Non-error return cleanup
         code.put_label(code.return_label)
         for entry in lenv.var_entries:
-            if entry.is_arg and entry.type.is_pyobject:
+            if entry.is_arg:
+                # The conversion from PyObject to CyObject always creates a new CyObject reference.
+                # This decrements all arguments-as-variables converted straight from an actual argument.
+                # This includes CyObjects converted directly from a corresponding PyObject argument.
                 if entry.xdecref_cleanup:
                     code.put_var_xdecref(entry)
                 else:
                     code.put_var_decref(entry)
-            elif entry.is_arg and entry.type.is_cyp_class:
-                # The conversion from PyObject to CyObject creates a new CyObject reference.
-                code.put_cyxdecref(entry.cname)
         for entry in lenv.arg_entries:
             if entry.type.is_cyp_class:
-                # The conversion from PyObject to CyObject creates a new CyObject reference.
-                # Such a conversion occurs even when the argument is not explicitly marked
-                # as needing a conversion and declared as a variable in var_entries.
-                code.put_cyxdecref(entry.cname)
+                # The conversion from PyObject to CyObject always creates a new CyObject reference.
+                # This decrements CyObjects converted from generic PyObject args passed via tuple and kw dict.
+                if entry.xdecref_cleanup:
+                    code.put_var_xdecref(entry)
+                else:
+                    code.put_var_decref(entry)
 
         code.put_finish_refcount_context()
         if not self.return_type.is_void:
@@ -6626,7 +6618,7 @@ class ReturnStatNode(StatNode):
                 value = None
 
         if self.return_type.is_cyp_class:
-            code.put_cyxdecref(Naming.retval_cname)
+            code.put_xdecref(Naming.retval_cname, self.return_type)
 
         if value:
             value.generate_evaluation_code(code)

Cython/Compiler/PyrexTypes.py

@@ -4387,6 +4387,43 @@ class CypClassType(CppClassType):
         init_entry.is_cfunction = 1
         return init_entry
 
+    def generate_incref(self, code, cname, **kwds):
+        code.putln("Cy_INCREF(%s);" % cname)
+
+    generate_xincref = generate_incref
+
+    def generate_decref(self, code, cname, **kwds):
+        code.putln("Cy_DECREF(%s);" % cname)
+
+    def generate_decref_clear(self, code, cname, **kwds):
+        code.putln("Cy_DECREF(%s);" % cname)
+        code.putln("%s = NULL;" % cname)
+
+    def generate_xdecref(self, code, cname, **kwds):
+        code.putln("Cy_XDECREF(%s);" % cname)
+
+    def generate_xdecref_clear(self, code, cname, **kwds):
+        code.putln("Cy_XDECREF(%s);" % cname)
+        code.putln("%s = NULL;" % cname)
+
+    def generate_gotref(self, code, cname):
+        code.putln("Cy_GOTREF(%s);" % cname)
+
+    def generate_xgotref(self, code, cname):
+        code.putln("Cy_XGOTREF(%s);" % cname)
+
+    def generate_giveref(self, code, cname):
+        code.putln("Cy_GIVEREF(%s);" % cname)
+
+    def generate_xgiveref(self, code, cname):
+        code.putln("Cy_XGIVEREF(%s);" % cname)
+
+    def generate_decref_set(self, code, cname, rhs_cname):
+        raise NotImplementedError("Cy_DECREF_SET is not defined for decrementing a group of cyobjects")
+
+    def generate_xdecref_set(self, code, cname, rhs_cname):
+        raise NotImplementedError("Cy_XDECREF_SET is not defined for decrementing a group of cyobjects")
+
 
 class TemplatePlaceholderType(CType):
 

Cython/Compiler/UtilNodes.py

@@ -201,10 +201,8 @@ class ResultRefNode(AtomicExprNode):
         if self.type.is_pyobject or self.type.is_cyp_class:
             rhs.make_owned_reference(code)
             if not self.lhs_of_first_assignment:
-                if self.type.is_pyobject:
+                if self.type.is_pyobject or self.type.is_cyp_class:
                     code.put_decref(self.result(), self.ctype())
-                elif self.type.is_cyp_class:
-                    code.put_cydecref(self.result())
         code.putln('%s = %s;' % (
             self.result(),
             rhs.result() if overloaded_assignment else rhs.result_as(self.ctype()),
@@ -249,10 +247,8 @@ class LetNodeMixin:
             self.temp_expression.generate_disposal_code(code)
             self.temp_expression.free_temps(code)
         else:
-            if self.temp_type.is_pyobject:
+            if self.temp_type.is_pyobject or self.temp_type.is_cyp_class:
                 code.put_decref_clear(self.temp, self.temp_type)
-            elif self.temp_type.is_cyp_class:
-                code.put_cyxdecref(self.temp)
             code.funcstate.release_temp(self.temp)