merge

Cython/Compiler/Buffer.py

@@ -291,7 +291,7 @@ def put_assign_to_buffer(lhs_cname, rhs_cname, buffer_aux, buffer_type,
         # Release any existing buffer
         code.putln('__Pyx_SafeReleaseBuffer(&%s);' % bufstruct)
         # Acquire
-        retcode_cname = code.funcstate.allocate_temp(PyrexTypes.c_int_type)
+        retcode_cname = code.funcstate.allocate_temp(PyrexTypes.c_int_type, manage_ref=False)
         code.putln("%s = %s;" % (retcode_cname, getbuffer % rhs_cname))
         code.putln('if (%s) {' % (code.unlikely("%s < 0" % retcode_cname)))
         # If acquisition failed, attempt to reacquire the old buffer
@@ -353,7 +353,7 @@ def put_buffer_lookup_code(entry, index_signeds, index_cnames, options, pos, cod
         # We allocate a temporary which is initialized to -1, meaning OK (!).
         # If an error occurs, the temp is set to the dimension index the
         # error is occuring at.
-        tmp_cname = code.funcstate.allocate_temp(PyrexTypes.c_int_type)
+        tmp_cname = code.funcstate.allocate_temp(PyrexTypes.c_int_type, manage_ref=False)
         code.putln("%s = -1;" % tmp_cname)
         for dim, (signed, cname, shape) in enumerate(zip(index_signeds, index_cnames,
                                                          bufaux.shapevars)):

Cython/Compiler/Code.py

@@ -104,7 +104,7 @@ class FunctionState(object):
     def label_used(self, lbl):
         return lbl in self.labels_used
 
-    def allocate_temp(self, type, manage_ref=True):
+    def allocate_temp(self, type, manage_ref):
         """
         Allocates a temporary (which may create a new one or get a previously
         allocated and released one of the same type). Type is simply registered
@@ -115,11 +115,16 @@ class FunctionState(object):
         handling clauses. Otherwise the caller has to deal with any reference
         counting of the variable.
 
+        If not type.is_pyobject, then manage_ref will be ignored, but it
+        still has to be passed. It is recommended to pass False by convention
+        if it is known that type will never be a Python object.
+
         A C string referring to the variable is returned.
         """
-        if not type.is_pyobject and manage_ref is not True:
-            # Make manage_ref canonical for temps_free lookup purposes
-            raise ValueError("manage_ref only applicable when type.is_pyobject")
+        if not type.is_pyobject:
+            # Make manage_ref canonical, so that manage_ref will always mean
+            # a decref is needed.
+            manage_ref = False
         freelist = self.temps_free.get((type, manage_ref))
         if freelist is not None and len(freelist) > 0:
             result = freelist.pop()
@@ -163,7 +168,14 @@ class FunctionState(object):
         """
         return [(name, type)
                 for name, type, manage_ref in self.temps_in_use()
-                if (type.is_pyobject and manage_ref)]
+                if manage_ref]
+
+    def all_managed_temps(self):
+        """Return a list of (cname, type) tuples of refcount-managed Python objects.
+        """
+        return [(cname, type)
+                for cname, type, manage_ref in self.temps_allocated
+                if manage_ref]
 
 class GlobalState(object):
     # filename_table   {string : int}  for finding filename table indexes

Cython/Compiler/ExprNodes.py

@@ -613,7 +613,7 @@ class NewTempExprNode(ExprNode):
                 if self.backwards_compatible_result:
                     self.temp_code = self.backwards_compatible_result
                 else:
-                    self.temp_code = code.funcstate.allocate_temp(type)
+                    self.temp_code = code.funcstate.allocate_temp(type, manage_ref=True)
             else:
                 self.temp_code = None
 
@@ -1718,18 +1718,13 @@ class IndexNode(ExprNode):
         if self.buffer_type.dtype.is_pyobject:
             # Must manage refcounts. Decref what is already there
             # and incref what we put in.
-            ptr = code.funcstate.allocate_temp(self.buffer_type.buffer_ptr_type)
-            if rhs.is_temp:
-                rhs_code = code.funcstate.allocate_temp(rhs.type)
-            else:
+            ptr = code.funcstate.allocate_temp(self.buffer_type.buffer_ptr_type, manage_ref=False)
             rhs_code = rhs.result()
             code.putln("%s = %s;" % (ptr, ptrexpr))
             code.putln("Py_DECREF(*%s); Py_INCREF(%s);" % (
                 ptr, rhs_code
                 ))
             code.putln("*%s %s= %s;" % (ptr, op, rhs_code))
-            if rhs.is_temp:
-                code.funcstate.release_temp(rhs_code)
             code.funcstate.release_temp(ptr)
         else: 
             # Simple case
@@ -1772,7 +1767,7 @@ class IndexNode(ExprNode):
 
     def buffer_lookup_code(self, code):
         # Assign indices to temps
-        index_temps = [code.funcstate.allocate_temp(i.type) for i in self.indices]
+        index_temps = [code.funcstate.allocate_temp(i.type, manage_ref=False) for i in self.indices]
         for temp, index in zip(index_temps, self.indices):
             code.putln("%s = %s;" % (temp, index.result()))
         # Generate buffer access code using these temps

Cython/Compiler/Nodes.py

@@ -1063,8 +1063,7 @@ class FuncDefNode(StatNode, BlockNode):
             # cleanup temps the old way
             code.put_var_xdecrefs(lenv.temp_entries)
             # cleanup temps the new way
-            for cname, type, manage_ref in code.funcstate.temps_allocated:
-                if type.is_pyobject and manage_ref:
+            for cname, type in code.funcstate.all_managed_temps():
                 code.put_xdecref(cname, type)
 
             # Clean up buffers -- this calls a Python function

tests/run/bufaccess.pyx

@@ -972,6 +972,33 @@ def assign_to_object(object[object] buf, int idx, obj):
     """
     buf[idx] = obj
     
+@testcase
+def assign_temporary_to_object(object[object] buf):
+    """
+    See comments on printbuf_object above.
+
+    >>> a, b = [1, 2, 3], {4:23}
+    >>> get_refcount(a)
+    2
+    >>> addref(a)
+    >>> A = ObjectMockBuffer(None, [b, a])
+    >>> get_refcount(a)
+    3
+    >>> assign_temporary_to_object(A)
+    >>> get_refcount(a)
+    2
+    
+    >>> printbuf_object(A, (2,))
+    {4: 23} 2
+    {1: 8} 2
+
+    To avoid leaking a reference in our testcase we need to
+    replace the temporary with something we can manually decref :-)
+    >>> assign_to_object(A, 1, a)
+    >>> decref(a)
+    """
+    buf[1] = {3-2: 2+(2*4)-2}
+
 #
 # cast option
 #