Correctly giving compiler errors on global/attribute buffers

Also, do not stop compilation on first buffer-related error.

Cython/Compiler/Main.py

@@ -330,6 +330,9 @@ class Context:
                     verbose_flag = options.show_version,
                     cplus = options.cplus)
 
+    def nonfatal_error(self, exc):
+        return Errors.report_error(exc)
+
     def run_pipeline(self, pipeline, source):
         errors_occurred = False
         data = source

Cython/Compiler/ParseTreeTransforms.py

@@ -83,7 +83,7 @@ ERR_BUF_MISSING = '"%s" missing'
 ERR_BUF_INT = '"%s" must be an integer'
 ERR_BUF_NONNEG = '"%s" must be non-negative'
 ERR_CDEF_INCLASS = 'Cannot assign default value to cdef class attributes'
-
+ERR_BUF_LOCALONLY = 'Buffer types only allowed as function local variables'
 class PostParse(CythonTransform):
     """
     Basic interpretation of the parse tree, as well as validity
@@ -106,19 +106,25 @@ class PostParse(CythonTransform):
     """
 
     # Track our context.
-    in_class_body = False
+    scope_type = None # can be either of 'module', 'function', 'class'
+
+    def visit_ModuleNode(self, node):
+        self.scope_type = 'module'
+        self.visitchildren(node)
+        return node
+    
     def visit_ClassDefNode(self, node):
-        prev = self.in_class_body
-        self.in_class_body = True
+        prev = self.scope_type
+        self.scope_type = 'class'
         self.visitchildren(node)
-        self.in_class_body = prev
+        self.scope_type = prev
         return node
 
     def visit_FuncDefNode(self, node):
-        prev = self.in_class_body
-        self.in_class_body = False
+        prev = self.scope_type
+        self.scope_type = 'function'
         self.visitchildren(node)
-        self.in_class_body = prev
+        self.scope_type = prev
         return node
 
     # cdef variables
@@ -127,14 +133,20 @@ class PostParse(CythonTransform):
         # declaration. Also, it makes use of the fact that a cdef decl
         # must appear before the first use, so we don't have to deal with
         # "i = 3; cdef int i = i" and can simply move the nodes around.
+        try:
             self.visitchildren(node)
+        except PostParseError, e:
+            # An error in a cdef clause is ok, simply remove the declaration
+            # and try to move on to report more errors
+            self.context.nonfatal_error(e)
+            return None
         stats = [node]
         for decl in node.declarators:
             while isinstance(decl, CPtrDeclaratorNode):
                 decl = decl.base
             if isinstance(decl, CNameDeclaratorNode):
                 if decl.default is not None:
-                    if self.in_class_body:
+                    if self.scope_type == 'class':
                         raise PostParseError(decl.pos, ERR_CDEF_INCLASS)
                     stats.append(SingleAssignmentNode(node.pos,
                         lhs=NameNode(node.pos, name=decl.name),
@@ -145,10 +157,13 @@ class PostParse(CythonTransform):
     # buffer access
     buffer_options = ("dtype", "ndim") # ordered!
     def visit_CBufferAccessTypeNode(self, node):
+        if not self.scope_type == 'function':
+            raise PostParseError(node.pos, ERR_BUF_LOCALONLY)
+        
         options = {}
         # Fetch positional arguments
         if len(node.positional_args) > len(self.buffer_options):
-            self.context.error(ERR_BUF_TOO_MANY)
+            raise PostParseError(node.pos, ERR_BUF_TOO_MANY)
         for arg, unicode_name in zip(node.positional_args, self.buffer_options):
             name = str(unicode_name)
             options[name] = arg
@@ -156,17 +171,16 @@ class PostParse(CythonTransform):
         for item in node.keyword_args.key_value_pairs:
             name = str(item.key.value)
             if not name in self.buffer_options:
-                raise PostParseError(item.key.pos,
-                                     ERR_BUF_UNKNOWN % name)
+                raise PostParseError(item.key.pos, ERR_BUF_OPTION_UNKNOWN % name)
             if name in options.keys():
-                raise PostParseError(item.key.pos,
-                                     ERR_BUF_DUP % key)
+                raise PostParseError(item.key.pos, ERR_BUF_DUP % key)
             options[name] = item.value
 
         provided = options.keys()
         # get dtype
         dtype = options.get("dtype")
-        if dtype is None: raise PostParseError(node.pos, ERR_BUF_MISSING % 'dtype')
+        if dtype is None:
+            raise PostParseError(node.pos, ERR_BUF_MISSING % 'dtype')
         node.dtype_node = dtype
 
         # get ndim

Cython/Compiler/Tests/TestBuffer.py

@@ -47,21 +47,35 @@ class TestBufferParsing(CythonTest):
         self.not_parseable("Non-keyword arg following keyword arg",
                            u"cdef object[foo=1, 2] x")
 
+
+# See also tests/error/e_bufaccess.pyx and tets/run/bufaccess.pyx
 class TestBufferOptions(CythonTest):
     # Tests the full parsing of the options within the brackets
 
-    def parse_opts(self, opts):
-        s = u"cdef object[%s] x" % opts
-        root = self.fragment(s, pipeline=[PostParse(self)]).root
-        buftype = root.stats[0].base_type
+    def nonfatal_error(self, error):
+        # We're passing self as context to transform to trap this
+        self.error = error
+        self.assert_(self.expect_error)
+
+    def parse_opts(self, opts, expect_error=False):
+        s = u"def f():\n  cdef object[%s] x" % opts
+        self.expect_error = expect_error
+        root = self.fragment(s, pipeline=[NormalizeTree(self), PostParse(self)]).root
+        if not expect_error:
+            vardef = root.stats[0].body.stats[0]
+            assert isinstance(vardef, CVarDefNode) # use normal assert as this is to validate the test code
+            buftype = vardef.base_type
             self.assert_(isinstance(buftype, CBufferAccessTypeNode))
             self.assert_(isinstance(buftype.base_type_node, CSimpleBaseTypeNode))
             self.assertEqual(u"object", buftype.base_type_node.name)
             return buftype
+        else:
+            self.assert_(len(root.stats[0].body.stats) == 0)
 
     def non_parse(self, expected_err, opts):
-        e = self.should_fail(lambda: self.parse_opts(opts))
-        self.assertEqual(expected_err, e.message_only)
+        self.parse_opts(opts, expect_error=True)
+#        e = self.should_fail(lambda: self.parse_opts(opts))
+        self.assertEqual(expected_err, self.error.message_only)
         
     def test_basic(self):
         buf = self.parse_opts(u"unsigned short int, 3")
@@ -86,10 +100,12 @@ class TestBufferOptions(CythonTest):
     def test_use_DEF(self):
         t = self.fragment(u"""
         DEF ndim = 3
+        def f():
             cdef object[int, ndim] x
             cdef object[ndim=ndim, dtype=int] y
-        """, pipeline=[PostParse(self)]).root
-        self.assert_(t.stats[1].base_type.ndim == 3)
-        self.assert_(t.stats[2].base_type.ndim == 3)
+        """, pipeline=[NormalizeTree(self), PostParse(self)]).root
+        stats = t.stats[0].body.stats
+        self.assert_(stats[0].base_type.ndim == 3)
+        self.assert_(stats[1].base_type.ndim == 3)
 
-    # add exotic and impossible combinations as they come along
+    # add exotic and impossible combinations as they come along...

tests/errors/e_bufaccess.pyx

+cdef object[int] buf
+cdef class A:
+    cdef object[int] buf
+
+def f():
+    cdef object[fakeoption=True] buf1
+    cdef object[int, -1] buf1b 
+    cdef object[ndim=-1] buf2
+    cdef object[int, 'a'] buf3
+    cdef object[int,2,3,4,5,6] buf4
+
+_ERRORS = u"""
+1:11: Buffer types only allowed as function local variables
+3:15: Buffer types only allowed as function local variables
+6:27: "fakeoption" is not a buffer option
+7:22: "ndim" must be non-negative
+8:15: "dtype" missing
+9:21: "ndim" must be an integer
+10:15: Too many buffer options
+"""
+