Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad code generated for except +* function #3065

Closed
navytux opened this issue Aug 1, 2019 · 7 comments
Closed

Bad code generated for except +* function #3065

navytux opened this issue Aug 1, 2019 · 7 comments

Comments

@navytux
Copy link
Contributor

navytux commented Aug 1, 2019

Hello up there. Please consider the following example:

---- 8< ---- (y.pyx)

# cython: language_level=2
# distutils: language=c++

cdef extern from *:
    """
    #include <exception>
    void ggg() {
        throw std::bad_alloc();
    }
    """
    void ggg()

cdef void fff() except *:
    raise RuntimeError('aaa')

cdef void hhh() except +*:
    fff()
    ggg()

def aaa():
    hhh()

Explanation:

  • ggg throws C++ exception and is not marked with except + -- thus functions that calls ggg may throw C++ exception themselves;
  • fff raises Python exception but returns void. It thus is marked with except * for its caller to check whether Python exception occurred.
  • hhh calls both fff and ggg. It thus may raise either Python exception (from fff) or C++ exception (from ggg). hhh is thus marked as except +*.

When trying to cythonize y.pyx, it gives:

/home/kirr/tmp/trashme/pyx/y.cpp: In function ‘void __pyx_f_1y_hhh()’:
/home/kirr/tmp/trashme/pyx/y.cpp:1189:3: error: ‘__pyx_r’ was not declared in this scope
   __pyx_r = <Cython.Compiler.ExprNodes.CharNode object at 0x7f8e76e20810>;
   ^~~~~~~
/home/kirr/tmp/trashme/pyx/y.cpp:1189:3: note: suggested alternative: ‘__pyx_f’
   __pyx_r = <Cython.Compiler.ExprNodes.CharNode object at 0x7f8e76e20810>;
   ^~~~~~~
   __pyx_f
/home/kirr/tmp/trashme/pyx/y.cpp:1189:13: error: expected primary-expression before ‘<’ token
   __pyx_r = <Cython.Compiler.ExprNodes.CharNode object at 0x7f8e76e20810>;
             ^
/home/kirr/tmp/trashme/pyx/y.cpp:1189:14: error: ‘Cython’ was not declared in this scope
   __pyx_r = <Cython.Compiler.ExprNodes.CharNode object at 0x7f8e76e20810>;
              ^~~~~~
/home/kirr/tmp/trashme/pyx/y.cpp: In function ‘PyObject* __pyx_pf_1y_aaa(PyObject*)’:
/home/kirr/tmp/trashme/pyx/y.cpp:1229:5: error: ‘__Pyx_CppExn2PyErr’ was not declared in this scope
     __Pyx_CppExn2PyErr();
     ^~~~~~~~~~~~~~~~~~

With the following code generated for hhh:

static void __pyx_f_1y_hhh(void) {
  __Pyx_RefNannyDeclarations
  __Pyx_RefNannySetupContext("hhh", 0);

  /* "y.pyx":17
 * 
 * cdef void hhh() except +*:
 *     fff()             # <<<<<<<<<<<<<<
 *     ggg()
 * 
 */
  __pyx_f_1y_fff(); if (unlikely(PyErr_Occurred())) __PYX_ERR(0, 17, __pyx_L1_error)

  /* "y.pyx":18
 * cdef void hhh() except +*:
 *     fff()
 *     ggg()             # <<<<<<<<<<<<<<
 * 
 * def aaa():
 */
  ggg();

  /* "y.pyx":16
 *     raise RuntimeError('aaa')
 * 
 * cdef void hhh() except +*:             # <<<<<<<<<<<<<<
 *     fff()
 *     ggg()
 */

  /* function exit code */
  goto __pyx_L0;
  __pyx_L1_error:;
  __Pyx_AddTraceback("y.hhh", __pyx_clineno, __pyx_lineno, __pyx_filename);
  __pyx_r = <Cython.Compiler.ExprNodes.CharNode object at 0x7f8e76e20810>;
  __pyx_L0:;
  __Pyx_RefNannyFinishContext();
}

Notice the __pyx_r = <Cython.Compiler.ExprNodes.CharNode object at 0x7f8e76e20810>; line.

Cython version: 0.29.13-514-gac1c9fe47 (today's master).

Thanks beforehand,
Kirill

@navytux
Copy link
Contributor Author

navytux commented Aug 1, 2019

#3064 is different, but probably related.

@maxbachmann
Copy link
Contributor

Running into this as well. My current work around for this issue is the following dummy function:

cdef dummy() except +:
    dummy()

which tricks cython into generating

/* CppExceptionConversion.proto */
#ifndef __Pyx_CppExn2PyErr
#include <new>
#include <typeinfo>
#include <stdexcept>
#include <ios>
static void __Pyx_CppExn2PyErr() {
  try {
    if (PyErr_Occurred())
      ; // let the latest Python exn pass through and ignore the current one
    else
      throw;
  } catch (const std::bad_alloc& exn) {
    PyErr_SetString(PyExc_MemoryError, exn.what());
  } catch (const std::bad_cast& exn) {
    PyErr_SetString(PyExc_TypeError, exn.what());
  } catch (const std::bad_typeid& exn) {
    PyErr_SetString(PyExc_TypeError, exn.what());
  } catch (const std::domain_error& exn) {
    PyErr_SetString(PyExc_ValueError, exn.what());
  } catch (const std::invalid_argument& exn) {
    PyErr_SetString(PyExc_ValueError, exn.what());
  } catch (const std::ios_base::failure& exn) {
    PyErr_SetString(PyExc_IOError, exn.what());
  } catch (const std::out_of_range& exn) {
    PyErr_SetString(PyExc_IndexError, exn.what());
  } catch (const std::overflow_error& exn) {
    PyErr_SetString(PyExc_OverflowError, exn.what());
  } catch (const std::range_error& exn) {
    PyErr_SetString(PyExc_ArithmeticError, exn.what());
  } catch (const std::underflow_error& exn) {
    PyErr_SetString(PyExc_ArithmeticError, exn.what());
  } catch (const std::exception& exn) {
    PyErr_SetString(PyExc_RuntimeError, exn.what());
  }
  catch (...)
  {
    PyErr_SetString(PyExc_RuntimeError, "Unknown exception");
  }
}
#endif

@da-woods
Copy link
Contributor

Looking at the original, only cdef extern functions should be except +. It definitely isn't safe to let C++ exceptions propagate through Cython-generated code. Therefore ggg should be except+ and hhh is except *.

It looks like there's somewhere that doesn't generate the correct utility code though... I'm not sure if that's because the assume that a Cython-defined function can't be except + (in which case we need to enforce it with an error message) or if the issue is elsewhere

@maxbachmann
Copy link
Contributor

maxbachmann commented Feb 21, 2021

In my case I have C++ functions, that can throw exceptions or have an error set from e.g. PyUnicode_READY.

cdef extern from "example.hpp":
    double func1(...) except +*
    double func2() except +*

it was also not enough to add a dummy function in the C++ code

cdef extern from "example.hpp":
    double func1(...) except +*
    double func2() except +*
    dummy except +

apparently it only generates the code when the function is actually called

@da-woods
Copy link
Contributor

da-woods commented Feb 21, 2021

Thanks @maxbachmann - that's a useful clarification

da-woods added a commit to da-woods/cython that referenced this issue Feb 21, 2021
@scoder scoder added this to the 3.0 milestone May 24, 2021
@scoder scoder added the C++ label Jun 17, 2021
scoder pushed a commit that referenced this issue Jun 17, 2021
Fixes issue #3065
Fixes issue #3066

* delete cpp_exception_declaration_compatibility test
  Because I'm emitting an error at an earlier stage I don't think there's a way to get this test to work.
@da-woods
Copy link
Contributor

bba6905

@navytux
Copy link
Contributor Author

navytux commented Jun 23, 2021

Thanks, @da-woods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants