gh-74690: Avoid a costly type check where possible in _ProtocolMeta.__subclasscheck__ by AlexWaygood · Pull Request #112717 · python/cpython (original) (raw)

Thanks for the review! :D

Looks good! Is it important that the "issubclass() arg 1 must be a class" error message takes precedence over the others? It's not obvious to me that it should, but I'm not very familiar with this code.

Yeah, so to clarify, with the type checks, this is the behaviour we get with this PR branch:

Python 3.13.0a2+ (heads/main:9560e0d6d7, Dec 4 2023, 13:50:58) [MSC v.1932 64 bit (AMD64)] on win32 Type "help", "copyright", "credits" or "license" for more information.

import typing as t @t.runtime_checkable ... class Foo(t.Protocol): ... X = 1 ... issubclass("not a class", Foo) Traceback (most recent call last): File "", line 1, in issubclass("not a class", Foo) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^ File "C:\Users\alexw\coding\fast-cpython\Lib\typing.py", line 1842, in subclasscheck _type_check_subclasscheck_second_arg(other) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^ File "C:\Users\alexw\coding\fast-cpython\Lib\typing.py", line 1796, in _type_check_subclasscheck_second_arg raise TypeError('issubclass() arg 1 must be a class') TypeError: issubclass() arg 1 must be a class

But without the type checks, we'd get this behaviour instead:

import typing as t @t.runtime_checkable ... class Foo(t.Protocol): ... X = 1 ... issubclass("not a class", Foo) Traceback (most recent call last): File "", line 1, in issubclass("not a class", Foo) ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^ File "C:\Users\alexw\coding\fast-cpython\Lib\typing.py", line 1847, in subclasscheck raise TypeError( ...<2 lines>... ) TypeError: Protocols with non-method members don't support issubclass(). Non-method members: 'X'.

It wouldn't be the worst thing in the world if we had that error message instead. But the fact that Foo has non-method members is clearly not the most significant problem with this person's code! So I definitely prefer to prioritise the "issubclass() arg 1 must be a class" message, if possible :)