[Python-checkins] bpo-46219, 46221: simplify except* implementation following exc_info changes. Move helpers to exceptions.c. Do not assume that exception groups are truthy. (GH-30289)
iritkatriel
webhook-mailer at python.org
Sun Jan 2 18:22:47 EST 2022
https://github.com/python/cpython/commit/65e7c1f90e9136fc61f4af029b065d9f6c5664c3
commit: 65e7c1f90e9136fc61f4af029b065d9f6c5664c3
branch: main
author: Irit Katriel <1055913+iritkatriel at users.noreply.github.com>
committer: iritkatriel <1055913+iritkatriel at users.noreply.github.com>
date: 2022-01-02T23:22:42Z
summary:
bpo-46219, 46221: simplify except* implementation following exc_info changes. Move helpers to exceptions.c. Do not assume that exception groups are truthy. (GH-30289)
files:
A Misc/NEWS.d/next/Core and Builtins/2022-01-01-14-23-57.bpo-46221.7oGp-I.rst
M .github/CODEOWNERS
M Doc/library/dis.rst
M Include/internal/pycore_pyerrors.h
M Lib/importlib/_bootstrap_external.py
M Lib/test/test_except_star.py
M Objects/exceptions.c
M Python/ceval.c
M Python/compile.c
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
index ce5121e7ac8f8..f484664f7b712 100644
--- a/.github/CODEOWNERS
+++ b/.github/CODEOWNERS
@@ -21,6 +21,14 @@ Python/ceval.c @markshannon
Python/compile.c @markshannon
Python/ast_opt.c @isidentical
+# Exceptions
+Lib/traceback.py @iritkatriel
+Lib/test/test_except*.py @iritkatriel
+Lib/test/test_traceback.py @iritkatriel
+Objects/exceptions.c @iritkatriel
+Python/traceback.c @iritkatriel
+Python/pythonrun.c @iritkatriel
+
# Hashing
**/*hashlib* @python/crypto-team @tiran
**/*pyhash* @python/crypto-team @tiran
diff --git a/Doc/library/dis.rst b/Doc/library/dis.rst
index ffade3c9bfe7c..14de191265cf2 100644
--- a/Doc/library/dis.rst
+++ b/Doc/library/dis.rst
@@ -911,8 +911,8 @@ All of the following opcodes use their arguments.
Combines the raised and reraised exceptions list from TOS, into an exception
group to propagate from a try-except* block. Uses the original exception
group from TOS1 to reconstruct the structure of reraised exceptions. Pops
- two items from the stack and pushes 0 (for lasti, which is unused) followed
- by the exception to reraise or ``None`` if there isn't one.
+ two items from the stack and pushes the exception to reraise or ``None``
+ if there isn't one.
.. versionadded:: 3.11
diff --git a/Include/internal/pycore_pyerrors.h b/Include/internal/pycore_pyerrors.h
index b9fc36cf06760..f375337a405bb 100644
--- a/Include/internal/pycore_pyerrors.h
+++ b/Include/internal/pycore_pyerrors.h
@@ -87,9 +87,9 @@ PyAPI_FUNC(PyObject *) _PyExc_CreateExceptionGroup(
const char *msg,
PyObject *excs);
-PyAPI_FUNC(PyObject *) _PyExc_ExceptionGroupProjection(
- PyObject *left,
- PyObject *right);
+PyAPI_FUNC(PyObject *) _PyExc_PrepReraiseStar(
+ PyObject *orig,
+ PyObject *excs);
PyAPI_FUNC(int) _PyErr_CheckSignalsTstate(PyThreadState *tstate);
diff --git a/Lib/importlib/_bootstrap_external.py b/Lib/importlib/_bootstrap_external.py
index 5ead6caf9f3c7..095c1274bebaf 100644
--- a/Lib/importlib/_bootstrap_external.py
+++ b/Lib/importlib/_bootstrap_external.py
@@ -375,6 +375,7 @@ def _write_atomic(path, data, mode=0o666):
# Python 3.11a4 3467 (Change CALL_xxx opcodes)
# Python 3.11a4 3468 (Add SEND opcode)
# Python 3.11a4 3469 (bpo-45711: remove type, traceback from exc_info)
+# Python 3.11a4 3470 (bpo-46221: PREP_RERAISE_STAR no longer pushes lasti)
#
# MAGIC must change whenever the bytecode emitted by the compiler may no
@@ -384,7 +385,7 @@ def _write_atomic(path, data, mode=0o666):
# Whenever MAGIC_NUMBER is changed, the ranges in the magic_values array
# in PC/launcher.c must also be updated.
-MAGIC_NUMBER = (3469).to_bytes(2, 'little') + b'\r\n'
+MAGIC_NUMBER = (3470).to_bytes(2, 'little') + b'\r\n'
_RAW_MAGIC_NUMBER = int.from_bytes(MAGIC_NUMBER, 'little') # For import.c
_PYCACHE = '__pycache__'
diff --git a/Lib/test/test_except_star.py b/Lib/test/test_except_star.py
index b03de9c1de32e..490b159e3b71b 100644
--- a/Lib/test/test_except_star.py
+++ b/Lib/test/test_except_star.py
@@ -952,6 +952,34 @@ def derive(self, excs):
self.assertEqual(teg.code, 42)
self.assertEqual(teg.exceptions[0].code, 101)
+ def test_falsy_exception_group_subclass(self):
+ class FalsyEG(ExceptionGroup):
+ def __bool__(self):
+ return False
+
+ def derive(self, excs):
+ return FalsyEG(self.message, excs)
+
+ try:
+ try:
+ raise FalsyEG("eg", [TypeError(1), ValueError(2)])
+ except *TypeError as e:
+ tes = e
+ raise
+ except *ValueError as e:
+ ves = e
+ pass
+ except Exception as e:
+ exc = e
+
+ for e in [tes, ves, exc]:
+ self.assertFalse(e)
+ self.assertIsInstance(e, FalsyEG)
+
+ self.assertExceptionIsLike(exc, FalsyEG("eg", [TypeError(1)]))
+ self.assertExceptionIsLike(tes, FalsyEG("eg", [TypeError(1)]))
+ self.assertExceptionIsLike(ves, FalsyEG("eg", [ValueError(2)]))
+
class TestExceptStarCleanup(ExceptStarTest):
def test_exc_info_restored(self):
diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-01-01-14-23-57.bpo-46221.7oGp-I.rst b/Misc/NEWS.d/next/Core and Builtins/2022-01-01-14-23-57.bpo-46221.7oGp-I.rst
new file mode 100644
index 0000000000000..0cb3e90a28d75
--- /dev/null
+++ b/Misc/NEWS.d/next/Core and Builtins/2022-01-01-14-23-57.bpo-46221.7oGp-I.rst
@@ -0,0 +1 @@
+:opcode:`PREP_RERAISE_STAR` no longer pushes ``lasti`` to the stack.
diff --git a/Objects/exceptions.c b/Objects/exceptions.c
index d82340b8e78ab..403d2d4a3fddf 100644
--- a/Objects/exceptions.c
+++ b/Objects/exceptions.c
@@ -1207,8 +1207,8 @@ collect_exception_group_leaves(PyObject *exc, PyObject *leaves)
* of eg which contains all leaf exceptions that are contained
* in any exception group in keep.
*/
-PyObject *
-_PyExc_ExceptionGroupProjection(PyObject *eg, PyObject *keep)
+static PyObject *
+exception_group_projection(PyObject *eg, PyObject *keep)
{
assert(_PyBaseExceptionGroup_Check(eg));
assert(PyList_CheckExact(keep));
@@ -1245,6 +1245,122 @@ _PyExc_ExceptionGroupProjection(PyObject *eg, PyObject *keep)
return result;
}
+static bool
+is_same_exception_metadata(PyObject *exc1, PyObject *exc2)
+{
+ assert(PyExceptionInstance_Check(exc1));
+ assert(PyExceptionInstance_Check(exc2));
+
+ PyBaseExceptionObject *e1 = (PyBaseExceptionObject *)exc1;
+ PyBaseExceptionObject *e2 = (PyBaseExceptionObject *)exc2;
+
+ return (e1->note == e2->note &&
+ e1->traceback == e2->traceback &&
+ e1->cause == e2->cause &&
+ e1->context == e2->context);
+}
+
+/*
+ This function is used by the interpreter to calculate
+ the exception group to be raised at the end of a
+ try-except* construct.
+
+ orig: the original except that was caught.
+ excs: a list of exceptions that were raised/reraised
+ in the except* clauses.
+
+ Calculates an exception group to raise. It contains
+ all exceptions in excs, where those that were reraised
+ have same nesting structure as in orig, and those that
+ were raised (if any) are added as siblings in a new EG.
+
+ Returns NULL and sets an exception on failure.
+*/
+PyObject *
+_PyExc_PrepReraiseStar(PyObject *orig, PyObject *excs)
+{
+ assert(PyExceptionInstance_Check(orig));
+ assert(PyList_Check(excs));
+
+ Py_ssize_t numexcs = PyList_GET_SIZE(excs);
+
+ if (numexcs == 0) {
+ return Py_NewRef(Py_None);
+ }
+
+ if (!_PyBaseExceptionGroup_Check(orig)) {
+ /* a naked exception was caught and wrapped. Only one except* clause
+ * could have executed,so there is at most one exception to raise.
+ */
+
+ assert(numexcs == 1 || (numexcs == 2 && PyList_GET_ITEM(excs, 1) == Py_None));
+
+ PyObject *e = PyList_GET_ITEM(excs, 0);
+ assert(e != NULL);
+ return Py_NewRef(e);
+ }
+
+ PyObject *raised_list = PyList_New(0);
+ if (raised_list == NULL) {
+ return NULL;
+ }
+ PyObject* reraised_list = PyList_New(0);
+ if (reraised_list == NULL) {
+ Py_DECREF(raised_list);
+ return NULL;
+ }
+
+ /* Now we are holding refs to raised_list and reraised_list */
+
+ PyObject *result = NULL;
+
+ /* Split excs into raised and reraised by comparing metadata with orig */
+ for (Py_ssize_t i = 0; i < numexcs; i++) {
+ PyObject *e = PyList_GET_ITEM(excs, i);
+ assert(e != NULL);
+ if (Py_IsNone(e)) {
+ continue;
+ }
+ bool is_reraise = is_same_exception_metadata(e, orig);
+ PyObject *append_list = is_reraise ? reraised_list : raised_list;
+ if (PyList_Append(append_list, e) < 0) {
+ goto done;
+ }
+ }
+
+ PyObject *reraised_eg = exception_group_projection(orig, reraised_list);
+ if (reraised_eg == NULL) {
+ goto done;
+ }
+
+ if (!Py_IsNone(reraised_eg)) {
+ assert(is_same_exception_metadata(reraised_eg, orig));
+ }
+ Py_ssize_t num_raised = PyList_GET_SIZE(raised_list);
+ if (num_raised == 0) {
+ result = reraised_eg;
+ }
+ else if (num_raised > 0) {
+ int res = 0;
+ if (!Py_IsNone(reraised_eg)) {
+ res = PyList_Append(raised_list, reraised_eg);
+ }
+ Py_DECREF(reraised_eg);
+ if (res < 0) {
+ goto done;
+ }
+ result = _PyExc_CreateExceptionGroup("", raised_list);
+ if (result == NULL) {
+ goto done;
+ }
+ }
+
+done:
+ Py_XDECREF(raised_list);
+ Py_XDECREF(reraised_list);
+ return result;
+}
+
static PyMemberDef BaseExceptionGroup_members[] = {
{"message", T_OBJECT, offsetof(PyBaseExceptionGroupObject, msg), READONLY,
PyDoc_STR("exception message")},
diff --git a/Python/ceval.c b/Python/ceval.c
index 9976bdeffbe96..43925e6db269c 100644
--- a/Python/ceval.c
+++ b/Python/ceval.c
@@ -1092,7 +1092,6 @@ match_class(PyThreadState *tstate, PyObject *subject, PyObject *type,
static int do_raise(PyThreadState *tstate, PyObject *exc, PyObject *cause);
-static PyObject *do_reraise_star(PyObject *excs, PyObject *orig);
static int exception_group_match(
PyObject* exc_value, PyObject *match_type,
PyObject **match, PyObject **rest);
@@ -2777,7 +2776,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
assert(PyList_Check(excs));
PyObject *orig = POP();
- PyObject *val = do_reraise_star(excs, orig);
+ PyObject *val = _PyExc_PrepReraiseStar(orig, excs);
Py_DECREF(excs);
Py_DECREF(orig);
@@ -2785,8 +2784,6 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
goto error;
}
- PyObject *lasti_unused = Py_NewRef(_PyLong_GetZero());
- PUSH(lasti_unused);
PUSH(val);
DISPATCH();
}
@@ -6313,134 +6310,6 @@ exception_group_match(PyObject* exc_value, PyObject *match_type,
return 0;
}
-/* Logic for the final raise/reraise of a try-except* contruct
- (too complicated for inlining).
-*/
-
-static bool
-is_same_exception_metadata(PyObject *exc1, PyObject *exc2)
-{
- assert(PyExceptionInstance_Check(exc1));
- assert(PyExceptionInstance_Check(exc2));
-
- PyObject *tb1 = PyException_GetTraceback(exc1);
- PyObject *ctx1 = PyException_GetContext(exc1);
- PyObject *cause1 = PyException_GetCause(exc1);
- PyObject *tb2 = PyException_GetTraceback(exc2);
- PyObject *ctx2 = PyException_GetContext(exc2);
- PyObject *cause2 = PyException_GetCause(exc2);
-
- bool result = (Py_Is(tb1, tb2) &&
- Py_Is(ctx1, ctx2) &&
- Py_Is(cause1, cause2));
-
- Py_XDECREF(tb1);
- Py_XDECREF(ctx1);
- Py_XDECREF(cause1);
- Py_XDECREF(tb2);
- Py_XDECREF(ctx2);
- Py_XDECREF(cause2);
- return result;
-}
-
-/*
- excs: a list of exceptions to raise/reraise
- orig: the original except that was caught
-
- Calculates an exception group to raise. It contains
- all exceptions in excs, where those that were reraised
- have same nesting structure as in orig, and those that
- were raised (if any) are added as siblings in a new EG.
-
- Returns NULL and sets an exception on failure.
-*/
-static PyObject *
-do_reraise_star(PyObject *excs, PyObject *orig)
-{
- assert(PyList_Check(excs));
- assert(PyExceptionInstance_Check(orig));
-
- Py_ssize_t numexcs = PyList_GET_SIZE(excs);
-
- if (numexcs == 0) {
- return Py_NewRef(Py_None);
- }
-
- if (!_PyBaseExceptionGroup_Check(orig)) {
- /* a naked exception was caught and wrapped. Only one except* clause
- * could have executed,so there is at most one exception to raise.
- */
-
- assert(numexcs == 1 || (numexcs == 2 && PyList_GET_ITEM(excs, 1) == Py_None));
-
- PyObject *e = PyList_GET_ITEM(excs, 0);
- assert(e != NULL);
- return Py_NewRef(e);
- }
-
-
- PyObject *raised_list = PyList_New(0);
- if (raised_list == NULL) {
- return NULL;
- }
- PyObject* reraised_list = PyList_New(0);
- if (reraised_list == NULL) {
- Py_DECREF(raised_list);
- return NULL;
- }
-
- /* Now we are holding refs to raised_list and reraised_list */
-
- PyObject *result = NULL;
-
- /* Split excs into raised and reraised by comparing metadata with orig */
- for (Py_ssize_t i = 0; i < numexcs; i++) {
- PyObject *e = PyList_GET_ITEM(excs, i);
- assert(e != NULL);
- if (Py_IsNone(e)) {
- continue;
- }
- bool is_reraise = is_same_exception_metadata(e, orig);
- PyObject *append_list = is_reraise ? reraised_list : raised_list;
- if (PyList_Append(append_list, e) < 0) {
- goto done;
- }
- }
-
- PyObject *reraised_eg = _PyExc_ExceptionGroupProjection(orig, reraised_list);
- if (reraised_eg == NULL) {
- goto done;
- }
-
- if (!Py_IsNone(reraised_eg)) {
- assert(is_same_exception_metadata(reraised_eg, orig));
- }
-
- Py_ssize_t num_raised = PyList_GET_SIZE(raised_list);
- if (num_raised == 0) {
- result = reraised_eg;
- }
- else if (num_raised > 0) {
- int res = 0;
- if (!Py_IsNone(reraised_eg)) {
- res = PyList_Append(raised_list, reraised_eg);
- }
- Py_DECREF(reraised_eg);
- if (res < 0) {
- goto done;
- }
- result = _PyExc_CreateExceptionGroup("", raised_list);
- if (result == NULL) {
- goto done;
- }
- }
-
-done:
- Py_XDECREF(raised_list);
- Py_XDECREF(reraised_list);
- return result;
-}
-
/* Iterate v argcnt times and store the results on the stack (via decreasing
sp). Return 1 for success, 0 if error.
diff --git a/Python/compile.c b/Python/compile.c
index 8e90b1ab37037..48250b5dba973 100644
--- a/Python/compile.c
+++ b/Python/compile.c
@@ -1134,7 +1134,7 @@ stack_effect(int opcode, int oparg, int jump)
return jump ? 1 : 0;
case PREP_RERAISE_STAR:
- return 0;
+ return -1;
case RERAISE:
return -1;
case PUSH_EXC_INFO:
@@ -3488,12 +3488,16 @@ compiler_try_except(struct compiler *c, stmt_ty s)
[orig, res, rest] Ln+1: LIST_APPEND 1 ) add unhandled exc to res (could be None)
[orig, res] PREP_RERAISE_STAR
- [i, exc] POP_JUMP_IF_TRUE RER
- [i, exc] POP
- [i] POP
+ [exc] DUP_TOP
+ [exc, exc] LOAD_CONST None
+ [exc, exc, None] COMPARE_IS
+ [exc, is_none] POP_JUMP_IF_FALSE RER
+ [exc] POP_TOP
[] JUMP_FORWARD L0
- [i, exc] RER: POP_EXCEPT_AND_RERAISE
+ [exc] RER: ROT_TWO
+ [exc, prev_exc_info] POP_EXCEPT
+ [exc] RERAISE 0
[] L0: <next statement>
*/
@@ -3657,18 +3661,21 @@ compiler_try_star_except(struct compiler *c, stmt_ty s)
compiler_use_next_block(c, reraise_star);
ADDOP(c, PREP_RERAISE_STAR);
ADDOP(c, DUP_TOP);
- ADDOP_JUMP(c, POP_JUMP_IF_TRUE, reraise);
+ ADDOP_LOAD_CONST(c, Py_None);
+ ADDOP_COMPARE(c, Is);
+ ADDOP_JUMP(c, POP_JUMP_IF_FALSE, reraise);
NEXT_BLOCK(c);
- /* Nothing to reraise - pop it */
- ADDOP(c, POP_TOP);
+ /* Nothing to reraise */
ADDOP(c, POP_TOP);
ADDOP(c, POP_BLOCK);
ADDOP(c, POP_EXCEPT);
ADDOP_JUMP(c, JUMP_FORWARD, end);
compiler_use_next_block(c, reraise);
ADDOP(c, POP_BLOCK);
- ADDOP(c, POP_EXCEPT_AND_RERAISE);
+ ADDOP(c, ROT_TWO);
+ ADDOP(c, POP_EXCEPT);
+ ADDOP_I(c, RERAISE, 0);
compiler_use_next_block(c, cleanup);
ADDOP(c, POP_EXCEPT_AND_RERAISE);
compiler_use_next_block(c, orelse);
More information about the Python-checkins
mailing list