Refactor&simplify the approach:

1. declare that the return value of general RPython function is undefined and
   that you should not rely on it

2. merge @llhelper_can_raise and @ll_error_value into a combined
   @llhelper_error_value: this is the only case which we ultimately care about,
   and it's simpler to test/implement them together than separately

3. introduce a failing test (test_enforce_llhelper_error_value_in_case_of_nested_exception)
   which will be fixed in the next commit.

6 files changed, 70 insertions, 162 deletions

diff --git a/rpython/rlib/objectmodel.py b/rpython/rlib/objectmodel.py
index cf0170ded6..5faa6850fd 100644
--- a/rpython/rlib/objectmodel.py
+++ b/rpython/rlib/objectmodel.py
@@ -226,29 +226,20 @@ def not_rpython(func):
     func._not_rpython_ = True
     return func
 
-def llhelper_can_raise(error_value):
+def llhelper_error_value(error_value):
     """
-    Instruct ll2ctypes that this llhelper can raise RPython exceptions:
+    This decorator has two effects:
 
-      - when called from RPython, the function will be propagated
+      1. declare that this llhelper can raise RPython exceptions, which should
+         be correctly propagated
 
-      - when called from C, the function will set the exc_data and return error_value.
-
-    This decorator also implies @ll_error_value.
-    """
-    def decorate(func):
-        func._llhelper_can_raise_ = True
-        return ll_error_value(error_value)(func)
-    return decorate
-
-def ll_error_value(llval):
-    """
-    Specify the error value returned by the function when it raises an
-    exception
+      2. specify the error_value to return in case of exception.
     """
-    # this is tested by test_exceptiontransform.test_custom_error_value
+    # relevant tests:
+    #     - test_ll2ctypes.test_llhelper_error_value
+    #     - test_exceptiontransform.test_custom_error_value
     def decorate(func):
-        func._ll_error_value_ = llval
+        func._llhelper_error_value_ = error_value
         return func
     return decorate
 
diff --git a/rpython/rtyper/llinterp.py b/rpython/rtyper/llinterp.py
index 6c4fbfea20..ac5cff518d 100644
--- a/rpython/rtyper/llinterp.py
+++ b/rpython/rtyper/llinterp.py
@@ -26,10 +26,10 @@ log.output_disabled = True
 class LLException(Exception):
 
     # .error_value is used only by tests: in particular,
-    # test_exceptiontransform uses it check that in case of exception the
-    # function returns the expected value
+    # test_exceptiontransform uses it to check what is the return value of the
+    # function in case of exception
     UNDEFINED_ERROR_VALUE = object() # sentinel for self.error_value
-    
+
     def __init__(self, *args, **kwargs):
         "NOT_RPYTHON"
         Exception.__init__(self, *args)
@@ -356,12 +356,7 @@ class LLFrame(object):
                         tracer.dump('raise')
                     exc_data.exc_type = lltype.typeOf(etype)._defl()
                     exc_data.exc_value = lltype.typeOf(evalue)._defl()
-                    # check that the exc-transformed graph returns the error
-                    # value when it returns with an exception set
-                    from rpython.translator import exceptiontransform
-                    errvalue = exceptiontransform.error_value(self.graph)
-                    assert result == errvalue
-                    raise LLException(etype, evalue, error_value=errvalue)
+                    raise LLException(etype, evalue, error_value=result)
             if tracer:
                 tracer.dump('return')
             return None, result
diff --git a/rpython/rtyper/lltypesystem/ll2ctypes.py b/rpython/rtyper/lltypesystem/ll2ctypes.py
index 2dfff25b0c..696e043961 100644
--- a/rpython/rtyper/lltypesystem/ll2ctypes.py
+++ b/rpython/rtyper/lltypesystem/ll2ctypes.py
@@ -895,9 +895,9 @@ def lltype2ctypes(llobj, normalize=True):
                         global _callback_exc_info
                         _callback_exc_info = sys.exc_info()
                         _callable = getattr(container, '_callable', None)
-                        if hasattr(_callable, '_llhelper_can_raise_'):
-                            # see rlib.objectmodel.llhelper_can_raise
-                            llres = _callable._ll_error_value_
+                        if hasattr(_callable, '_llhelper_error_value_'):
+                            # see rlib.objectmodel.llhelper_error_value
+                            llres = _callable._llhelper_error_value_
                             assert lltype.typeOf(llres) == T.TO.RESULT
                             return ctypes_return_value(llres)
                         else:
@@ -1358,7 +1358,7 @@ def get_ctypes_trampoline(FUNCTYPE, cfunc):
             etype, evalue, etb = _callback_exc_info
             # cres is the actual C result returned by the function. Stick it
             # into the exception so that we can check it inside tests (see
-            # e.g. test_llhelper_can_raise)
+            # e.g. test_llhelper_error_value)
             evalue._ll2ctypes_c_result = cres
             _callback_exc_info = None
             raise etype, evalue, etb
diff --git a/rpython/rtyper/lltypesystem/test/test_ll2ctypes.py b/rpython/rtyper/lltypesystem/test/test_ll2ctypes.py
index 3d7e1861e0..c0375e0099 100644
--- a/rpython/rtyper/lltypesystem/test/test_ll2ctypes.py
+++ b/rpython/rtyper/lltypesystem/test/test_ll2ctypes.py
@@ -480,11 +480,11 @@ class TestLL2Ctypes(object):
         assert res == 42
         assert not ALLOCATED     # detects memory leaks in the test
 
-    def test_llhelper_can_raise(self, monkeypatch):
-        from rpython.rlib.objectmodel import llhelper_can_raise
+    def test_llhelper_error_value(self, monkeypatch):
+        from rpython.rlib.objectmodel import llhelper_error_value
         class FooError(Exception):
             pass
-        @llhelper_can_raise(error_value=-7)
+        @llhelper_error_value(error_value=-7)
         def dummy(n):
             raise FooError(n + 2)
 
diff --git a/rpython/translator/exceptiontransform.py b/rpython/translator/exceptiontransform.py
index d1c66874c8..ca1d45ff99 100644
--- a/rpython/translator/exceptiontransform.py
+++ b/rpython/translator/exceptiontransform.py
@@ -16,6 +16,19 @@ from rpython.rtyper.llannotation import lltype_to_annotation
 from rpython.rtyper.annlowlevel import MixLevelHelperAnnotator
 from rpython.tool.sourcetools import func_with_new_name
 
+# ~~~ NOTE ~~~
+# The exact value returned by a function is NOT DEFINED. The returned value is
+# IGNORED EVERYWHERE in RPython and the question "was_an_exception_raised()"
+# is implemented by checking the content of the global exc_data.
+#
+# The only case in which the returned error value is significant is for
+# llhelpers which can be called by 3rd-party C functions, such as e.g. the HPy
+# API. In that case, the error value is specified by @llhelper_error_value.
+#
+# The following table defines the default error values to return, but in
+# general the returned value is not guaranteed. The only case in which it is
+# guaranteed is for functions decorated with @llhelper_error_value
+
 PrimitiveErrorValue = {lltype.Signed: -1,
                        lltype.Unsigned: r_uint(-1),
                        lltype.SignedLongLong: r_longlong(-1),
@@ -43,12 +56,12 @@ def default_error_value(T):
 def error_value(graph):
     assert isinstance(graph, FunctionGraph)
     T = graph.returnblock.inputargs[0].concretetype
-    if hasattr(graph, 'func') and hasattr(graph.func, '_ll_error_value_'):
+    if hasattr(graph, 'func') and hasattr(graph.func, '_llhelper_error_value_'):
         # custom case
-        errval = graph.func._ll_error_value_
+        errval = graph.func._llhelper_error_value_
         if lltype.typeOf(errval) != T:
-            raise TypeError('Wrong type for @error_value: expected %s but got %s' %
-                            (T, lltype.typeOf(errval)))
+            raise TypeError('Wrong type for @llhelper_error_value: expected %s '
+                            'but got %s' % (T, lltype.typeOf(errval)))
         return errval
     # default case
     return default_error_value(T)
diff --git a/rpython/translator/test/test_exceptiontransform.py b/rpython/translator/test/test_exceptiontransform.py
index 4690f48ddc..2479115021 100644
--- a/rpython/translator/test/test_exceptiontransform.py
+++ b/rpython/translator/test/test_exceptiontransform.py
@@ -4,7 +4,10 @@ from rpython.translator.simplify import join_blocks
 from rpython.translator import exceptiontransform
 from rpython.flowspace.model import summary
 from rpython.rtyper.llinterp import LLException
+from rpython.rtyper.lltypesystem import lltype
+from rpython.rtyper.annlowlevel import llhelper
 from rpython.rtyper.test.test_llinterp import get_interpreter
+from rpython.rlib.objectmodel import llhelper_error_value, dont_inline
 from rpython.translator.backendopt.all import backend_optimizations
 from rpython.conftest import option
 import sys
@@ -281,148 +284,54 @@ class TestExceptionTransform:
         assert exc.value.error_value == -1
         assert 'ValueError' in str(exc.value)
 
-    def test_custom_error_value(self):
-        from rpython.rlib.objectmodel import ll_error_value
-        @ll_error_value(-456)
-        def foo(x):
+    def test_llhelper_error_value(self):
+        @llhelper_error_value(-456)
+        def _foo(x):
             if x == 42:
                 raise ValueError
             return x
 
-        result = interpret(foo, [123])
+        FN = lltype.Ptr(lltype.FuncType([lltype.Signed], lltype.Signed))
+        def bar(x):
+            foo = llhelper(FN, _foo)
+            return foo(x)
+
+        result = interpret(bar, [123])
         assert result == 123
         with py.test.raises(LLException) as exc:
-            interpret(foo, [42])
+            interpret(bar, [42])
         assert exc.value.error_value == -456
         assert 'ValueError' in str(exc.value)
         #
-        compiled_foo = self.compile(foo, [int], backendopt=False)
+        compiled_foo = self.compile(bar, [int], backendopt=False)
         assert compiled_foo(123) == 123
         compiled_foo(42, expected_exception_name='ValueError')
 
-    def test_custom_error_value_propagate(self):
-        from rpython.rlib.objectmodel import ll_error_value
-        @ll_error_value(-456)
-        def foo(x):
-            if x == 42:
-                raise ValueError
-            return x
-
-        def bar(x):
-            try:
-                return foo(x)
-            except ValueError:
-                return 123
-
-        result = interpret(bar, [10])
-        assert result == 10
-        result = interpret(bar, [42])
-        assert result == 123
-        #
-        compiled_bar = self.compile(bar, [int], backendopt=False)
-        assert compiled_bar(10) == 10
-        assert compiled_bar(42) == 123
-
-    def test_custom_error_value_reraise(self):
-        from rpython.rlib.objectmodel import ll_error_value
-        @ll_error_value(-456)
-        def foo(x):
-            if x == 42:
-                raise ValueError
-            return x
-
-        def bar(x):
-            try:
-                return foo(x)
-            except ValueError:
-                raise
-
-        with py.test.raises(LLException) as exc:
-            interpret(bar, [42])
-        assert exc.value.error_value == -1
-
-    def test_raise_from_llhelper(self):
-        from rpython.rtyper.lltypesystem import lltype
-        from rpython.rtyper.annlowlevel import llhelper
-
-        def fn(a, b):
+    @pytest.mark.xfail('WIP')
+    def test_enforce_llhelper_error_value_in_case_of_nested_exception(self):
+        @dont_inline
+        def my_divide(a, b):
             if b == 0:
                 raise ZeroDivisionError
             return a/b
 
-        FN = lltype.Ptr(lltype.FuncType([lltype.Signed, lltype.Signed], lltype.Signed))
-        def h(a, b):
-            fnptr = llhelper(FN, fn)
-            try:
-                return fnptr(a, b)
-            except ZeroDivisionError:
-                return -a
-
-        compiled_h = self.compile(h, [int, int])
-        res = compiled_h(39, 3)
-        assert res == 13
-        res = compiled_h(39, 0)
-        assert res == -39
-
-    def test_propagate_from_llhelper(self):
-        from rpython.rtyper.lltypesystem import lltype
-        from rpython.rtyper.annlowlevel import llhelper
-
-        def fn(a, b):
-            if b == 0:
-                raise ZeroDivisionError
-            return a/b
+        @llhelper_error_value(-456)
+        def _foo(a, b):
+            res = my_divide(a, b)
+            return res
 
         FN = lltype.Ptr(lltype.FuncType([lltype.Signed, lltype.Signed], lltype.Signed))
-        def h(a, b):
-            fnptr = llhelper(FN, fn)
-            try:
-                return fnptr(a, b)
-            except ZeroDivisionError:
-                return -a
-
-        compiled_h = self.compile(h, [int, int])
-        res = compiled_h(39, 3)
-        assert res == 13
-        res = compiled_h(39, 0)
-        assert res == -39
-
-    def test_llhelper_can_raise_custome_error_value(self):
-        from rpython.rtyper.lltypesystem import lltype
-        from rpython.rtyper.annlowlevel import llhelper
-        from rpython.rlib.objectmodel import llhelper_can_raise
+        def bar(a, b):
+            foo = llhelper(FN, _foo)
+            return foo(a, b)
 
-        @llhelper_can_raise(error_value=-123)
-        def fn(a, b):
-            if b == 0:
-                raise ZeroDivisionError
-            return a/b
-
-        # check that if we call it directly, it returns -123
+        result = interpret(bar, [21, 3])
+        assert result == 7
         with py.test.raises(LLException) as exc:
-            interpret(fn, [3, 0])
-        assert exc.value.error_value == -123
-
-
-        # ==============
-        # XXX this test is BROKEN at the moment, DO NOT MERGE THIS BRANCH
-        #
-        # The problem is that the return types of h and fnptr are both Signed:
-        # so, exceptiontransform creates a graph which directly return the
-        # value of fnptr, EVEN IN CASE OF EXCEPTION. But in this case, h
-        # should return -1, not -123.
+            interpret(bar, [21, 0])
+        assert exc.value.error_value == -456
+        assert 'ZeroDivisionError' in str(exc.value)
         #
-        # If you do "return float(fntptr(a, b))" the test passes, because in
-        # this case exceptiontransform have to put an explit
-        # "has-an-rpy-exc-occurred" check after the indirect call
-        # ==============
-
-        # check that we can call it as a llhelper
-        FN = lltype.Ptr(lltype.FuncType([lltype.Signed, lltype.Signed], lltype.Signed))
-        def h(a, b):
-            fnptr = llhelper(FN, fn)
-            return fnptr(a, b)
-
-        with py.test.raises(LLException) as exc:
-            interpret(h, [3, 0])
-        assert exc.value.error_value == -1 # the return value of h, not of fn!
+        compiled_foo = self.compile(bar, [int], backendopt=False)
+        assert compiled_foo(123) == 123
+        compiled_foo(42, expected_exception_name='ValueError')