Avoid using strings for exception_value of func_type

Cython/Compiler/ExprNodes.py

@@ -610,6 +610,25 @@ def compile_time_value_error(self, e):
         error(self.pos, "Error in compile-time expression: %s: %s" % (
             e.__class__.__name__, e))
 
+    def as_exception_value(self, env):
+        # Return the compile_time_value if possible.
+        # This can be either a Python constant or a string
+        # for types that can't be represented by a Python constant
+        # (e.g. enums)
+        result = None
+        if self.constant_result is not constant_value_not_set:
+            return self.constant_result
+        if isinstance(result, Symtab.Entry):
+            result = result.cname
+        if result is None:
+            # this isn't the preferred fallback because it can end up with
+            # hard to distinguish between identical types, e.g. -1.0 vs -1
+            # for floats. However, it lets things like NULL and typecasts work
+            result = self.get_constant_c_result_code()
+        if result is not None:
+            return result
+        error(self.pos, "Exception value must be constant")
+
     # ------------- Declaration Analysis ----------------
 
     def analyse_target_declaration(self, env):
@@ -1311,6 +1330,33 @@ def calculate_result_code(self):
     def generate_result_code(self, code):
         pass
 
+    @staticmethod
+    def for_type(pos, value, type):
+        cls = ConstNode
+        if type is PyrexTypes.c_null_ptr_type or (
+                (value == "NULL" or value == 0) and type.is_ptr):
+            return NullNode(pos)  # value and type are preset here
+        # char node is deliberately skipped and treated as IntNode
+        elif type.is_int or PyrexTypes.c_bint_type:
+            # use this instead of BoolNode for c_bint_type because
+            # BoolNode handles values differently to most other ConstNode
+            # derivatives (they aren't strings).
+            cls = IntNode
+        elif type.is_float:
+            cls = FloatNode
+        elif type is bytes_type:
+            cls = BytesNode
+        elif type is unicode_type:
+            cls = UnicodeNode
+
+        if cls.type is type:
+            result = cls(pos, value=value)
+        else:
+            result = cls(pos, value=value, type=type)
+
+        result.calculate_constant_result()
+        return result
+
 
 class BoolNode(ConstNode):
     type = PyrexTypes.c_bint_type
@@ -6377,7 +6423,7 @@ def c_call_code(self):
 
     def is_c_result_required(self):
         func_type = self.function_type()
-        if not func_type.exception_value or func_type.exception_check == '+':
+        if func_type.exception_value is None or func_type.exception_check == '+':
             return False  # skip allocation of unused result temp
         return True
 

Cython/Compiler/Nodes.py

@@ -740,9 +740,9 @@ def analyse(self, return_type, env, nonempty=0, directive_locals=None, visibilit
                     #   for now.
                     if not env.is_c_class_scope and not isinstance(self.base, CPtrDeclaratorNode):
                         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 = ConstNode.for_type(
+                            self.pos, value=str(return_type.exception_value), type=return_type)
+            if self.exception_value is not None:
                 if self.exception_check == '+':
                     self.exception_value = self.exception_value.analyse_const_expression(env)
                     exc_val_type = self.exception_value.type
@@ -759,9 +759,7 @@ def analyse(self, return_type, env, nonempty=0, directive_locals=None, visibilit
                 else:
                     self.exception_value = self.exception_value.analyse_types(env).coerce_to(
                         return_type, env).analyse_const_expression(env)
-                    exc_val = self.exception_value.get_constant_c_result_code()
-                    if exc_val is None:
-                        error(self.exception_value.pos, "Exception value must be constant")
+                    exc_val = self.exception_value.as_exception_value(env)
                     if not return_type.assignable_from(self.exception_value.type):
                         error(self.exception_value.pos,
                               "Exception value incompatible with function return type")
@@ -2651,7 +2649,7 @@ def analyse_declarations(self, env):
                   "Function with optional arguments may not be declared public or api")
 
         if typ.exception_check == '+' and self.visibility != 'extern':
-            if typ.exception_value and typ.exception_value.is_name:
+            if typ.exception_value is not None and typ.exception_value.is_name:
                 # it really is impossible to reason about what the user wants to happens
                 # if they've specified a C++ exception translation function. Therefore,
                 # raise an error.
@@ -3121,8 +3119,8 @@ def as_cfunction(self, cfunc=None, scope=None, overridable=True, returns=None, e
 
         if exception_value is None and cfunc_type.exception_value is not None:
             from .ExprNodes import ConstNode
-            exception_value = ConstNode(
-                self.pos, value=cfunc_type.exception_value, type=cfunc_type.return_type)
+            exception_value = ConstNode.for_type(
+                self.pos, value=str(cfunc_type.exception_value), type=cfunc_type.return_type)
         declarator = CFuncDeclaratorNode(self.pos,
                                          base=CNameDeclaratorNode(self.pos, name=self.name, cname=None),
                                          args=self.args,

Cython/Compiler/PyrexTypes.py

@@ -608,7 +608,7 @@ def overflow_check_binop(self, binop, env, const_rhs=False):
 
     def error_condition(self, result_code):
         if self.typedef_is_external:
-            if self.exception_value:
+            if self.exception_value is not None:
                 condition = "(%s == %s)" % (
                     result_code, self.cast_code(self.exception_value))
                 if self.exception_check:
@@ -2921,7 +2921,7 @@ def assignment_failure_extra_info(self, src_type):
             if self.base_type.pointer_assignable_from_resolved_type(copied_src_type):
                 # the only reason we can't assign is because of exception incompatibility
                 msg = "Exception values are incompatible."
-                if not self.base_type.exception_check and not self.base_type.exception_value:
+                if not self.base_type.exception_check and self.base_type.exception_value is None:
                     msg += " Suggest adding 'noexcept' to type '{}'.".format(src_type)
                 return msg
         return super(CPtrType, self).assignment_failure_extra_info(src_type)
@@ -3080,7 +3080,7 @@ def __repr__(self):
         arg_reprs = list(map(repr, self.args))
         if self.has_varargs:
             arg_reprs.append("...")
-        if self.exception_value:
+        if self.exception_value is not None:
             except_clause = " %r" % self.exception_value
         else:
             except_clause = ""
@@ -3308,11 +3308,11 @@ def declaration_code(self, entity_code,
             arg_decl_code = "void"
         trailer = ""
         if (pyrex or for_display) and not self.return_type.is_pyobject:
-            if self.exception_value and self.exception_check:
+            if self.exception_value is not None and self.exception_check:
                 trailer = " except? %s" % self.exception_value
-            elif self.exception_value and not self.exception_check:
+            elif self.exception_value is not None and not self.exception_check:
                 trailer = " except %s" % self.exception_value
-            elif not self.exception_value and not self.exception_check:
+            elif self.exception_value is None and not self.exception_check:
                 trailer = " noexcept"
             elif self.exception_check == '+':
                 trailer = " except +"
@@ -3514,7 +3514,7 @@ def __init__(self, arg_name, arg_type):
             except_clause = 'except *'
         elif self.return_type.is_pyobject:
             except_clause = ''
-        elif self.exception_value:
+        elif self.exception_value is not None:
             except_clause = ('except? %s' if self.exception_check else 'except %s') % self.exception_value
         else:
             except_clause = 'except *'

tests/run/exceptionpropagation.pyx

@@ -133,3 +133,80 @@ def test_ptr_func2(int failure_mode):
             print("NULL")
     except RuntimeError:
         print("exception")
+
+cdef double return_double(double arg, bint fail):
+    if fail:
+        raise RuntimeError
+    return arg
+
+ctypedef double (*func_ptr1)(double, bint) except? -1
+
+def test_return_double(fail):
+    """
+    >>> test_return_double(False)
+    2.0
+    >>> test_return_double(True)
+    exception
+    """
+    # Test that we can assign to the function pointer we expect
+    # https://github.com/cython/cython/issues/5709 - representation of the "-1" was fragile
+    cdef func_ptr1 p1 = return_double
+    try:
+        return p1(2.0, fail)
+    except RuntimeError:
+        print("exception")
+
+cdef enum E:
+    E1
+    E2
+
+cdef E return_enum1(bint fail) except? E.E1:
+    if fail:
+        raise RuntimeError
+    return E.E2
+
+cdef E return_enum2(bint fail) except? E1:
+    if fail:
+        raise RuntimeError
+    return E.E2
+
+def test_enum1(fail):
+    """
+    >>> test_enum1(False)
+    True
+    >>> test_enum1(True)
+    exception
+    """
+    try:
+        return return_enum1(fail) == E2
+    except RuntimeError:
+        print("exception")
+
+def test_enum2(fail):
+    """
+    >>> test_enum2(False)
+    True
+    >>> test_enum2(True)
+    exception
+    """
+    try:
+        return return_enum2(fail) == E2
+    except RuntimeError:
+        print("exception")
+
+cdef char return_char(fail) except 'a':
+    if fail:
+        raise RuntimeError
+    return 'b'
+
+def test_return_char(fail):
+    """
+    >>> test_return_char(False)
+    'b'
+    >>> test_return_char(True)
+    exception
+    """
+    try:
+        return chr(return_char(fail))
+    except RuntimeError:
+        print("exception")

tests/run/pure_py.py

@@ -408,13 +408,15 @@ def ccall_except_check(x):
     return x+1
 
 
-@cython.test_fail_if_path_exists("//CFuncDeclaratorNode//IntNode[@base_10_value = '-1']")
 @cython.test_assert_path_exists("//CFuncDeclaratorNode")
 @cython.ccall
 @cython.returns(cython.long)
 @cython.exceptval(check=True)
 def ccall_except_check_always(x):
     """
+    Note that this actually takes the same shortcut as for a Cython-syntax cdef function
+    and does except ?-1
+
     >>> ccall_except_check_always(41)
     42
     >>> ccall_except_check_always(0)