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 · web-flow · commit 65e7c1f90e91 · 2022-01-02T23:22:42.000Z
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1 @@
+:opcode:`PREP_RERAISE_STAR` no longer pushes ``lasti`` to the stack.
diff --git 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
@@ -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,16 +2776,14 @@ _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);
 
             if (val == NULL) {
                 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

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+:opcode:`PREP_RERAISE_STAR` no longer pushes ``lasti`` to the stack.