Author: Matti Picus <matti.pi...@gmail.com>
Branch: 
Changeset: r92160:40ee3c492e28
Date: 2017-08-16 22:56 +0300
http://bitbucket.org/pypy/pypy/changeset/40ee3c492e28/

Log:    refactor 9ddefd44f80d handling pre-existing exceptions, add tests,
        still not bulletproof

diff --git a/pypy/module/cpyext/api.py b/pypy/module/cpyext/api.py
--- a/pypy/module/cpyext/api.py
+++ b/pypy/module/cpyext/api.py
@@ -1575,7 +1575,7 @@
         assert cpyext_glob_tid_ptr[0] == 0
         cpyext_glob_tid_ptr[0] = tid
 
-        preexist_error = PyErr_Occurred(space) is not None
+        preexist_error = PyErr_Occurred(space)
         try:
             # Call the function
             result = call_external_function(func, *boxed_args)
@@ -1597,18 +1597,20 @@
                     ret = None
 
             # Check for exception consistency
-            has_error = PyErr_Occurred(space) is not None
+            # XXX best attempt, will miss preexisting error that is
+            # overwritten with a new error of the same type
+            error = PyErr_Occurred(space)
+            has_new_error = (error is not None) and (error is not 
preexist_error)
             has_result = ret is not None
-            if not preexist_error:
-                if has_error and has_result:
-                    raise oefmt(space.w_SystemError,
-                                "An exception was set, but function returned a 
"
-                                "value")
-                elif not expect_null and not has_error and not has_result:
-                    raise oefmt(space.w_SystemError,
-                                "Function returned a NULL result without 
setting "
-                                "an exception")
-            if has_error:
+            if not expect_null and has_new_error and has_result:
+                raise oefmt(space.w_SystemError,
+                            "An exception was set, but function returned a "
+                            "value")
+            elif not expect_null and not has_new_error and not has_result:
+                raise oefmt(space.w_SystemError,
+                            "Function returned a NULL result without setting "
+                            "an exception")
+            elif has_new_error:
                 state = space.fromcache(State)
                 state.check_and_raise_exception()
 
diff --git a/pypy/module/cpyext/test/test_cpyext.py 
b/pypy/module/cpyext/test/test_cpyext.py
--- a/pypy/module/cpyext/test/test_cpyext.py
+++ b/pypy/module/cpyext/test/test_cpyext.py
@@ -24,6 +24,10 @@
 def PyPy_Crash2(space):
     1/0
 
+@api.cpython_api([api.PyObject], api.PyObject, result_is_ll=True)
+def PyPy_Noop(space, pyobj):
+    return pyobj
+
 class TestApi:
     def test_signature(self):
         common_functions = api.FUNCTIONS_BY_HEADER[api.pypy_decl]
@@ -685,6 +689,7 @@
         body = """
         PyAPI_FUNC(PyObject*) PyPy_Crash1(void);
         PyAPI_FUNC(long) PyPy_Crash2(void);
+        PyAPI_FUNC(PyObject*) PyPy_Noop(PyObject*);
         static PyObject* foo_crash1(PyObject* self, PyObject *args)
         {
             return PyPy_Crash1();
@@ -708,9 +713,27 @@
             int a = PyPy_Crash2();
             return PyFloat_FromDouble(a);
         }
+        static PyObject* foo_noop(PyObject* self, PyObject* args)
+        {
+            Py_INCREF(args);
+            return PyPy_Noop(args);
+        }
+        static PyObject* foo_set(PyObject* self, PyObject *args)
+        {
+            PyErr_SetString(PyExc_TypeError, "clear called with no error");
+            if (PyInt_Check(args)) {
+                Py_INCREF(args);
+                return args;
+            }
+            return NULL;
+        }
         static PyObject* foo_clear(PyObject* self, PyObject *args)
         {
             PyErr_Clear();
+            if (PyInt_Check(args)) {
+                Py_INCREF(args);
+                return args;
+            }
             return NULL;
         }
         static PyMethodDef methods[] = {
@@ -718,20 +741,53 @@
             { "crash2", foo_crash2, METH_NOARGS },
             { "crash3", foo_crash3, METH_NOARGS },
             { "crash4", foo_crash4, METH_NOARGS },
-            { "clear",  foo_clear, METH_NOARGS },
+            { "clear",  foo_clear,  METH_O },
+            { "set",    foo_set,    METH_O },
+            { "noop",   foo_noop,   METH_O },
             { NULL }
         };
         """
         module = self.import_module(name='foo', init=init, body=body)
+
         # uncaught interplevel exceptions are turned into SystemError
-        raises(SystemError, module.crash1)
-        raises(SystemError, module.crash2)
-        # caught exception
+        expected = "ZeroDivisionError('integer division or modulo by zero',)"
+        exc = raises(SystemError, module.crash1)
+        assert exc.value[0] == expected
+
+        exc = raises(SystemError, module.crash2)
+        assert exc.value[0] == expected
+
+        # caught exception, api.cpython_api return value works
         assert module.crash3() == -1
-        # An exception was set, but function returned a value
-        raises(SystemError, module.crash4)
-        # No exception set, but NULL returned
-        raises(SystemError, module.clear)
+
+        expected = 'An exception was set, but function returned a value'
+        # PyPy only incompatibility/extension
+        exc = raises(SystemError, module.crash4)
+        assert exc.value[0] == expected
+
+        # An exception was set by the previous call, it can pass
+        # cleanly through a call that doesn't check error state
+        assert module.noop(1) == 1
+
+        # clear the exception but return NULL, signalling an error
+        expected = 'Function returned a NULL result without setting an 
exception'
+        exc = raises(SystemError, module.clear, None)
+        assert exc.value[0] == expected
+
+        # Set an exception and return NULL
+        raises(TypeError, module.set, None)
+
+        # clear any exception and return a value 
+        assert module.clear(1) == 1
+
+        # Set an exception, but return non-NULL
+        expected = 'An exception was set, but function returned a value'
+        exc = raises(SystemError, module.set, 1)
+        assert exc.value[0] == expected
+        
+
+        # Clear the exception and return a value, all is OK
+        assert module.clear(1) == 1
 
     def test_new_exception(self):
         mod = self.import_extension('foo', [
_______________________________________________
pypy-commit mailing list
pypy-commit@python.org
https://mail.python.org/mailman/listinfo/pypy-commit

Reply via email to