-
-
Notifications
You must be signed in to change notification settings - Fork 31.8k
gh-67377: Document that PyErr_SetString, etc. chain exceptions #20329
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: main
Are you sure you want to change the base?
Conversation
cjerdonek
commented
May 23, 2020
•
edited by AlexWaygood
Loading
edited by AlexWaygood
- Issue: Provide a C helper function to chain raised (but not yet caught) exceptions #67377
a2b0659
to
fc68664
Compare
It is not clear what "caught" mean. The rough equivalent of the Python code: try:
...
except AttributeError:
raises TypeError is the C code: ...
if (result == NULL) {
if (PyErr_ExceptionMatches(PyExc_AttributeError)) {
PyErr_SetNone(PyExc_TypeError);
}
return NULL;
} Is an AttributeError exception caught here? |
The precise condition inside Lines 117 to 119 in be63019
However, this is a private function. So some other ways to say this (without reference to C functions) are (1) if there's an "active exception" (the wording in
Do you have a preference? |
I would say "active exception". Could you make that change? In that case, I can merge this PR unless @serhiy-storchaka has further comments. |
Is the term "active exception" explained anywhere? Should not a link be added? |
The term used by the |
LGTM. @serhiy-storchaka Can you review it? |
Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>
@@ -124,7 +124,10 @@ Raising exceptions | |||
|
|||
These functions help you set the current thread's error indicator. | |||
For convenience, some of these functions will always return a | |||
``NULL`` pointer for use in a ``return`` statement. | |||
``NULL`` pointer for use in a ``return`` statement. Also, for all of the | |||
functions in this subsection, if an exception has already been caught, |
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.
@serhiy-storchaka raised an issue with this terminology, I don't think the previous reviewers' comments have been addressed: #20329 (comment)