Buffer refactor nearly done, but there is a tricky segfaulting bug somewhere...

Cython/Compiler/Buffer.py

@@ -8,6 +8,103 @@ from Cython.Compiler.Errors import CompileError
 import PyrexTypes
 from sets import Set as set
 
+def used_buffer_aux_vars(entry):
+    buffer_aux = entry.buffer_aux
+    buffer_aux.buffer_info_var.used = True
+    for s in buffer_aux.shapevars: s.used = True
+    for s in buffer_aux.stridevars: s.used = True
+
+def put_unpack_buffer_aux_into_scope(buffer_aux, code):
+    bufstruct = buffer_aux.buffer_info_var.cname
+
+    code.putln(" ".join(["%s = %s.strides[%d];" %
+                         (s.cname, bufstruct, idx)
+                         for idx, s in enumerate(buffer_aux.stridevars)]))
+    code.putln(" ".join(["%s = %s.shape[%d];" %
+                         (s.cname, bufstruct, idx)
+                         for idx, s in enumerate(buffer_aux.shapevars)]))
+
+def put_zero_buffer_aux_into_scope(buffer_aux, code):
+    # If new buffer is None, set up to access 0
+    # for a "safer segfault" on access
+    code.putln("%s.buf = 0;" % buffer_aux.buffer_info_var.cname) 
+    code.putln(" ".join(["%s = 0;" % s.cname
+                         for s in buffer_aux.stridevars]))
+    code.putln(" ".join(["%s = 0;" % s.cname
+                         for s in buffer_aux.shapevars]))    
+
+def put_acquire_arg_buffer(entry, code, pos):
+    buffer_aux = entry.buffer_aux
+    cname  = entry.cname
+    bufstruct = buffer_aux.buffer_info_var.cname
+    flags = '0'
+    # Acquire any new buffer
+    code.put('if (%s != Py_None) ' % cname)
+    code.begin_block()
+    code.putln('%s.buf = 0;' % bufstruct) # PEP requirement
+    code.put(code.error_goto_if(
+        'PyObject_GetBuffer(%s, &%s, %s) == -1' % (
+        cname, bufstruct, flags), pos))
+    # An exception raised in arg parsing cannot be catched, so no
+    # need to do care about the buffer then.
+    put_unpack_buffer_aux_into_scope(buffer_aux, code)
+    code.end_block()
+
+def put_release_buffer(entry, code):
+    code.putln("if (%s != Py_None) PyObject_ReleaseBuffer(%s, &%s);" % (
+        entry.cname, entry.cname, entry.buffer_aux.buffer_info_var.cname))
+
+def put_assign_to_buffer(lhs_cname, rhs_cname, buffer_aux, is_initialized, pos, code):
+    bufstruct = buffer_aux.buffer_info_var.cname
+    flags = '0'
+
+    if is_initialized:
+        # Release any existing buffer
+        code.put('if (%s != Py_None) ' % lhs_cname)
+        code.begin_block();
+        code.putln('PyObject_ReleaseBuffer(%s, &%s);' % (
+            lhs_cname, bufstruct))
+        code.end_block()
+    # Acquire any new buffer
+    code.put('if (%s != Py_None) ' % rhs_cname)
+    code.begin_block()
+    code.putln('%s.buf = 0;' % bufstruct) # PEP requirement
+    code.put('if (%s) ' % code.unlikely(
+        'PyObject_GetBuffer(%s, &%s, %s) == -1' % (
+            rhs_cname,
+            bufstruct,
+            flags)
+         + ' || %s((char*)%s.format) == NULL' % (
+            buffer_aux.tschecker.cname, bufstruct
+        )))
+    code.begin_block()
+    # If acquisition failed, attempt to reacquire the old buffer
+    # before raising the exception. A failure of reacquisition
+    # will cause the reacquisition exception to be reported, one
+    # can consider working around this later.
+    if is_initialized:
+        put_zero_buffer_aux_into_scope(buffer_aux, code)
+        code.put('if (%s != Py_None && PyObject_GetBuffer(%s, &%s, %s) == -1) ' % (
+            lhs_cname, lhs_cname, bufstruct, flags))
+        code.begin_block()
+        put_zero_buffer_aux_into_scope(buffer_aux, code)
+        code.end_block()
+    else:
+        # our entry had no previous vaule, so set to None when acquisition fails
+        code.putln('%s = Py_None; Py_INCREF(Py_None);' % lhs_cname)
+    code.putln(code.error_goto(pos))
+    code.end_block() # acquisition failure
+    # Unpack indices
+    put_unpack_buffer_aux_into_scope(buffer_aux, code)
+    code.putln('} else {')
+    # If new buffer is None, set up to access 0
+    # for a "safer segfault" on access
+    put_zero_buffer_aux_into_scope(buffer_aux, code)
+    code.end_block()
+
+    # Everything is ok, assign object variable
+    code.putln("%s = %s;" % (lhs_cname, rhs_cname))
+
 class PureCFuncNode(Node):
     child_attrs = []
     
@@ -103,27 +200,25 @@ class IntroduceBufferAuxiliaryVars(CythonTransform):
             tschecker = self.tschecker(buftype.dtype)
 
             # Declare auxiliary vars
-            bufinfo = scope.declare_var(temp_name_handle(u"%s_bufinfo" % name),
-                                        self.bufstruct_type, node.pos)
-
-            temp_var =  scope.declare_var(temp_name_handle(u"%s_tmp" % name),
-                                        entry.type, node.pos)
+            cname = scope.mangle(Naming.bufstruct_prefix, name)
+            bufinfo = scope.declare_var(name="$%s" % cname, cname=cname,
+                                        type=self.bufstruct_type, pos=node.pos)
+
+            bufinfo.used = True
+
+            def var(prefix, idx):
+                cname = scope.mangle(prefix, "%d_%s" % (idx, name))
+                result = scope.declare_var("$%s" % cname, PyrexTypes.c_py_ssize_t_type,
+                                         node.pos, cname=cname, is_cdef=True)
+                result.init = "0"
+                if entry.is_arg:
+                    result.used = True
+                return result
             
+            stridevars = [var(Naming.bufstride_prefix, i) for i in range(entry.type.ndim)]
+            shapevars = [var(Naming.bufshape_prefix, i) for i in range(entry.type.ndim)]            
+            entry.buffer_aux = Symtab.BufferAux(bufinfo, stridevars, shapevars, tschecker)
             
-            stridevars = []
-            shapevars = []
-            for idx in range(buftype.ndim):
-                # stride
-                varname = temp_name_handle(u"%s_%s%d" % (name, "stride", idx))
-                var = scope.declare_var(varname, PyrexTypes.c_int_type, node.pos, is_cdef=True)
-                stridevars.append(var)
-                # shape
-                varname = temp_name_handle(u"%s_%s%d" % (name, "shape", idx))
-                var = scope.declare_var(varname, PyrexTypes.c_uint_type, node.pos, is_cdef=True)
-                shapevars.append(var)
-            entry.buffer_aux = Symtab.BufferAux(bufinfo, stridevars, 
-                                                shapevars, tschecker)
-            entry.buffer_aux.temp_var = temp_var
         scope.buffer_entries = bufvars
         self.scope = scope
 

Cython/Compiler/Code.py

@@ -211,7 +211,7 @@ class CCodeWriter:
             #print "...private and not definition, skipping" ###
             return
         if not entry.used and visibility == "private":
-            #print "not used and private, skipping" ###
+            #print "not used and private, skipping", entry.cname ###
             return
         storage_class = ""
         if visibility == 'extern':
@@ -352,12 +352,15 @@ class CCodeWriter:
             pos[1],
             cinfo,
             lbl)
-            
-    def error_goto_if(self, cond, pos):
+
+    def unlikely(self, cond):
         if Options.gcc_branch_hints:
-            return "if (unlikely(%s)) %s" % (cond, self.error_goto(pos))
+            return 'unlikely(%s)' % cond
         else:
-            return "if (%s) %s" % (cond, self.error_goto(pos))
+            return cond
+        
+    def error_goto_if(self, cond, pos):
+        return "if (%s) %s" % (self.unlikely(cond), self.error_goto(pos))
             
     def error_goto_if_null(self, cname, pos):
         return self.error_goto_if("!%s" % cname, pos)

Cython/Compiler/ExprNodes.py

@@ -894,10 +894,14 @@ class NameNode(AtomicExprNode):
             self.type = PyrexTypes.error_type
         self.entry.used = 1
         if self.entry.type.is_buffer:
-            # Need some temps
-            
-        print self.dump()
-        
+            # Have an rhs temp just in case. All rhs I could
+            # think of had a single symbol result_code but better
+            # safe than sorry. Feel free to change this.
+            import Buffer
+            self.new_buffer_temp = Symtab.new_temp(self.entry.type)
+            self.temps = [self.new_buffer_temp]
+            Buffer.used_buffer_aux_vars(self.entry)
+                
     def analyse_rvalue_entry(self, env):
         #print "NameNode.analyse_rvalue_entry:", self.name ###
         #print "Entry:", self.entry.__dict__ ###
@@ -1012,7 +1016,7 @@ class NameNode(AtomicExprNode):
         entry = self.entry
         if entry is None:
             return # There was an error earlier
-
+        
         # is_pyglobal seems to be True for module level-globals only.
         # We use this to access class->tp_dict if necessary.
         if entry.is_pyglobal:
@@ -1054,11 +1058,26 @@ class NameNode(AtomicExprNode):
                         code.put_xdecref(self.result_code, self.ctype())
                 else:
                     code.put_decref(self.result_code, self.ctype())
-            code.putln('%s = %s;' % (self.result_code, rhs.result_as(self.ctype())))
+            if self.type.is_buffer:
+                self.generate_acquire_buffer(rhs, code)
+            else:
+                code.putln('%s = %s;' % (self.result_code, rhs.result_as(self.ctype())))
             if debug_disposal_code:
                 print("NameNode.generate_assignment_code:")
                 print("...generating post-assignment code for %s" % rhs)
             rhs.generate_post_assignment_code(code)
+
+    def generate_acquire_buffer(self, rhs, code):
+        rhstmp = self.new_buffer_temp.cname
+        buffer_aux = self.entry.buffer_aux
+        bufstruct = buffer_aux.buffer_info_var.cname
+        code.putln('%s = %s;' % (rhstmp, rhs.result_as(self.ctype())))
+
+        import Buffer
+        Buffer.put_assign_to_buffer(self.result_code, rhstmp, buffer_aux,
+                                    is_initialized=not self.skip_assignment_decref,
+                                    pos=self.pos, code=code)
+        code.putln("%s = 0;" % rhstmp)
     
     def generate_deletion_code(self, code):
         if self.entry is None:

Cython/Compiler/Naming.py

@@ -31,6 +31,10 @@ prop_set_prefix   = pyrex_prefix + "setprop_"
 type_prefix       = pyrex_prefix + "t_"
 typeobj_prefix    = pyrex_prefix + "type_"
 var_prefix        = pyrex_prefix + "v_"
+bufstruct_prefix  = pyrex_prefix + "bstruct_"
+bufstride_prefix  = pyrex_prefix + "bstride_"
+bufshape_prefix   = pyrex_prefix + "bshape_"
+bufoffset_prefix  = pyrex_prefix + "boffset_"
 vtable_prefix     = pyrex_prefix + "vtable_"
 vtabptr_prefix    = pyrex_prefix + "vtabptr_"
 vtabstruct_prefix = pyrex_prefix + "vtabstruct_"

Cython/Compiler/Nodes.py

@@ -828,6 +828,7 @@ class FuncDefNode(StatNode, BlockNode):
         return lenv
                 
     def generate_function_definitions(self, env, code, transforms):
+        import Buffer
         # Generate C code for header and body of function
         code.init_labels()
         lenv = self.local_scope
@@ -876,7 +877,9 @@ class FuncDefNode(StatNode, BlockNode):
         for entry in lenv.arg_entries:
             if entry.type.is_pyobject and lenv.control_flow.get_state((entry.name, 'source')) != 'arg':
                 code.put_var_incref(entry)
-        # ----- Initialise local variables
+            if entry.type.is_buffer:
+                Buffer.put_acquire_arg_buffer(entry, code, self.pos)
+        # ----- Initialise local variables 
         for entry in lenv.var_entries:
             if entry.type.is_pyobject and entry.init_to_none and entry.used:
                 code.put_init_var_to_py_none(entry)
@@ -925,6 +928,8 @@ class FuncDefNode(StatNode, BlockNode):
             for entry in lenv.var_entries:
                 if lenv.control_flow.get_state((entry.name, 'initalized')) is not True:
                     entry.xdecref_cleanup = 1
+        for entry in lenv.buffer_entries:
+            Buffer.put_release_buffer(entry, code)
         code.put_var_decrefs(lenv.var_entries, used_only = 1)
         # Decref any increfed args
         for entry in lenv.arg_entries:

tests/run/bufaccess.pyx

 cimport __cython__
 
+__doc__ = u"""
+    >>> A = MockBuffer("i", range(10), label="A")
+    >>> B = MockBuffer("i", range(10), label="B")
+    >>> E = ErrorBuffer("E")
+    
+    >>> acquire_release(A, B)
+    acquired A
+    released A
+    acquired B
+    released B
+"""
+
 __doc__ = u"""
     >>> A = MockBuffer("i", range(10), label="A")
     >>> B = MockBuffer("i", range(10), label="B")
@@ -63,6 +75,13 @@ __doc__ = u"""
     released
 """
 
+__sdfdoc__ = """
+    >>> printbuf_float(MockBuffer("f", [1.0, 1.25, 0.75, 1.0]), (4,))
+    acquired
+    1.0 1.25 0.75 1.0
+    released
+"""
+
 ctypedef char* (*write_func_ptr)(char*, object)
 cdef char* write_float(char* buf, object value):
     (<float*>buf)[0] = <float>value
@@ -89,6 +108,7 @@ def acquire_release(o1, o2):
 def acquire_raise(o):
     cdef object[int] buf
     buf = o
+    o.printlog()
     raise Exception("on purpose")
 
 def as_argument(object[int] bufarg, int n):