gh-92810: Address review fixes

Author: dolfinus

Lib/test/test_abc.py

@@ -14,7 +14,6 @@
 
 def test_factory(abc_ABCMeta, abc_get_cache_token):
     class TestLegacyAPI(unittest.TestCase):
-
         def test_abstractproperty_basics(self):
             @abc.abstractproperty
             def foo(self): pass
@@ -70,6 +69,25 @@ def foo(): return 4
 
 
     class TestABC(unittest.TestCase):
+        def check_isinstance(self, obj, target_class):
+            self.assertIsInstance(obj, target_class)
+            self.assertIsInstance(obj, (target_class,))
+            self.assertIsInstance(obj, target_class | target_class)
+
+        def check_not_isinstance(self, obj, target_class):
+            self.assertNotIsInstance(obj, target_class)
+            self.assertNotIsInstance(obj, (target_class,))
+            self.assertNotIsInstance(obj, target_class | target_class)
+
+        def check_issubclass(self, klass, target_class):
+            self.assertIsSubclass(klass, target_class)
+            self.assertIsSubclass(klass, (target_class,))
+            self.assertIsSubclass(klass, target_class | target_class)
+
+        def check_not_issubclass(self, klass, target_class):
+            self.assertNotIsSubclass(klass, target_class)
+            self.assertNotIsSubclass(klass, (target_class,))
+            self.assertNotIsSubclass(klass, target_class | target_class)
 
         def test_ABC_helper(self):
             # create an ABC using the helper class and perform basic checks
@@ -283,39 +301,24 @@ class C(A):
             c = C()
             # trigger caching
             for _ in range(2):
-                self.assertIsInstance(a, A)
-                self.assertIsInstance(a, (A,))
-                self.assertNotIsInstance(a, B)
-                self.assertNotIsInstance(a, (B,))
-                self.assertNotIsInstance(a, C)
-                self.assertNotIsInstance(a, (C,))
-
-                self.assertIsInstance(b, B)
-                self.assertIsInstance(b, (B,))
-                self.assertIsInstance(b, A)
-                self.assertIsInstance(b, (A,))
-                self.assertNotIsInstance(b, C)
-                self.assertNotIsInstance(b, (C,))
-
-                self.assertIsInstance(c, C)
-                self.assertIsInstance(c, (C,))
-                self.assertIsInstance(c, A)
-                self.assertIsInstance(c, (A,))
-                self.assertNotIsInstance(c, B)
-                self.assertNotIsInstance(c, (B,))
-
-                self.assertIsSubclass(B, A)
-                self.assertIsSubclass(B, (A,))
-                self.assertIsSubclass(C, A)
-                self.assertIsSubclass(C, (A,))
-                self.assertNotIsSubclass(B, C)
-                self.assertNotIsSubclass(B, (C,))
-                self.assertNotIsSubclass(C, B)
-                self.assertNotIsSubclass(C, (B,))
-                self.assertNotIsSubclass(A, B)
-                self.assertNotIsSubclass(A, (B,))
-                self.assertNotIsSubclass(A, C)
-                self.assertNotIsSubclass(A, (C,))
+                self.check_isinstance(a, A)
+                self.check_not_isinstance(a, B)
+                self.check_not_isinstance(a, C)
+
+                self.check_isinstance(b, B)
+                self.check_isinstance(b, A)
+                self.check_not_isinstance(b, C)
+
+                self.check_isinstance(c, C)
+                self.check_isinstance(c, A)
+                self.check_not_isinstance(c, B)
+
+                self.check_issubclass(B, A)
+                self.check_issubclass(C, A)
+                self.check_not_issubclass(B, C)
+                self.check_not_issubclass(C, B)
+                self.check_not_issubclass(A, B)
+                self.check_not_issubclass(A, C)
 
         def test_registration_basics(self):
             class A(metaclass=abc_ABCMeta):
@@ -327,45 +330,33 @@ class B(object):
             b = B()
             # trigger caching
             for _ in range(2):
-                self.assertNotIsSubclass(B, A)
-                self.assertNotIsSubclass(B, (A,))
-                self.assertNotIsInstance(b, A)
-                self.assertNotIsInstance(b, (A,))
+                self.check_not_issubclass(B, A)
+                self.check_not_isinstance(b, A)
 
-                self.assertNotIsSubclass(A, B)
-                self.assertNotIsSubclass(A, (B,))
-                self.assertNotIsInstance(a, B)
-                self.assertNotIsInstance(a, (B,))
+                self.check_not_issubclass(A, B)
+                self.check_not_isinstance(a, B)
 
             B1 = A.register(B)
             # trigger caching
             for _ in range(2):
-                self.assertIsSubclass(B, A)
-                self.assertIsSubclass(B, (A,))
-                self.assertIsInstance(b, A)
-                self.assertIsInstance(b, (A,))
+                self.check_issubclass(B, A)
+                self.check_isinstance(b, A)
                 self.assertIs(B1, B)
 
-                self.assertNotIsSubclass(A, B)
-                self.assertNotIsSubclass(A, (B,))
-                self.assertNotIsInstance(a, B)
-                self.assertNotIsInstance(a, (B,))
+                self.check_not_issubclass(A, B)
+                self.check_not_isinstance(a, B)
 
             class C(B):
                 pass
 
             c = C()
             # trigger caching
             for _ in range(2):
-                self.assertIsSubclass(C, A)
-                self.assertIsSubclass(C, (A,))
-                self.assertIsInstance(c, A)
-                self.assertIsInstance(c, (A,))
+                self.check_issubclass(C, A)
+                self.check_isinstance(c, A)
 
-                self.assertNotIsSubclass(A, C)
-                self.assertNotIsSubclass(A, (C,))
-                self.assertNotIsInstance(a, C)
-                self.assertNotIsInstance(a, (C,))
+                self.check_not_issubclass(A, C)
+                self.check_not_isinstance(a, C)
 
         def test_register_as_class_deco(self):
             class A(metaclass=abc_ABCMeta):
@@ -508,15 +499,15 @@ class Parent2(metaclass=abc_ABCMeta):
 
             # trigger caching
             for _ in range(2):
-                self.assertIsInstance(A(), Parent1)
-                self.assertIsSubclass(A, Parent1)
-                self.assertNotIsInstance(B(), Parent1)
-                self.assertNotIsSubclass(B, Parent1)
-
-                self.assertIsInstance(A(), Parent2)
-                self.assertIsSubclass(A, Parent2)
-                self.assertNotIsInstance(B(), Parent2)
-                self.assertNotIsSubclass(B, Parent2)
+                self.check_isinstance(A(), Parent1)
+                self.check_issubclass(A, Parent1)
+                self.check_not_isinstance(B(), Parent1)
+                self.check_not_issubclass(B, Parent1)
+
+                self.check_isinstance(A(), Parent2)
+                self.check_issubclass(A, Parent2)
+                self.check_not_isinstance(B(), Parent2)
+                self.check_not_issubclass(B, Parent2)
 
         def test_issubclass_bad_arguments(self):
             class A(metaclass=abc_ABCMeta):

Misc/NEWS.d/next/Core_and_Builtins/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst renamed to Misc/NEWS.d/next/Library/2025-06-13-17-22-34.gh-issue-92810.Tb6x6C.rst

@@ -181,10 +181,14 @@ _get_impl(PyObject *module, PyObject *self)
     return (_abc_data *)impl;
 }
 
+/* If class is inherited from ABC, set data to point to internal ABC state of class, and return 1.
+   If object is not inherited from ABC, return 0.
+   If error is encountered, return -1.
+ */
 static int
-_get_impl_optional(PyObject *module, PyObject *self, _abc_data **data)
+_get_optional_impl(_abcmodule_state *state, PyObject *self, _abc_data **data)
 {
-    _abcmodule_state *state = get_abc_state(module);
+    assert(data != NULL);
     PyObject *impl = NULL;
     int res = PyObject_GetOptionalAttr(self, &_Py_ID(_abc_impl), &impl);
     if (res <= 0) {
@@ -841,7 +845,7 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
         }
 
         _abc_data *scls_impl;
-        int scls_is_abc = _get_impl_optional(module, scls, &scls_impl);
+        int scls_is_abc = _get_optional_impl(state, scls, &scls_impl);
         if (scls_is_abc < 0) {
             goto end;
         }
@@ -858,14 +862,16 @@ _abc__abc_subclasscheck_impl(PyObject *module, PyObject *self,
 
         int r = PyObject_IsSubclass(subclass, scls);
         Py_DECREF(scls);
+        if (r < 0) {
+            Py_XDECREF(scls_impl);
+            goto end;
+        }
+
         if (scls_is_abc > 0) {
             unset_issubclasscheck_recursive(scls_impl);
-            Py_DECREF(scls_impl);
         }
+        Py_XDECREF(scls_impl);
 
-        if (r < 0) {
-            goto end;
-        }
         if (r > 0) {
             if (!is_issubclasscheck_recursive(impl)) {
                 if (_add_to_weak_set(impl, &impl->_abc_cache, subclass) < 0) {