Merge pull request #1865 from scoder/default_exc_value

Attempt to use a faster exception value for return type annotations that check for exceptions

Cython/Compiler/Nodes.py

@@ -514,7 +514,7 @@ class CNameDeclaratorNode(CDeclaratorNode):
 
     default = None
 
-    def analyse(self, base_type, env, nonempty=0):
+    def analyse(self, base_type, env, nonempty=0, visibility=None, in_pxd=False):
         if nonempty and self.name == '':
             # May have mistaken the name for the type.
             if base_type.is_ptr or base_type.is_array or base_type.is_buffer:
@@ -540,11 +540,11 @@ class CPtrDeclaratorNode(CDeclaratorNode):
     def analyse_templates(self):
         return self.base.analyse_templates()
 
-    def analyse(self, base_type, env, nonempty=0):
+    def analyse(self, base_type, env, nonempty=0, visibility=None, in_pxd=False):
         if base_type.is_pyobject:
             error(self.pos, "Pointer base type cannot be a Python object")
         ptr_type = PyrexTypes.c_ptr_type(base_type)
-        return self.base.analyse(ptr_type, env, nonempty=nonempty)
+        return self.base.analyse(ptr_type, env, nonempty=nonempty, visibility=visibility, in_pxd=in_pxd)
 
 
 class CReferenceDeclaratorNode(CDeclaratorNode):
@@ -555,11 +555,11 @@ class CReferenceDeclaratorNode(CDeclaratorNode):
     def analyse_templates(self):
         return self.base.analyse_templates()
 
-    def analyse(self, base_type, env, nonempty=0):
+    def analyse(self, base_type, env, nonempty=0, visibility=None, in_pxd=False):
         if base_type.is_pyobject:
             error(self.pos, "Reference base type cannot be a Python object")
         ref_type = PyrexTypes.c_ref_type(base_type)
-        return self.base.analyse(ref_type, env, nonempty=nonempty)
+        return self.base.analyse(ref_type, env, nonempty=nonempty, visibility=visibility, in_pxd=in_pxd)
 
 
 class CArrayDeclaratorNode(CDeclaratorNode):
@@ -568,7 +568,7 @@ class CArrayDeclaratorNode(CDeclaratorNode):
 
     child_attrs = ["base", "dimension"]
 
-    def analyse(self, base_type, env, nonempty=0):
+    def analyse(self, base_type, env, nonempty=0, visibility=None, in_pxd=False):
         if (base_type.is_cpp_class and base_type.is_template_type()) or base_type.is_cfunction:
             from .ExprNodes import TupleNode
             if isinstance(self.dimension, TupleNode):
@@ -582,7 +582,7 @@ class CArrayDeclaratorNode(CDeclaratorNode):
                 base_type = error_type
             else:
                 base_type = base_type.specialize_here(self.pos, values)
-            return self.base.analyse(base_type, env, nonempty=nonempty)
+            return self.base.analyse(base_type, env, nonempty=nonempty, visibility=visibility, in_pxd=in_pxd)
         if self.dimension:
             self.dimension = self.dimension.analyse_const_expression(env)
             if not self.dimension.type.is_int:
@@ -603,7 +603,7 @@ class CArrayDeclaratorNode(CDeclaratorNode):
         if base_type.is_cfunction:
             error(self.pos, "Array element cannot be a function")
         array_type = PyrexTypes.c_array_type(base_type, size)
-        return self.base.analyse(array_type, env, nonempty=nonempty)
+        return self.base.analyse(array_type, env, nonempty=nonempty, visibility=visibility, in_pxd=in_pxd)
 
 
 class CFuncDeclaratorNode(CDeclaratorNode):
@@ -646,7 +646,7 @@ class CFuncDeclaratorNode(CDeclaratorNode):
         else:
             return None
 
-    def analyse(self, return_type, env, nonempty=0, directive_locals=None):
+    def analyse(self, return_type, env, nonempty=0, directive_locals=None, visibility=None, in_pxd=False):
         if directive_locals is None:
             directive_locals = {}
         if nonempty:
@@ -698,6 +698,16 @@ class CFuncDeclaratorNode(CDeclaratorNode):
                 and self.exception_check != '+'):
             error(self.pos, "Exception clause not allowed for function returning Python object")
         else:
+            if self.exception_value is None and self.exception_check and self.exception_check != '+':
+                # Use an explicit exception return value to speed up exception checks.
+                # Even if it is not declared, we can use the default exception value of the return type,
+                # unless the function is some kind of external function that we do not control.
+                if return_type.exception_value is not None and (visibility != 'extern' and not in_pxd):
+                    # Extension types are more difficult because the signature must match the base type signature.
+                    if not env.is_c_class_scope:
+                        from .ExprNodes import ConstNode
+                        self.exception_value = ConstNode(
+                            self.pos, value=return_type.exception_value, type=return_type)
             if self.exception_value:
                 self.exception_value = self.exception_value.analyse_const_expression(env)
                 if self.exception_check == '+':
@@ -752,7 +762,7 @@ class CFuncDeclaratorNode(CDeclaratorNode):
                 error(self.pos, "cannot have both '%s' and '%s' "
                       "calling conventions" % (current, callspec))
             func_type.calling_convention = callspec
-        return self.base.analyse(func_type, env)
+        return self.base.analyse(func_type, env, visibility=visibility, in_pxd=in_pxd)
 
     def declare_optional_arg_struct(self, func_type, env, fused_cname=None):
         """
@@ -792,12 +802,12 @@ class CConstDeclaratorNode(CDeclaratorNode):
 
     child_attrs = ["base"]
 
-    def analyse(self, base_type, env, nonempty=0):
+    def analyse(self, base_type, env, nonempty=0, visibility=None, in_pxd=False):
         if base_type.is_pyobject:
             error(self.pos,
                   "Const base type cannot be a Python object")
         const = PyrexTypes.c_const_type(base_type)
-        return self.base.analyse(const, env, nonempty=nonempty)
+        return self.base.analyse(const, env, nonempty=nonempty, visibility=visibility, in_pxd=in_pxd)
 
 
 class CArgDeclNode(Node):
@@ -1314,9 +1324,11 @@ class CVarDefNode(StatNode):
             if create_extern_wrapper:
                 declarator.overridable = False
             if isinstance(declarator, CFuncDeclaratorNode):
-                name_declarator, type = declarator.analyse(base_type, env, directive_locals=self.directive_locals)
+                name_declarator, type = declarator.analyse(
+                    base_type, env, directive_locals=self.directive_locals, visibility=visibility, in_pxd=self.in_pxd)
             else:
-                name_declarator, type = declarator.analyse(base_type, env)
+                name_declarator, type = declarator.analyse(
+                    base_type, env, visibility=visibility, in_pxd=self.in_pxd)
             if not type.is_complete():
                 if not (self.visibility == 'extern' and type.is_array or type.is_memoryviewslice):
                     error(declarator.pos, "Variable type '%s' is incomplete" % type)
@@ -1568,7 +1580,8 @@ class CTypeDefNode(StatNode):
 
     def analyse_declarations(self, env):
         base = self.base_type.analyse(env)
-        name_declarator, type = self.declarator.analyse(base, env)
+        name_declarator, type = self.declarator.analyse(
+            base, env, visibility=self.visibility, in_pxd=self.in_pxd)
         name = name_declarator.name
         cname = name_declarator.cname
 
@@ -2281,10 +2294,10 @@ class CFuncDefNode(FuncDefNode):
         if isinstance(self.declarator, CFuncDeclaratorNode):
             name_declarator, type = self.declarator.analyse(
                 base_type, env, nonempty=2 * (self.body is not None),
-                directive_locals=self.directive_locals)
+                directive_locals=self.directive_locals, visibility=self.visibility)
         else:
             name_declarator, type = self.declarator.analyse(
-                base_type, env, nonempty=2 * (self.body is not None))
+                base_type, env, nonempty=2 * (self.body is not None), visibility=self.visibility)
         if not type.is_cfunction:
             error(self.pos, "Suite attached to non-function declaration")
         # Remember the actual type according to the function header

Cython/Compiler/Options.py

@@ -143,7 +143,7 @@ _directive_defaults = {
     'initializedcheck' : True,
     'embedsignature' : False,
     'locals' : {},
-    'exceptval' : (None, False),  # (except value, check=False)
+    'exceptval' : None,  # (except value=None, check=True)
     'auto_cpdef': False,
     'auto_pickle': None,
     'cdivision': False, # was True before 0.12

Cython/Compiler/ParseTreeTransforms.py

@@ -905,8 +905,9 @@ class InterpretCompilerDirectives(CythonTransform, SkipDeclarations):
         if optname == 'np_pythran' and not self.context.cpp:
             raise PostParseError(pos, 'The %s directive can only be used in C++ mode.' % optname)
         elif optname == 'exceptval':
+            # default: exceptval(None, check=True)
             arg_error = len(args) > 1
-            check = False
+            check = True
             if kwds and kwds.key_value_pairs:
                 kw = kwds.key_value_pairs[0]
                 if (len(kwds.key_value_pairs) == 1 and
@@ -2344,6 +2345,12 @@ class AdjustDefByDirectives(CythonTransform, SkipDeclarations):
         return_type_node = self.directives.get('returns')
         if return_type_node is None and self.directives['annotation_typing']:
             return_type_node = node.return_type_annotation
+            # for Python anntations, prefer safe exception handling by default
+            if return_type_node is not None and except_val is None:
+                except_val = (None, True)  # except *
+        elif except_val is None:
+            # backward compatible default: no exception check
+            except_val = (None, False)
         if 'ccall' in self.directives:
             node = node.as_cfunction(
                 overridable=True, modifiers=modifiers,

Cython/Shadow.py

@@ -113,7 +113,7 @@ returns = wraparound = boundscheck = initializedcheck = nonecheck = \
     unraisable_tracebacks = freelist = \
         lambda _: _EmptyDecoratorAndManager()
 
-exceptval = lambda _=None, check=False: _EmptyDecoratorAndManager()
+exceptval = lambda _=None, check=True: _EmptyDecoratorAndManager()
 
 optimization = _Optimization()
 

tests/run/annotation_typing.pyx

@@ -146,6 +146,56 @@ def call_struct_io(s : MyStruct) -> MyStruct:
     return struct_io(s)
 
 
+@cython.test_assert_path_exists(
+    "//CFuncDefNode",
+    "//CFuncDefNode//DefNode",
+    "//CFuncDefNode[@return_type]",
+    "//CFuncDefNode[@return_type.is_struct_or_union = True]",
+)
+@cython.ccall
+def struct_convert(d) -> MyStruct:
+    """
+    >>> d = struct_convert(dict(x=1, y=2, data=3))
+    >>> sorted(d.items())
+    [('data', 3.0), ('x', 1), ('y', 2)]
+    >>> struct_convert({})  # make sure we can raise exceptions through struct return values
+    Traceback (most recent call last):
+    ValueError: No value specified for struct attribute 'x'
+    """
+    return d
+
+
+@cython.test_assert_path_exists(
+    "//CFuncDefNode",
+    "//CFuncDefNode//DefNode",
+    "//CFuncDefNode[@return_type]",
+    "//CFuncDefNode[@return_type.is_int = True]",
+)
+@cython.ccall
+def exception_default(raise_exc : cython.bint = False) -> cython.int:
+    """
+    >>> exception_default(raise_exc=False)
+    10
+    >>> exception_default(raise_exc=True)
+    Traceback (most recent call last):
+    ValueError: huhu!
+    """
+    if raise_exc:
+        raise ValueError("huhu!")
+    return 10
+
+
+def call_exception_default(raise_exc=False):
+    """
+    >>> call_exception_default(raise_exc=False)
+    10
+    >>> call_exception_default(raise_exc=True)
+    Traceback (most recent call last):
+    ValueError: huhu!
+    """
+    return exception_default(raise_exc)
+
+
 _WARNINGS = """
 8:32: Strings should no longer be used for type declarations. Use 'cython.int' etc. directly.
 8:47: Dicts should no longer be used as type annotations. Use 'cython.int' etc. directly.
@@ -156,4 +206,6 @@ _WARNINGS = """
 # BUG:
 46:6: 'pytypes_cpdef' redeclared
 121:0: 'struct_io' redeclared
+156:0: 'struct_convert' redeclared
+175:0: 'exception_default' redeclared
 """

tests/run/extern_impl.srctree

@@ -13,12 +13,12 @@ setup(
 
 ######## foo.pxd ########
 
-cdef void bar()
+cdef void bar() except *
 
 ######## foo.pyx ########
 
 cdef extern from "bar_impl.c":
-    void bar()
+    void bar() except *
 
 ######## bar_impl.c ########
 

tests/run/extern_impl_excvalue.srctree

+PYTHON setup.py build_ext --inplace
+PYTHON -c "import foo"
+PYTHON -c "import a"
+
+######## setup.py ########
+
+from Cython.Build import cythonize
+from distutils.core import setup
+
+setup(
+  ext_modules = cythonize("*.pyx"),
+)
+
+######## foo.pxd ########
+
+cdef int bar() except *
+
+######## foo.pyx ########
+
+cdef extern from "bar_impl.c":
+    int bar() except *
+
+######## bar_impl.c ########
+
+static int bar() { return -1; }
+
+######## a.pyx ########
+
+cimport cython
+from foo cimport bar
+
+assert bar() == -1