Apply first batch of suggestions from code review

Thank you Erlend! Co-authored-by: Erlend Egeberg Aasland <erlend.aasland@innova.no>
iritkatriel · Dec 14, 2021 · Nov 11, 2021 · Nov 17, 2021 · Nov 17, 2021 · Nov 18, 2021
commit 035fe5cf08a58b362bbd79ba02f3de80fc032582
@@ -974,4 +974,3 @@ def test_exc_info_restored(self):
 
 if __name__ == '__main__':
     unittest.main()
-
@@ -774,8 +774,7 @@ BaseExceptionGroup_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
 }
 
 PyObject *
-_PyExc_CreateExceptionGroup(
-    const char *msg_str, PyObject *excs)
+_PyExc_CreateExceptionGroup(const char *msg_str, PyObject *excs)
 {
     PyObject *msg = PyUnicode_FromString(msg_str);
     if (!msg) {
@@ -786,8 +785,7 @@ _PyExc_CreateExceptionGroup(
     if (!args) {
         return NULL;
     }
-    PyObject *result = PyObject_CallObject(
-        PyExc_BaseExceptionGroup, args);
+    PyObject *result = PyObject_CallObject(PyExc_BaseExceptionGroup, args);
     Py_DECREF(args);
     return result;
 }
@@ -979,12 +977,8 @@ exceptiongroup_split_check_match(PyObject *exc,
     }
     case EXCEPTION_GROUP_MATCH_INSTANCES: {
         assert(PySet_Check(matcher_value));
-        if (! _PyBaseExceptionGroup_Check(exc)) {
-            int include = PySet_Contains(matcher_value, exc);
-            if (include == -1) {
-                return -1;
-            }
-            return include;
+        if (!_PyBaseExceptionGroup_Check(exc)) {
+            return PySet_Contains(matcher_value, exc);
         }
         return 0;
     }
@@ -1168,9 +1162,9 @@ BaseExceptionGroup_subgroup(PyObject *self, PyObject *args)
 }
 
 static int
-collect_exception_group_leaves(PyObject* exc, PyObject *leaves)
+collect_exception_group_leaves(PyObject *exc, PyObject *leaves)
 {
-    if (exc == Py_None) {
+    if (Py_IsNone(exc)) {
         return 0;
     }
 
@@ -1180,7 +1174,7 @@ collect_exception_group_leaves(PyObject* exc, PyObject *leaves)
     /* Add all leaf exceptions in exc to the leaves set */
 
     if (!_PyBaseExceptionGroup_Check(exc)) {
-        if (PySet_Add(leaves, exc) == -1) {
+        if (PySet_Add(leaves, exc) < 0) {
             return -1;
         }
         return 0;
@@ -1219,7 +1213,7 @@ _PyExc_ExceptionGroupProjection(PyObject *eg, PyObject *keep)
         PyObject *e = PyList_GET_ITEM(keep, i);
         assert(e != NULL);
         assert(_PyBaseExceptionGroup_Check(e));
-        if (collect_exception_group_leaves(e, leaves) == -1) {
+        if (collect_exception_group_leaves(e, leaves) < 0) {
             Py_DECREF(leaves);
             return NULL;
         }
@@ -1231,7 +1225,7 @@ _PyExc_ExceptionGroupProjection(PyObject *eg, PyObject *keep)
                 eg, EXCEPTION_GROUP_MATCH_INSTANCES, leaves,
                 construct_rest, &split_result);
     Py_DECREF(leaves);
-    if (err == -1) {
+    if (err < 0) {
         return NULL;
     }
 

@@ -1092,7 +1092,7 @@ 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 PyObject *do_reraise_star(PyObject *excs, PyObject *orig);
 static int exception_group_match(
     PyObject *exc_type, PyObject* exc_value, PyObject *match_type,
     PyObject **match, PyObject **rest);
@@ -2814,7 +2814,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
 
             PyObject *lasti_unused = Py_NewRef(_PyLong_GetZero());
             PUSH(lasti_unused);
-            if (val != Py_None) {
+            if (!Py_IsNone(val)) {
                 PyObject *tb = PyException_GetTraceback(val);
                 PUSH(tb ? tb : Py_NewRef(Py_None));
                 PUSH(val);
@@ -3970,10 +3970,10 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
             }
 
             PyObject *match = NULL, *rest = NULL;
-            int res = exception_group_match(
-                exc_type, exc_value, match_type, &match, &rest);
+            int res = exception_group_match(exc_type, exc_value,
+                                            match_type, &match, &rest);
             Py_DECREF(match_type);
-            if (res == -1) {
+            if (res < 0) {
                 goto error;
             }
 
@@ -3983,7 +3983,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
                 goto error;
             }
 
-            if (match == Py_None) {
+            if (Py_IsNone(match)) {
                 Py_DECREF(match);
                 Py_XDECREF(rest);
                 /* no match - jump to target */
@@ -4003,7 +4003,7 @@ _PyEval_EvalFrameDefault(PyThreadState *tstate, InterpreterFrame *frame, int thr
                 PyObject *val = SECOND();
                 PyObject *tb = THIRD();
 
-                if (rest != Py_None) {
+                if (!Py_IsNone(rest)) {
                     /* tb remains the same */
                     SET_TOP(Py_NewRef(Py_TYPE(rest)));
                     SET_SECOND(Py_NewRef(rest));
@@ -6248,12 +6248,11 @@ do_raise(PyThreadState *tstate, PyObject *exc, PyObject *cause)
 */
 
 static int
-exception_group_match(
-    PyObject *exc_type, PyObject* exc_value, PyObject *match_type,
-    PyObject **match, PyObject **rest)
+exception_group_match(PyObject *exc_type, PyObject* exc_value,
+                      PyObject *match_type, PyObject **match, PyObject **rest)
 {
-    if (exc_type == Py_None) {
-        assert(exc_value == Py_None);
+    if (Py_IsNone(exc_type)) {
+        assert(Py_IsNone(exc_value));
         *match = Py_NewRef(Py_None);
         *rest = Py_NewRef(Py_None);
         return 0;
@@ -6292,8 +6291,8 @@ exception_group_match(
      * Check for partial match if it's an exception group.
      */
     if (_PyBaseExceptionGroup_Check(exc_value)) {
-        PyObject *pair = PyObject_CallMethod(
-            exc_value, "split", "(O)", match_type);
+        PyObject *pair = PyObject_CallMethod(exc_value, "split", "(O)",
+                                             match_type);
         if (pair == NULL) {
             return -1;
         }
@@ -6327,9 +6326,9 @@ is_same_exception_metadata(PyObject *exc1, PyObject *exc2)
     PyObject *ctx2 = PyException_GetContext(exc2);
     PyObject *cause2 = PyException_GetCause(exc2);
 
-    bool result = ((tb1 == tb2) &&
-                   (ctx1 == ctx2) &&
-                   (cause1 == cause2));
+    bool result = (Py_Is(tb1, tb2) &&
+                   Py_Is(ctx1, ctx2) &&
+                   Py_Is(cause1, cause2));
 
     Py_XDECREF(tb1);
     Py_XDECREF(ctx1);
@@ -6351,7 +6350,7 @@ is_same_exception_metadata(PyObject *exc1, PyObject *exc2)
 
    Returns NULL and sets an exception on failure.
 */
-static PyObject*
+static PyObject *
 do_reraise_star(PyObject *excs, PyObject *orig)
 {
     assert(PyList_Check(excs));
@@ -6394,7 +6393,7 @@ do_reraise_star(PyObject *excs, PyObject *orig)
     for (Py_ssize_t i = 0; i < numexcs; i++) {
         PyObject *e = PyList_GET_ITEM(excs, i);
         assert(e != NULL);
-        if (e == Py_None) {
+        if (Py_IsNone(e)) {
             continue;
         }
         bool is_reraise = is_same_exception_metadata(e, orig);
@@ -6409,20 +6408,21 @@ do_reraise_star(PyObject *excs, PyObject *orig)
         goto done;
     }
 
-    if (reraised_eg != Py_None) {
+    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) {
+    }
+    else if (num_raised > 0) {
         int res = 0;
-        if (reraised_eg != Py_None) {
+        if (!Py_IsNone(reraised_eg)) {
             res = PyList_Append(raised_list, reraised_eg);
         }
         Py_DECREF(reraised_eg);
-        if (res == -1) {
+        if (res < 0) {
             goto done;
         }
         result = _PyExc_CreateExceptionGroup("", raised_list);
@@ -7334,7 +7334,7 @@ import_all_from(PyThreadState *tstate, PyObject *locals, PyObject *v)
                          "BaseException is not allowed"
 
 #define CANNOT_EXCEPT_STAR_EG "catching ExceptionGroup with except* "\
-                          "is not allowed. Use except instead."
+                              "is not allowed. Use except instead."
 
 static int
 check_except_type_valid(PyThreadState *tstate, PyObject* right)
@@ -7370,12 +7370,11 @@ check_except_star_type_valid(PyThreadState *tstate, PyObject* right)
     // reject except *ExceptionGroup
     int res = 0;
     if (PyTuple_Check(right)) {
-        Py_ssize_t i, length;
-        length = PyTuple_GET_SIZE(right);
-        for (i = 0; i < length; i++) {
+        Py_ssize_t length = PyTuple_GET_SIZE(right);
+        for (Py_ssize_t i = 0; i < length; i++) {
             PyObject *exc = PyTuple_GET_ITEM(right, i);
             res = PyObject_IsSubclass(exc, PyExc_BaseExceptionGroup);
-            if (res == -1) {
+            if (res < 0) {
                 return -1;
             }
             if (res == 1) {
@@ -7385,7 +7384,7 @@ check_except_star_type_valid(PyThreadState *tstate, PyObject* right)
     }
     else {
         res = PyObject_IsSubclass(right, PyExc_BaseExceptionGroup);
-        if (res == -1) {
+        if (res < 0) {
             return -1;
         }
     }

@@ -3225,8 +3225,9 @@ compiler_try_star_finally(struct compiler *c, stmt_ty s)
     if (!compiler_push_fblock(c, FINALLY_TRY, body, end, s->v.TryStar.finalbody))
         return 0;
     if (s->v.TryStar.handlers && asdl_seq_LEN(s->v.TryStar.handlers)) {
-        if (!compiler_try_star_except(c, s))
+        if (!compiler_try_star_except(c, s)) {
             return 0;
+        }
     }
     else {
         VISIT_SEQ(c, stmt, s->v.TryStar.body);
@@ -3241,8 +3242,9 @@ compiler_try_star_finally(struct compiler *c, stmt_ty s)
     UNSET_LOC(c);
     ADDOP_JUMP(c, SETUP_CLEANUP, cleanup);
     ADDOP(c, PUSH_EXC_INFO);
-    if (!compiler_push_fblock(c, FINALLY_END, end, NULL, NULL))
+    if (!compiler_push_fblock(c, FINALLY_END, end, NULL, NULL)) {
         return 0;
+    }
     VISIT_SEQ(c, stmt, s->v.TryStar.finalbody);
     compiler_pop_fblock(c, FINALLY_END, end);
     ADDOP_I(c, RERAISE, 0);
@@ -3485,13 +3487,15 @@ compiler_try_star_except(struct compiler *c, stmt_ty s)
         return 0;
 
     reraise_star = compiler_new_block(c);
-    if (reraise_star == NULL)
+    if (reraise_star == NULL) {
         return 0;
+    }
 
     ADDOP_JUMP(c, SETUP_FINALLY, except);
     compiler_use_next_block(c, body);
-    if (!compiler_push_fblock(c, TRY_EXCEPT, body, NULL, NULL))
+    if (!compiler_push_fblock(c, TRY_EXCEPT, body, NULL, NULL)) {
         return 0;
+    }
     VISIT_SEQ(c, stmt, s->v.TryStar.body);
     compiler_pop_fblock(c, TRY_EXCEPT, body);
     ADDOP_NOLINE(c, POP_BLOCK);
@@ -3510,8 +3514,9 @@ compiler_try_star_except(struct compiler *c, stmt_ty s)
             s->v.TryStar.handlers, i);
         SET_LOC(c, handler);
         except = compiler_new_block(c);
-        if (except == NULL)
+        if (except == NULL) {
             return 0;
+        }
         if (i == 0) {
             /* Push the original EG into the stack */
             /*
@@ -3577,7 +3582,7 @@ compiler_try_star_except(struct compiler *c, stmt_ty s)
         /* name = None; del name; # Mark as artificial */
         UNSET_LOC(c);
         ADDOP(c, POP_BLOCK);
-        if(handler->v.ExceptHandler.name) {
+        if (handler->v.ExceptHandler.name) {
             ADDOP_LOAD_CONST(c, Py_None);
             compiler_nameop(c, handler->v.ExceptHandler.name, Store);
             compiler_nameop(c, handler->v.ExceptHandler.name, Del);
@@ -3590,7 +3595,7 @@ compiler_try_star_except(struct compiler *c, stmt_ty s)
         /* name = None; del name; # Mark as artificial */
         UNSET_LOC(c);
 
-        if(handler->v.ExceptHandler.name) {
+        if (handler->v.ExceptHandler.name) {
             ADDOP_LOAD_CONST(c, Py_None);
             compiler_nameop(c, handler->v.ExceptHandler.name, Store);
             compiler_nameop(c, handler->v.ExceptHandler.name, Del);
@@ -3616,10 +3621,10 @@ compiler_try_star_except(struct compiler *c, stmt_ty s)
     /* Mark as artificial */
     UNSET_LOC(c);
     compiler_pop_fblock(c, EXCEPTION_GROUP_HANDLER, NULL);
-    basicblock *reraise;
-    reraise = compiler_new_block(c);
-    if (!reraise)
+    basicblock *reraise = compiler_new_block(c);
+    if (!reraise) {
         return 0;
+    }
 
     compiler_use_next_block(c, reraise_star);
     ADDOP(c, PREP_RERAISE_STAR);
@@ -3656,11 +3661,14 @@ compiler_try(struct compiler *c, stmt_ty s) {
 }
 
 static int
-compiler_try_star(struct compiler *c, stmt_ty s) {
-    if (s->v.TryStar.finalbody && asdl_seq_LEN(s->v.TryStar.finalbody))
+compiler_try_star(struct compiler *c, stmt_ty s)
+{
+    if (s->v.TryStar.finalbody && asdl_seq_LEN(s->v.TryStar.finalbody)) {
         return compiler_try_star_finally(c, s);
-    else
+    }
+    else {
         return compiler_try_star_except(c, s);
+    }   
 }
 
 static int
Original file line number	Diff line number	Diff line change
Expand Up		@@ -974,4 +974,3 @@ def test_exc_info_restored(self):

		if __name__ == '__main__':
		unittest.main()