gh-74690: Avoid a costly type check where possible in _ProtocolMeta.__subclasscheck__ (#112717)

AlexWaygood · web-flow · commit 2ed20d3bd84f · 2023-12-04T19:35:46.000Z
diff --git a/Lib/test/test_typing.py b/Lib/test/test_typing.py
@@ -3533,13 +3533,26 @@ def __subclasshook__(cls, other):
 
     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')
diff --git a/Lib/typing.py b/Lib/typing.py
@@ -1790,6 +1790,23 @@ def _pickle_pskwargs(pskwargs):
 _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 @@ def __subclasscheck__(cls, other):
             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 @@ def __subclasscheck__(cls, other):
                     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
@@ -0,0 +1,2 @@
+Speedup :func:`issubclass` checks against simple :func:`runtime-checkable
+protocols <typing.runtime_checkable>` by around 6%. Patch by Alex Waygood.

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	+Speedup :func:`issubclass` checks against simple :func:`runtime-checkable
	`2`	+protocols <typing.runtime_checkable>` by around 6%. Patch by Alex Waygood.