-
-
Notifications
You must be signed in to change notification settings - Fork 33.5k
gh-141732: Fix ExceptionGroup repr changing when original exception sequence is mutated
#141736
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?
Changes from 25 commits
b1afcb8
59bc3c7
ca2d21d
493ab01
cd90d51
177b535
af4f086
72918ea
b45402a
62d72a4
f93ae8f
56eefee
79dd615
e9fec3d
f5acfef
2fca592
4885a6b
4801399
1bcddeb
ccef7c5
9ca573e
9531fe9
1da9238
fcaf314
2c67e71
d962581
ce39eec
9211d01
737206a
8d17ef3
383af2f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Ensure the repr for :exc:`ExceptionGroup` and :exc:`BaseExceptionGroup` does | ||
dr-carlos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| not change when the exception sequence that was original passed in to its constructor is subsequently mutated. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -694,12 +694,12 @@ PyTypeObject _PyExc_ ## EXCNAME = { \ | |
|
|
||
| #define ComplexExtendsException(EXCBASE, EXCNAME, EXCSTORE, EXCNEW, \ | ||
| EXCMETHODS, EXCMEMBERS, EXCGETSET, \ | ||
| EXCSTR, EXCDOC) \ | ||
| EXCSTR, EXCREPR, EXCDOC) \ | ||
| static PyTypeObject _PyExc_ ## EXCNAME = { \ | ||
| PyVarObject_HEAD_INIT(NULL, 0) \ | ||
| # EXCNAME, \ | ||
| sizeof(Py ## EXCSTORE ## Object), 0, \ | ||
| EXCSTORE ## _dealloc, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, \ | ||
| EXCSTORE ## _dealloc, 0, 0, 0, 0, EXCREPR, 0, 0, 0, 0, 0, \ | ||
| EXCSTR, 0, 0, 0, \ | ||
| Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HAVE_GC, \ | ||
| PyDoc_STR(EXCDOC), EXCSTORE ## _traverse, \ | ||
|
|
@@ -792,7 +792,7 @@ StopIteration_traverse(PyObject *op, visitproc visit, void *arg) | |
| } | ||
|
|
||
| ComplexExtendsException(PyExc_Exception, StopIteration, StopIteration, | ||
| 0, 0, StopIteration_members, 0, 0, | ||
| 0, 0, StopIteration_members, 0, 0, 0, | ||
| "Signal the end from iterator.__next__()."); | ||
|
|
||
|
|
||
|
|
@@ -865,7 +865,7 @@ static PyMemberDef SystemExit_members[] = { | |
| }; | ||
|
|
||
| ComplexExtendsException(PyExc_BaseException, SystemExit, SystemExit, | ||
| 0, 0, SystemExit_members, 0, 0, | ||
| 0, 0, SystemExit_members, 0, 0, 0, | ||
| "Request to exit from the interpreter."); | ||
|
|
||
| /* | ||
|
|
@@ -890,6 +890,7 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds) | |
|
|
||
| PyObject *message = NULL; | ||
| PyObject *exceptions = NULL; | ||
| PyObject *exceptions_str = NULL; | ||
|
|
||
| if (!PyArg_ParseTuple(args, | ||
| "UO:BaseExceptionGroup.__new__", | ||
|
|
@@ -905,6 +906,11 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds) | |
| return NULL; | ||
| } | ||
|
|
||
| /* Save initial exceptions sequence as a string incase sequence is mutated */ | ||
dr-carlos marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| if (!PyList_Check(exceptions) && !PyTuple_Check(exceptions)) { | ||
| exceptions_str = PyObject_Repr(exceptions); | ||
dr-carlos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
dr-carlos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| exceptions = PySequence_Tuple(exceptions); | ||
| if (!exceptions) { | ||
| return NULL; | ||
|
|
@@ -988,9 +994,11 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds) | |
|
|
||
| self->msg = Py_NewRef(message); | ||
| self->excs = exceptions; | ||
| self->excs_str = exceptions_str; | ||
| return (PyObject*)self; | ||
| error: | ||
| Py_DECREF(exceptions); | ||
| Py_XDECREF(exceptions_str); | ||
| return NULL; | ||
| } | ||
|
|
||
|
|
@@ -1029,6 +1037,7 @@ BaseExceptionGroup_clear(PyObject *op) | |
| PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op); | ||
| Py_CLEAR(self->msg); | ||
| Py_CLEAR(self->excs); | ||
| Py_CLEAR(self->excs_str); | ||
| return BaseException_clear(op); | ||
| } | ||
|
|
||
|
|
@@ -1046,6 +1055,7 @@ BaseExceptionGroup_traverse(PyObject *op, visitproc visit, void *arg) | |
| PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op); | ||
| Py_VISIT(self->msg); | ||
| Py_VISIT(self->excs); | ||
| Py_VISIT(self->excs_str); | ||
| return BaseException_traverse(op, visit, arg); | ||
| } | ||
|
|
||
|
|
@@ -1063,6 +1073,42 @@ BaseExceptionGroup_str(PyObject *op) | |
| self->msg, num_excs, num_excs > 1 ? "s" : ""); | ||
| } | ||
|
|
||
| static PyObject * | ||
| BaseExceptionGroup_repr(PyObject *op) | ||
| { | ||
| PyBaseExceptionGroupObject *self = PyBaseExceptionGroupObject_CAST(op); | ||
| assert(self->msg); | ||
|
|
||
| PyObject *exceptions_str = Py_XNewRef(self->excs_str); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Instead: PyObject *exception_str = NULL;
if (!self->excs_str) {
exception_str = ...
}
else {
exception_str = Py_NewRef(self->excs_str);
}
assert(exception_str != NULL);
/* format */
Py_DECREF(exception_str);It's not faster, but I think it's clearer to the reader. Up to you to make the change though.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wasn't sure so I attempted to clarify with a comment instead. Happy to do something different if that's preferable :)
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem wasn't the comment but rather the fact that we are using |
||
|
|
||
| /* If the initial exceptions string was not saved in the constructor. */ | ||
| if (!exceptions_str) { | ||
| assert(self->excs); | ||
|
|
||
| /* Older versions of this code delegated to BaseException's repr, inserting | ||
| * the current value of self.args[1]. However, mutating that sequence makes | ||
| * the repr appear as if the ExceptionGroup itself has changed, which it hasn't. | ||
| * So we use the actual exceptions tuple for accuracy, but make it look like the | ||
| * original exception sequence if possible, for backwards compatibility. */ | ||
| if (PyList_Check(PyTuple_GET_ITEM(self->args, 1))) { | ||
| PyObject *exceptions_list = PySequence_List(self->excs); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to check the result of
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Because the errors are not only on the type. |
||
| exceptions_str = PyObject_Repr(exceptions_list); | ||
dr-carlos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Py_DECREF(exceptions_list); | ||
| } | ||
| else { | ||
| exceptions_str = PyObject_Repr(self->excs); | ||
dr-carlos marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| } | ||
| } | ||
|
|
||
| const char *name = _PyType_Name(Py_TYPE(self)); | ||
| PyObject *repr = PyUnicode_FromFormat( | ||
| "%s(%R, %U)", name, | ||
| self->msg, exceptions_str); | ||
|
|
||
| Py_DECREF(exceptions_str); | ||
| return repr; | ||
| } | ||
|
|
||
| /*[clinic input] | ||
| @critical_section | ||
| BaseExceptionGroup.derive | ||
|
|
@@ -1682,6 +1728,8 @@ static PyMemberDef BaseExceptionGroup_members[] = { | |
| PyDoc_STR("exception message")}, | ||
| {"exceptions", _Py_T_OBJECT, offsetof(PyBaseExceptionGroupObject, excs), Py_READONLY, | ||
| PyDoc_STR("nested exceptions")}, | ||
| {"_exceptions_str", _Py_T_OBJECT, offsetof(PyBaseExceptionGroupObject, excs_str), Py_READONLY, | ||
| PyDoc_STR("private string representation of initial exceptions sequence")}, | ||
| {NULL} /* Sentinel */ | ||
| }; | ||
|
|
||
|
|
@@ -1697,7 +1745,7 @@ static PyMethodDef BaseExceptionGroup_methods[] = { | |
| ComplexExtendsException(PyExc_BaseException, BaseExceptionGroup, | ||
| BaseExceptionGroup, BaseExceptionGroup_new /* new */, | ||
| BaseExceptionGroup_methods, BaseExceptionGroup_members, | ||
| 0 /* getset */, BaseExceptionGroup_str, | ||
| 0 /* getset */, BaseExceptionGroup_str, BaseExceptionGroup_repr, | ||
| "A combination of multiple unrelated exceptions."); | ||
|
|
||
| /* | ||
|
|
@@ -2425,7 +2473,7 @@ static PyGetSetDef OSError_getset[] = { | |
| ComplexExtendsException(PyExc_Exception, OSError, | ||
| OSError, OSError_new, | ||
| OSError_methods, OSError_members, OSError_getset, | ||
| OSError_str, | ||
| OSError_str, 0, | ||
| "Base class for I/O related errors."); | ||
|
|
||
|
|
||
|
|
@@ -2566,7 +2614,7 @@ static PyMethodDef NameError_methods[] = { | |
| ComplexExtendsException(PyExc_Exception, NameError, | ||
| NameError, 0, | ||
| NameError_methods, NameError_members, | ||
| 0, BaseException_str, "Name not found globally."); | ||
| 0, BaseException_str, 0, "Name not found globally."); | ||
|
|
||
| /* | ||
| * UnboundLocalError extends NameError | ||
|
|
@@ -2700,7 +2748,7 @@ static PyMethodDef AttributeError_methods[] = { | |
| ComplexExtendsException(PyExc_Exception, AttributeError, | ||
| AttributeError, 0, | ||
| AttributeError_methods, AttributeError_members, | ||
| 0, BaseException_str, "Attribute not found."); | ||
| 0, BaseException_str, 0, "Attribute not found."); | ||
|
|
||
| /* | ||
| * SyntaxError extends Exception | ||
|
|
@@ -2899,7 +2947,7 @@ static PyMemberDef SyntaxError_members[] = { | |
|
|
||
| ComplexExtendsException(PyExc_Exception, SyntaxError, SyntaxError, | ||
| 0, 0, SyntaxError_members, 0, | ||
| SyntaxError_str, "Invalid syntax."); | ||
| SyntaxError_str, 0, "Invalid syntax."); | ||
|
|
||
|
|
||
| /* | ||
|
|
@@ -2959,7 +3007,7 @@ KeyError_str(PyObject *op) | |
| } | ||
|
|
||
| ComplexExtendsException(PyExc_LookupError, KeyError, BaseException, | ||
| 0, 0, 0, 0, KeyError_str, "Mapping key not found."); | ||
| 0, 0, 0, 0, KeyError_str, 0, "Mapping key not found."); | ||
|
|
||
|
|
||
| /* | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.