Fix error propagation from memoryview-returning functions
authorMatěj Laitl <matej@laitl.cz>
Thu, 24 Jan 2013 10:59:23 +0000 (11:59 +0100)
committerMatěj Laitl <matej@laitl.cz>
Thu, 24 Jan 2013 10:59:23 +0000 (11:59 +0100)
A code like
|cdef double[:] foo():
|    raise AttributeError('dummy')

generated C code like this:
|  __pyx_L1_error:;
|  (...)
|  __pyx_r.memview = NULL;
|  __Pyx_AddTraceback("view_return_errors.foo", __pyx_clineno, __pyx_lineno, __pyx_filename);
|  __pyx_L0:;
|  if (unlikely(!__pyx_r.memview)) {
|    PyErr_SetString(PyExc_TypeError,"Memoryview return value is not initialized");
|  }
|  __Pyx_RefNannyFinishContext();
|  return __pyx_r;
|}

Which is incorrect in error case, because we set __pyx_r.memview to NULL and
then we test it, so that the PyErr_SetString() is always called in the error
case, which swallows previously-set error. (it also swallowed the traceback,
which I don't understand)

A fix is to jump to return_from_error_cleanup label in case return type is
memoryview, as it is currently done for the case when buffers are present.

A testcase that fauils w/out this fix applied is included, too.

v2: fix test under Python 3 by not using StandardError

Cython/Compiler/Nodes.py
tests/memoryview/view_return_errors.pyx [new file with mode: 0644]

index 07fc79b..e790698 100644 (file)
@@ -1770,7 +1770,7 @@ class FuncDefNode(StatNode, BlockNode):
             # If we are using the non-error cleanup section we should
             # jump past it if we have an error. The if-test below determine
             # whether this section is used.
-            if buffers_present or is_getbuffer_slot:
+            if buffers_present or is_getbuffer_slot or self.return_type.is_memoryviewslice:
                 code.put_goto(code.return_from_error_cleanup_label)
 
         # ----- Non-error return cleanup
diff --git a/tests/memoryview/view_return_errors.pyx b/tests/memoryview/view_return_errors.pyx
new file mode 100644 (file)
index 0000000..1099095
--- /dev/null
@@ -0,0 +1,31 @@
+# mode: run
+
+cdef double[:] foo(int i):
+    if i == 1:
+        raise AttributeError('dummy')
+    if i == 2:
+        raise RuntimeError('dummy')
+    if i == 3:
+        raise ValueError('dummy')
+    if i == 4:
+        raise TypeError('dummy')
+
+def propagate(i):
+    '''
+    >>> propagate(0)
+    TypeError('Memoryview return value is not initialized',)
+    >>> propagate(1)
+    AttributeError('dummy',)
+    >>> propagate(2)
+    RuntimeError('dummy',)
+    >>> propagate(3)
+    ValueError('dummy',)
+    >>> propagate(4)
+    TypeError('dummy',)
+    '''
+    try:
+        foo(i)
+    except Exception as e:
+        print repr(e)
+    else:
+        print 'Exception sublass not raised'