Closed torokati44 closed 1 year ago
Thank you for the review! I have added the Py_DECREF
calls where I deemed necessary.
Perhaps instead the two if (traceback != pytrace)
checks, adding an initial Py_INCREF
on pytrace
would be nicer.
What do you think?
Actually, if the location can be queried through methods newly added by PyException
, then it has to be caught as that type, which requires including CPyCppyy
headers.
This adds some complication for the users of the library.
Would you be open to also appending the location info into the result of what
?
That way it's available even if the exception is caught as just std::exception
.
EDIT: Alternatively, if there is a straightforward/canonical way to query the location of PyException.h
in a deployment (such as, after a simple pip install
), that would also work. This is somewhere in site-packages
usually, but depends on a lot of things. This location is already in cppyy.gbl.gInterpreter.GetIncludePath()
, but having it on its own would be nicer.
Yes, PyException.h
is public and yes, it's added to the Cling include path, but the process of doing so is unfortunately rather involved: it's basically a lot of guessing locations and then stat
-ing whether the file is there. I wish Python package managers did anything straightforward ...
Yes, adding more information to what()
is perfectly fine.
Alright, instead of adding file()
and line()
, the source locaton is now appended to what()
.
I have also added co_name
in there when it's meaningful.
Also, I did the "Py_INCREF
instead of conditional Py_DECREF
s" change I outlined above.
The output of the snippet in https://github.com/wlav/cppyy/issues/106 now starts like this
Exception caught in C++:
----
RuntimeError: Ouch! (at cppyy_exceptionlocation.py:24 in call)
----
polite ping
This is an attempt to address https://github.com/wlav/cppyy/issues/106.