Erlend's first review

iritkatriel · Dec 14, 2021 · Nov 11, 2021 · Nov 17, 2021 · Nov 17, 2021 · Nov 18, 2021
commit 798aaa71eac221a198ddb46797979ffe66b20d72
@@ -32,6 +32,19 @@ def test_except_star_ExceptionGroup_is_runtime_error_tuple(self):
             except* (TypeError, ExceptionGroup):
                 pass
 
+    def test_except_star_invalid_exception_type(self):
+        with self.assertRaises(TypeError):
+            try:
+                raise ValueError
+            except* 42:
+                pass
+
+        with self.assertRaises(TypeError):
+            try:
+                raise ValueError
+            except* (ValueError, 42):
+                pass
+
 
 class ExceptStarTest(unittest.TestCase):
     def assertExceptionIsLike(self, exc, template):

@@ -894,9 +894,7 @@ static int
 get_matcher_type(PyObject *value,
                  _exceptiongroup_split_matcher_type *type)
 {
-    if (!value) {
-        goto error;
-    }
+    assert(value);
 
     if (PyFunction_Check(value)) {
         *type = EXCEPTION_GROUP_MATCH_BY_PREDICATE;
@@ -909,7 +907,7 @@ get_matcher_type(PyObject *value,
     }
 
     if (PyTuple_CheckExact(value)) {
-        Py_ssize_t n = PyTuple_Size(value);
+        Py_ssize_t n = PyTuple_GET_SIZE(value);
         for (Py_ssize_t i=0; i<n; i++) {
             if (!PyExceptionClass_Check(PyTuple_GET_ITEM(value, i))) {
                 goto error;
@@ -1156,10 +1154,7 @@ collect_exception_group_leaves(PyObject* exc, PyObject *leaves)
         return 0;
     }
     PyBaseExceptionGroupObject *eg = _PyBaseExceptionGroupObject_cast(exc);
-    Py_ssize_t num_excs = PyTuple_Size(eg->excs);
-    if (num_excs < 0) {
-        return -1;
-    }
+    Py_ssize_t num_excs = PyTuple_GET_SIZE(eg->excs);
     /* recursive calls */
     for (Py_ssize_t i = 0; i < num_excs; i++) {
         PyObject *e = PyTuple_GET_ITEM(eg->excs, i);
@@ -1182,16 +1177,12 @@ _PyExc_ExceptionGroupProjection(PyObject *eg, PyObject *keep)
     assert(_PyBaseExceptionGroup_Check(eg));
     assert(PyList_CheckExact(keep));
 
-    Py_ssize_t n = PyList_Size(keep);
-    if (n == -1) {
-        return NULL;
-    }
-
     PyObject *leaves = PySet_New(NULL);
     if (!leaves) {
         return NULL;
     }
 
+    Py_ssize_t n = PyList_GET_SIZE(keep);
     for (Py_ssize_t i = 0; i < n; i++) {
         PyObject *e = PyList_GET_ITEM(keep, i);
         assert(e != NULL);
@@ -1204,17 +1195,16 @@ _PyExc_ExceptionGroupProjection(PyObject *eg, PyObject *keep)
 
     _exceptiongroup_split_result split_result;
     bool construct_rest = false;
-    if (exceptiongroup_split_recursive(
-            eg, EXCEPTION_GROUP_MATCH_INSTANCES, leaves,
-            construct_rest, &split_result) == -1) {
-        Py_DECREF(leaves);
+    int err = exceptiongroup_split_recursive(
+                eg, EXCEPTION_GROUP_MATCH_INSTANCES, leaves,
+                construct_rest, &split_result);
+    Py_DECREF(leaves);
+    if (err == -1) {
         return NULL;
     }
-    Py_DECREF(leaves);
-    PyObject *result = Py_NewRef(
-            split_result.match ? split_result.match : Py_None);
 
-    Py_XDECREF(split_result.match);
+    PyObject *result = split_result.match ?
+        split_result.match : Py_NewRef(Py_None);
     assert(split_result.rest == NULL);
     return result;
 }

@@ -2591,9 +2591,8 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
             }
             else {
                 // nothing to reraise
-                Py_DECREF(val);
-                PUSH(Py_NewRef(Py_None));
                 PUSH(Py_NewRef(Py_None));
+                PUSH(val);
                 PUSH(Py_NewRef(Py_None));
             }
             DISPATCH();
@@ -3616,13 +3615,13 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
             }
 
             if (match == NULL || rest == NULL) {
-                Py_XDECREF(match);
-                Py_XDECREF(rest);
+                assert(match == NULL);
+                assert(rest == NULL);
                 goto error;
             }
 
             if (match == Py_None) {
-                Py_XDECREF(match);
+                Py_DECREF(match);
                 Py_XDECREF(rest);
                 /* no match - jump to target */
                 JUMPTO(oparg);
@@ -6137,10 +6136,7 @@ do_reraise_star(PyObject *excs, PyObject *orig)
     assert(PyList_Check(excs));
     assert(PyExceptionInstance_Check(orig));
 
-    Py_ssize_t numexcs = PyList_Size(excs);
-    if (numexcs == -1) {
-        return NULL;
-    }
+    Py_ssize_t numexcs = PyList_GET_SIZE(excs);
 
     if (numexcs == 0) {
         return Py_NewRef(Py_None);
@@ -6213,9 +6209,6 @@ do_reraise_star(PyObject *excs, PyObject *orig)
         if (result == NULL) {
             goto done;
         }
-    } else {
-        Py_DECREF(reraised_eg);
-        goto done;
     }
 
 done:
@@ -7120,35 +7113,37 @@ import_all_from(PyThreadState *tstate, PyObject *locals, PyObject *v)
 #define CANNOT_CATCH_MSG "catching classes that do not inherit from "\
                           "BaseException is not allowed"
 
- #define CANNOT_EXCEPT_STAR_EG "catching ExceptionGroup with except* "\
-                               "is not allowed. Use except instead."
-
- static int
- check_except_type_valid(PyThreadState *tstate, PyObject* right) {
-     if (PyTuple_Check(right)) {
-         Py_ssize_t i, length;
-         length = PyTuple_GET_SIZE(right);
-         for (i = 0; i < length; i++) {
-             PyObject *exc = PyTuple_GET_ITEM(right, i);
-             if (!PyExceptionClass_Check(exc)) {
-                 _PyErr_SetString(tstate, PyExc_TypeError,
-                     CANNOT_CATCH_MSG);
-                 return 0;
-             }
-         }
-     }
-     else {
-         if (!PyExceptionClass_Check(right)) {
-             _PyErr_SetString(tstate, PyExc_TypeError,
-                 CANNOT_CATCH_MSG);
-             return 0;
-         }
-     }
-     return 1;
- }
+#define CANNOT_EXCEPT_STAR_EG "catching ExceptionGroup with except* "\
+                          "is not allowed. Use except instead."
 
 static int
-check_except_star_type_valid(PyThreadState *tstate, PyObject* right) {
+check_except_type_valid(PyThreadState *tstate, PyObject* right)
+{
+    if (PyTuple_Check(right)) {
+        Py_ssize_t i, length;
+        length = PyTuple_GET_SIZE(right);
+        for (i = 0; i < length; i++) {
+            PyObject *exc = PyTuple_GET_ITEM(right, i);
+            if (!PyExceptionClass_Check(exc)) {
+                _PyErr_SetString(tstate, PyExc_TypeError,
+                    CANNOT_CATCH_MSG);
+                return 0;
+            }
+        }
+    }
+    else {
+        if (!PyExceptionClass_Check(right)) {
+            _PyErr_SetString(tstate, PyExc_TypeError,
+                CANNOT_CATCH_MSG);
+            return 0;
+        }
+    }
+    return 1;
+}
+
+static int
+check_except_star_type_valid(PyThreadState *tstate, PyObject* right)
+{
     if (!check_except_type_valid(tstate, right)) {
         return 0;
     }