gh-99181: fix except* on unhashable exceptions (GH-99192)

iritkatriel · web-flow · commit c43714fbcdb9 · 2022-11-08T09:32:20.000Z
diff --git a/Lib/test/test_except_star.py b/Lib/test/test_except_star.py
@@ -1000,5 +1000,204 @@ def test_exc_info_restored(self):
         self.assertEqual(sys.exc_info(), (None, None, None))
 
 
+class TestExceptStar_WeirdLeafExceptions(ExceptStarTest):
+    # Test that except* works when leaf exceptions are
+    # unhashable or have a bad custom __eq__
+
+    class UnhashableExc(ValueError):
+        __hash__ = None
+
+    class AlwaysEqualExc(ValueError):
+        def __eq__(self, other):
+            return True
+
+    class NeverEqualExc(ValueError):
+        def __eq__(self, other):
+            return False
+
+    class BrokenEqualExc(ValueError):
+        def __eq__(self, other):
+            raise RuntimeError()
+
+    def setUp(self):
+        self.bad_types = [self.UnhashableExc,
+                          self.AlwaysEqualExc,
+                          self.NeverEqualExc,
+                          self.BrokenEqualExc]
+
+    def except_type(self, eg, type):
+        match, rest = None, None
+        try:
+            try:
+                raise eg
+            except* type  as e:
+                match = e
+        except Exception as e:
+            rest = e
+        return match, rest
+
+    def test_catch_unhashable_leaf_exception(self):
+        for Bad in self.bad_types:
+            with self.subTest(Bad):
+                eg = ExceptionGroup("eg", [TypeError(1), Bad(2)])
+                match, rest = self.except_type(eg, Bad)
+                self.assertExceptionIsLike(
+                    match, ExceptionGroup("eg", [Bad(2)]))
+                self.assertExceptionIsLike(
+                    rest, ExceptionGroup("eg", [TypeError(1)]))
+
+    def test_propagate_unhashable_leaf(self):
+        for Bad in self.bad_types:
+            with self.subTest(Bad):
+                eg = ExceptionGroup("eg", [TypeError(1), Bad(2)])
+                match, rest = self.except_type(eg, TypeError)
+                self.assertExceptionIsLike(
+                    match, ExceptionGroup("eg", [TypeError(1)]))
+                self.assertExceptionIsLike(
+                    rest, ExceptionGroup("eg", [Bad(2)]))
+
+    def test_catch_nothing_unhashable_leaf(self):
+        for Bad in self.bad_types:
+            with self.subTest(Bad):
+                eg = ExceptionGroup("eg", [TypeError(1), Bad(2)])
+                match, rest = self.except_type(eg, OSError)
+                self.assertIsNone(match)
+                self.assertExceptionIsLike(rest, eg)
+
+    def test_catch_everything_unhashable_leaf(self):
+        for Bad in self.bad_types:
+            with self.subTest(Bad):
+                eg = ExceptionGroup("eg", [TypeError(1), Bad(2)])
+                match, rest = self.except_type(eg, Exception)
+                self.assertExceptionIsLike(match, eg)
+                self.assertIsNone(rest)
+
+    def test_reraise_unhashable_leaf(self):
+        for Bad in self.bad_types:
+            with self.subTest(Bad):
+                eg = ExceptionGroup(
+                    "eg", [TypeError(1), Bad(2), ValueError(3)])
+
+                try:
+                    try:
+                        raise eg
+                    except* TypeError:
+                        pass
+                    except* Bad:
+                        raise
+                except Exception as e:
+                    exc = e
+
+                self.assertExceptionIsLike(
+                    exc, ExceptionGroup("eg", [Bad(2), ValueError(3)]))
+
+
+class TestExceptStar_WeirdExceptionGroupSubclass(ExceptStarTest):
+    # Test that except* works with exception groups that are
+    # unhashable or have a bad custom __eq__
+
+    class UnhashableEG(ExceptionGroup):
+        __hash__ = None
+
+        def derive(self, excs):
+            return type(self)(self.message, excs)
+
+    class AlwaysEqualEG(ExceptionGroup):
+        def __eq__(self, other):
+            return True
+
+        def derive(self, excs):
+            return type(self)(self.message, excs)
+
+    class NeverEqualEG(ExceptionGroup):
+        def __eq__(self, other):
+            return False
+
+        def derive(self, excs):
+            return type(self)(self.message, excs)
+
+    class BrokenEqualEG(ExceptionGroup):
+        def __eq__(self, other):
+            raise RuntimeError()
+
+        def derive(self, excs):
+            return type(self)(self.message, excs)
+
+    def setUp(self):
+        self.bad_types = [self.UnhashableEG,
+                          self.AlwaysEqualEG,
+                          self.NeverEqualEG,
+                          self.BrokenEqualEG]
+
+    def except_type(self, eg, type):
+        match, rest = None, None
+        try:
+            try:
+                raise eg
+            except* type  as e:
+                match = e
+        except Exception as e:
+            rest = e
+        return match, rest
+
+    def test_catch_some_unhashable_exception_group_subclass(self):
+        for BadEG in self.bad_types:
+            with self.subTest(BadEG):
+                eg = BadEG("eg",
+                           [TypeError(1),
+                            BadEG("nested", [ValueError(2)])])
+
+                match, rest = self.except_type(eg, TypeError)
+                self.assertExceptionIsLike(match, BadEG("eg", [TypeError(1)]))
+                self.assertExceptionIsLike(rest,
+                    BadEG("eg", [BadEG("nested", [ValueError(2)])]))
+
+    def test_catch_none_unhashable_exception_group_subclass(self):
+        for BadEG in self.bad_types:
+            with self.subTest(BadEG):
+
+                eg = BadEG("eg",
+                           [TypeError(1),
+                            BadEG("nested", [ValueError(2)])])
+
+                match, rest = self.except_type(eg, OSError)
+                self.assertIsNone(match)
+                self.assertExceptionIsLike(rest, eg)
+
+    def test_catch_all_unhashable_exception_group_subclass(self):
+        for BadEG in self.bad_types:
+            with self.subTest(BadEG):
+
+                eg = BadEG("eg",
+                           [TypeError(1),
+                            BadEG("nested", [ValueError(2)])])
+
+                match, rest = self.except_type(eg, Exception)
+                self.assertExceptionIsLike(match, eg)
+                self.assertIsNone(rest)
+
+    def test_reraise_unhashable_eg(self):
+        for BadEG in self.bad_types:
+            with self.subTest(BadEG):
+
+                eg = BadEG("eg",
+                           [TypeError(1), ValueError(2),
+                            BadEG("nested", [ValueError(3), OSError(4)])])
+
+                try:
+                    try:
+                        raise eg
+                    except* ValueError:
+                        pass
+                    except* OSError:
+                        raise
+                except Exception as e:
+                    exc = e
+
+                self.assertExceptionIsLike(
+                    exc, BadEG("eg", [TypeError(1),
+                               BadEG("nested", [OSError(4)])]))
+
+
 if __name__ == '__main__':
     unittest.main()
diff --git a/Misc/NEWS.d/next/Core and Builtins/2022-11-07-10-29-41.gh-issue-99181.bfG4bI.rst b/Misc/NEWS.d/next/Core and Builtins/2022-11-07-10-29-41.gh-issue-99181.bfG4bI.rst
@@ -0,0 +1 @@
+Fix failure in :keyword:`except* <except_star>` with unhashable exceptions.
diff --git a/Objects/exceptions.c b/Objects/exceptions.c
@@ -962,11 +962,11 @@ typedef enum {
     EXCEPTION_GROUP_MATCH_BY_TYPE = 0,
     /* A PyFunction returning True for matching exceptions */
     EXCEPTION_GROUP_MATCH_BY_PREDICATE = 1,
-    /* A set of leaf exceptions to include in the result.
+    /* A set of the IDs of leaf exceptions to include in the result.
      * This matcher type is used internally by the interpreter
      * to construct reraised exceptions.
      */
-    EXCEPTION_GROUP_MATCH_INSTANCES = 2
+    EXCEPTION_GROUP_MATCH_INSTANCE_IDS = 2
 } _exceptiongroup_split_matcher_type;
 
 static int
@@ -1024,10 +1024,16 @@ exceptiongroup_split_check_match(PyObject *exc,
         Py_DECREF(exc_matches);
         return is_true;
     }
-    case EXCEPTION_GROUP_MATCH_INSTANCES: {
+    case EXCEPTION_GROUP_MATCH_INSTANCE_IDS: {
         assert(PySet_Check(matcher_value));
         if (!_PyBaseExceptionGroup_Check(exc)) {
-            return PySet_Contains(matcher_value, exc);
+            PyObject *exc_id = PyLong_FromVoidPtr(exc);
+            if (exc_id == NULL) {
+                return -1;
+            }
+            int res = PySet_Contains(matcher_value, exc_id);
+            Py_DECREF(exc_id);
+            return res;
         }
         return 0;
     }
@@ -1212,32 +1218,35 @@ BaseExceptionGroup_subgroup(PyObject *self, PyObject *args)
 }
 
 static int
-collect_exception_group_leaves(PyObject *exc, PyObject *leaves)
+collect_exception_group_leaf_ids(PyObject *exc, PyObject *leaf_ids)
 {
     if (Py_IsNone(exc)) {
         return 0;
     }
 
     assert(PyExceptionInstance_Check(exc));
-    assert(PySet_Check(leaves));
+    assert(PySet_Check(leaf_ids));
 
-    /* Add all leaf exceptions in exc to the leaves set */
+    /* Add IDs of all leaf exceptions in exc to the leaf_ids set */
 
     if (!_PyBaseExceptionGroup_Check(exc)) {
-        if (PySet_Add(leaves, exc) < 0) {
+        PyObject *exc_id = PyLong_FromVoidPtr(exc);
+        if (exc_id == NULL) {
             return -1;
         }
-        return 0;
+        int res = PySet_Add(leaf_ids, exc_id);
+        Py_DECREF(exc_id);
+        return res;
     }
     PyBaseExceptionGroupObject *eg = _PyBaseExceptionGroupObject_cast(exc);
     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);
-        if (_Py_EnterRecursiveCall(" in collect_exception_group_leaves")) {
+        if (_Py_EnterRecursiveCall(" in collect_exception_group_leaf_ids")) {
             return -1;
         }
-        int res = collect_exception_group_leaves(e, leaves);
+        int res = collect_exception_group_leaf_ids(e, leaf_ids);
         _Py_LeaveRecursiveCall();
         if (res < 0) {
             return -1;
@@ -1258,8 +1267,8 @@ exception_group_projection(PyObject *eg, PyObject *keep)
     assert(_PyBaseExceptionGroup_Check(eg));
     assert(PyList_CheckExact(keep));
 
-    PyObject *leaves = PySet_New(NULL);
-    if (!leaves) {
+    PyObject *leaf_ids = PySet_New(NULL);
+    if (!leaf_ids) {
         return NULL;
     }
 
@@ -1268,18 +1277,18 @@ exception_group_projection(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) < 0) {
-            Py_DECREF(leaves);
+        if (collect_exception_group_leaf_ids(e, leaf_ids) < 0) {
+            Py_DECREF(leaf_ids);
             return NULL;
         }
     }
 
     _exceptiongroup_split_result split_result;
     bool construct_rest = false;
     int err = exceptiongroup_split_recursive(
-                eg, EXCEPTION_GROUP_MATCH_INSTANCES, leaves,
+                eg, EXCEPTION_GROUP_MATCH_INSTANCE_IDS, leaf_ids,
                 construct_rest, &split_result);
-    Py_DECREF(leaves);
+    Py_DECREF(leaf_ids);
     if (err < 0) {
         return NULL;
     }

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	+Fix failure in :keyword:`except* <except_star>` with unhashable exceptions.