]> git.ipfire.org Git - thirdparty/Python/cpython.git/commitdiff
gh-74690: Avoid a costly type check where possible in `_ProtocolMeta.__subclasscheck_...
authorAlex Waygood <Alex.Waygood@Gmail.com>
Mon, 4 Dec 2023 19:35:46 +0000 (19:35 +0000)
committerGitHub <noreply@github.com>
Mon, 4 Dec 2023 19:35:46 +0000 (19:35 +0000)
Lib/test/test_typing.py
Lib/typing.py
Misc/NEWS.d/next/Library/2023-12-04-16-45-11.gh-issue-74690.pQYP5U.rst [new file with mode: 0644]

index 3572df7737f65262dbc73b70b72501da416a6840..2d10c39840ddf32a039236e09948bbbf5abb449c 100644 (file)
@@ -3533,13 +3533,26 @@ class ProtocolTests(BaseTestCase):
 
     def test_issubclass_fails_correctly(self):
         @runtime_checkable
-        class P(Protocol):
+        class NonCallableMembers(Protocol):
             x = 1
 
+        class NotRuntimeCheckable(Protocol):
+            def callable_member(self) -> int: ...
+
+        @runtime_checkable
+        class RuntimeCheckable(Protocol):
+            def callable_member(self) -> int: ...
+
         class C: pass
 
-        with self.assertRaisesRegex(TypeError, r"issubclass\(\) arg 1 must be a class"):
-            issubclass(C(), P)
+        # These three all exercise different code paths,
+        # but should result in the same error message:
+        for protocol in NonCallableMembers, NotRuntimeCheckable, RuntimeCheckable:
+            with self.subTest(proto_name=protocol.__name__):
+                with self.assertRaisesRegex(
+                    TypeError, r"issubclass\(\) arg 1 must be a class"
+                ):
+                    issubclass(C(), protocol)
 
     def test_defining_generic_protocols(self):
         T = TypeVar('T')
index aa64ed93f76fbfd871ac563f544c3466a4a87fce..61b88a560e9dc56383ab546717e2d8ceb791fe81 100644 (file)
@@ -1790,6 +1790,23 @@ _abc_instancecheck = ABCMeta.__instancecheck__
 _abc_subclasscheck = ABCMeta.__subclasscheck__
 
 
+def _type_check_issubclass_arg_1(arg):
+    """Raise TypeError if `arg` is not an instance of `type`
+    in `issubclass(arg, <protocol>)`.
+
+    In most cases, this is verified by type.__subclasscheck__.
+    Checking it again unnecessarily would slow down issubclass() checks,
+    so, we don't perform this check unless we absolutely have to.
+
+    For various error paths, however,
+    we want to ensure that *this* error message is shown to the user
+    where relevant, rather than a typing.py-specific error message.
+    """
+    if not isinstance(arg, type):
+        # Same error message as for issubclass(1, int).
+        raise TypeError('issubclass() arg 1 must be a class')
+
+
 class _ProtocolMeta(ABCMeta):
     # This metaclass is somewhat unfortunate,
     # but is necessary for several reasons...
@@ -1829,13 +1846,11 @@ class _ProtocolMeta(ABCMeta):
             getattr(cls, '_is_protocol', False)
             and not _allow_reckless_class_checks()
         ):
-            if not isinstance(other, type):
-                # Same error message as for issubclass(1, int).
-                raise TypeError('issubclass() arg 1 must be a class')
             if (
                 not cls.__callable_proto_members_only__
                 and cls.__dict__.get("__subclasshook__") is _proto_hook
             ):
+                _type_check_issubclass_arg_1(other)
                 non_method_attrs = sorted(
                     attr for attr in cls.__protocol_attrs__
                     if not callable(getattr(cls, attr, None))
@@ -1845,6 +1860,7 @@ class _ProtocolMeta(ABCMeta):
                     f" Non-method members: {str(non_method_attrs)[1:-1]}."
                 )
             if not getattr(cls, '_is_runtime_protocol', False):
+                _type_check_issubclass_arg_1(other)
                 raise TypeError(
                     "Instance and class checks can only be used with "
                     "@runtime_checkable protocols"
diff --git a/Misc/NEWS.d/next/Library/2023-12-04-16-45-11.gh-issue-74690.pQYP5U.rst b/Misc/NEWS.d/next/Library/2023-12-04-16-45-11.gh-issue-74690.pQYP5U.rst
new file mode 100644 (file)
index 0000000..8102f02
--- /dev/null
@@ -0,0 +1,2 @@
+Speedup :func:`issubclass` checks against simple :func:`runtime-checkable
+protocols <typing.runtime_checkable>` by around 6%. Patch by Alex Waygood.