-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Expose exception cause as std::nested_exception #4033
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
base: master
Are you sure you want to change the base?
Conversation
if (not cause or cause.is_none()) | ||
return std::nested_exception(); | ||
auto typ = type::of(cause); | ||
auto tb = reinterpret_steal<object>(PyException_GetTraceback(cause.ptr())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PyException_GetTraceback can return null. Will PyErr_Restore work if this is the case?
auto tb = reinterpret_steal<object>(PyException_GetTraceback(cause.ptr())); | ||
PyErr_Restore(typ.release().ptr(), cause.release().ptr(), tb.release().ptr()); | ||
try { | ||
throw error_already_set(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throwing is very slow and something that is best avoided here. is there a way we avoid having to throw the exception just to catch and return an empty std::nested_exception?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't we just use the CPython API to check if the exception is nested instead?
Description
Expose Python exception cause (
raise ... from ...
) to C++ viastd::nested_exception
. Inverse of #3608.Useful after #3605 etc.
Does not set std::nested_exception base if a C++ exception is already present, for backwards compatibility if anyone is already using
std::throw_with_nested(py::error_already_set())
.Suggested changelog entry: