Fix SWIG Python builtin custom exception

[amaeldoe]

When builtin mode is enabled, triggering custom exception object didn't work
and always lead to SWIG_RuntimeError exception.

We can now enable the exception_order_runme test when using builtin.

[wsfulton]

@amaeldoe - did you see the Travis failures? Looks like Python 3.2 fails but 3.3 and 3.4 is okay. Do you know what is different for exception handling for the later versions?

[amaeldoe]

Apparently subclassing (which seems required for Py3 exception) does not work this way. I have no idea why, do you ?

from li_std_except_as_class import *

# This work :
try: test_domain_error()
except domain_error: pass

# This won't :
if not is_python_builtin():
  try: test_domain_error()
  except logic_error: pass
  try: test_domain_error()
  except exception: pass

[amaeldoe]

We apparently need to find some way to derive custom exception object from PyExc_Exception. This could probably be done automatically when SWIG see that a given C++ class derive from std::exception.

However I don't think I know the SWIG internal enough to tackle this issue. Is it something that you could do ?

[wsfulton]

Have you seen the Builtin limitations section at http://swig.org/Doc3.0/Python.html#Python_builtin_limitations? Does deriving the Python class from PyExc_Exception (Python Exception class) result in the same?

There is the 'exceptionclass' feature mentioned in the CHANGES file. Probably we should get this feature working for builtin classes. It looks like it only works for classic Python proxy classes right now though. Look for "feature:exceptionclass" in python.cxx. If it is merely a matter of adding in 'Exception' as one of the bases, it'll be easy to add this in.

[amaeldoe]

Yes, I did : this is a simple limitation, and the correct fix for this is indeed to make sure the generated exception classes wrappers derive from the PyExc_Exception PyTypeObject.

However, my knowledge of SWIG internal is probably a bit limited to tackle this task. Would you be able to look into it ?

[wsfulton]

I can do this, but have other priorities right now. If you make the changes to the generated code by hand and test it, then let me know exactly what needs changing in the generated code, I will code it up. I suggest posting a patch of the required changes to the generated code for say the Examples/python/class example.

[amaeldoe]

Examples/python/class doesn't do any exception handling, so here it is with Examples/python/exception:

To me this is the primary change needed, and it should be done automatically if a classe derive from std::exception

--- Examples/python/exception/example_wrap.cxx  2015-04-28 07:42:26.068165321 +0200
+++ example_wrap.cxx    2015-04-28 07:42:00.786117483 +0200
@@ -5425,7 +5425,7 @@
   SwigPyBuiltin_SetMetaType(builtin_pytype, metatype);
   builtin_pytype->tp_new = PyType_GenericNew;
   builtin_base_count = 0;
-  builtin_bases[builtin_base_count] = NULL;
+  builtin_bases[builtin_base_count++] = (PyTypeObject *) PyExc_Exception;
   SwigPyBuiltin_InitBases(builtin_pytype, builtin_bases);
   PyDict_SetItemString(d, "this", this_descr);
   PyDict_SetItemString(d, "thisown", thisown_descr);

[ojwb]

The Python docs say that "All user-defined exceptions should also be derived from [class Exception]":

https://docs.python.org/2/library/exceptions.html

But in C++, deriving your exceptions from std::exception is allowed, but certainly not a "should" (for example, looking at The C++ Programming Language (3rd edition), none of the examples involving defining exceptions seem to).

This ought to be hooked up to the existing %exceptionclass mechanism, rather than being based on being derived from std::exception.

Perhaps classes derived from std::exception should be implicitly marked as if %exceptionclass is used (like classes used in throws() specifiers apparently are), but that's really a separate point.

[wsfulton]

Sorry, I meant to point you to the Examples/python/exceptproxy example as it is explicitly disabled for builtin. If I comment out:

diff --git a/Examples/python/exceptproxy/runme.py b/Examples/python/exceptproxy/runme.py
index 07e4b0a..c1a5e04 100644
--- a/Examples/python/exceptproxy/runme.py
+++ b/Examples/python/exceptproxy/runme.py
@@ -1,9 +1,9 @@
 # file: runme.py
 import example

-if example.is_python_builtin():
-  print "Skipping example: -builtin option does not support %exceptionclass"
-  exit(0)
+#if example.is_python_builtin():
+#  print "Skipping example: -builtin option does not support %exceptionclass"
+#  exit(0)

then it works with your patch for Python 2, but am surprised as I havn't changed anything wrt deriving from PyExc_Exception. Using Python 3 and deriving from PyExc_Exception seg faults when run, so could take a look and let me know what patch to the generated code in the exceptproxy example is needed please.

[wsfulton]

Agreed that %exceptionclass is needed to get this to work and it could be marked on std::exception if a user provides the declaration for std::exception as it should apply to all derived classes.

[amaeldoe]

Yes, I noticed the same problem when testing the patch more extensively. I unfortunatelly have no time to debug the issue right now, so if any of you can have a look, that would be welcome !

[ojwb]

The patch here needs more work, but stalled in 2015 it seems because nobody knows how to make it work with newer Python 3.x.

There's now issue #1572 tracking the issue this was trying to address, so closing this.

[ojwb]

I just happened upon this closed ticket while searching for something else entirely, and I notice I may have my conclusion wrong above having seemingly reversed the sense of what wsfulton actually said:

Looks like Python 3.2 fails but 3.3 and 3.4 is okay

We've also since dropped support for Python 3.2 (in SWIG 4.1.0) so maybe this patch could now be merged. Hence reopening.

[ojwb]

I rebased and both the testcases pass, but there are still issues with the exception example:

$ SWIG_FEATURES=-builtin make -s
incomplete type <Swig Object of type 'A *' at 0x7fd8e1bb5ff0>
37
I died.
Traceback (most recent call last):
  File "/home/olly/git/swig/Examples/python/exception/runme.py", line 24, in <module>
    t.hosed()
SystemError: _PyErr_SetObject: exception <class 'example.Exc'> is not a BaseException subclass

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/olly/git/swig/Examples/python/exception/runme.py", line 25, in <module>
    except example.Exc as e:
TypeError: catching classes that do not inherit from BaseException is not allowed
make[1]: *** [../../Makefile:1332: python_run] Error 1
make: *** [Makefile:10: check] Error 2

With git master, the output is all on stdout and is:

incomplete type <Swig Object of type 'A *' at 0x7f29af4c7a50>
37
I died.
42 Hosed
37
Bleah!
42 No-go-diggy-die

Is this actually a problem with the example though?

[ojwb]

Is this actually a problem with the example though?

I think it is - adding %exceptionclass Exc; fixes that, and seems like an omission in the current example that you happen to get away with without -builtin.

Sorry, I had lost the SWIG_FEATURES=-builtin after my initial tests. This still fails, and indeed everything still fails. The error is always:

TypeError: catching classes that do not inherit from BaseException is not allowed

I'm using Python 3.11.2.

Looking at example_wrap.cxx for the exception example, it seems %exceptionclass isn't hooked up under -builtin. However fixing that the example now segfaults. The generated code is now:

  /* type '::Exc' */
  builtin_pytype = (PyTypeObject *)&SwigPyBuiltin__Exc_type;
  builtin_pytype->tp_dict = d = PyDict_New();
  SwigPyBuiltin_SetMetaType(builtin_pytype, metatype);
  builtin_pytype->tp_new = PyType_GenericNew;
  builtin_base_count = 0;
  builtin_bases[builtin_base_count++] = (PyTypeObject *) PyExc_Exception;
  builtin_bases[builtin_base_count] = NULL;
  SwigPyBuiltin_InitBases(builtin_pytype, builtin_bases);
  PyDict_SetItemString(d, "this", this_descr);
  PyDict_SetItemString(d, "thisown", thisown_descr);
  if (PyType_Ready(builtin_pytype) < 0) {
    PyErr_SetString(PyExc_TypeError, "Could not create type 'Exc'.");
#if PY_VERSION_HEX >= 0x03000000
    return NULL;
#else
    return;
#endif
  }
  Py_INCREF(builtin_pytype);
  PyModule_AddObject(m, "Exc", (PyObject *)builtin_pytype);
  SwigPyBuiltin_AddPublicSymbol(public_interface, "Exc");
  d = md;

I think I'm out of my depth at this point, but I'll tidy up my branch and push it shortly.

I notice the text in the manual about this limitation makes it sound like the issue is a C/C++ struct layout incompatibility (and PyException_HEAD is still there in Python 3.12, though with slightly different fields at the end).

[ojwb]

It occurred to me to try making SwigPyObject always start with PyException_HEAD when using -builtin as that would address the problem the text in our manual describes.

The downside of this approach is that every SwigPyObject is larger whether it is ever thrown as an exception or not (but only when using -builtin).

In Python 3.3 (oldest we support now) the definition is:

#define PyException_HEAD PyObject_HEAD PyObject *dict;\
             PyObject *args; PyObject *traceback;\
             PyObject *context; PyObject *cause;\
             char suppress_context;

In 3.11 (newest I have installed) it's grown by PyObject *notes;:

#define PyException_HEAD PyObject_HEAD PyObject *dict;\
             PyObject *args; PyObject *notes; PyObject *traceback;\
             PyObject *context; PyObject *cause;\
             char suppress_context;

So (allowing for removing our dict because we can use the one in PyException_HEAD), that adds 4 or 5 PyObject* and one char to the structure. That's probably 20 or 24 bytes on a 32-bit architecture; on x86-64 Linux it's 48 bytes more (growing from 56 to 104 bytes) or 40 bytes more if we reorder the fields to put int own; right after PyException_HEAD.

I suppose doing this could be conditional on %exceptionclass being used at all, and maybe we can have two versions of SwigPyObject (the problem with that would be if we ever need to upcast a PyObject* to SwigPyObject* and only know it's a SWIG-wrapped object, not what type it is; I don't know if that happens or not)

However this change still doesn't seem to make it actually work...