gh-104555: Runtime-checkable protocols: Don't let previous calls to isinstance() influence whether issubclass() raises an exception

[AlexWaygood]

ABCMeta.__instancecheck__ caches isinstance() calls against classes that have ABCMeta as their metaclass. It uses these cache entries not only to inform how future isinstance() calls behave, but also how issubclass() calls behave. That means that on main we now have some rather unfortunate behaviour when it comes to runtime-checkable protocols, due to the fact that typing._ProtocolMeta is a subclass of ABCMeta, and typing._ProtocolMeta.__instancecheck__ calls super().__instancecheck__() too soon (following 47753ec):

Python 3.12.0a7+ (heads/main:1163782868, May 16 2023, 18:09:28) [MSC v.1932 64 bit (AMD64)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> from typing import *
>>> @runtime_checkable
... class Foo(Protocol):
...     x: int
...
>>> class Bar:
...     x = 42
...
>>> issubclass(Bar, Foo)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Users\alexw\coding\cpython\Lib\abc.py", line 123, in __subclasscheck__
    return _abc_subclasscheck(cls, subclass)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\alexw\coding\cpython\Lib\typing.py", line 1875, in _proto_hook
    raise TypeError("Protocols with non-method members"
TypeError: Protocols with non-method members don't support issubclass()
>>> isinstance(Bar(), Foo)
True
>>> issubclass(Bar, Foo)
False

This PR fixes the incorrect behaviour. It means that these isinstance() checks will be about twice as slow as they are on main:

@runtime_checkable
class Foo:
    def meth(self): ...

@Foo.register
class Bar: ...

isinstance(Bar(), Foo)

But they'll still be much faster than they are on 3.11. Use of ABCMeta.register with protocols is pretty rare anyway, as far as I know, since ABCMeta.register isn't supported by type checkers. Other kinds of isinstance() checks do not suffer a significant performance regression.

Fixes #104555.

(Skipping news, as this bug doesn't exist on any released version of Python.)

• Issue: Typing: runtime-checkable protocols are broken on main #104555

[AlexWaygood]

On main, the pyperformance bm_typing_runtime_protocols benchmark takes 177us +- 5us on my machine. With this PR, it takes 192us +- 5us. So, a slight slowdown. But that's okay if we avoid incorrect behaviour.

[sunmy2019]

LGTM

But I still think __subclasshook__ depending on the caller is the weirdest part (of #104555 (comment)). Though it cannot be changed or is hard to change.

[AlexWaygood]

Without this check, I realised that this patch would also have slowed down isinstance(3, SupportsIndex) quite dramatically.

[sunmy2019]

Oh, we will still hit the problematic super().__instancecheck__ sooner or later.

Consider this

from typing import runtime_checkable, Generic, Protocol, TypeVar

T = TypeVar("T")


@runtime_checkable
class Spam(Protocol[T]):
    x: T


class Eggs(Generic[T]):
    def __init__(self, x: T) -> None:
        self._x = x

    def __getattr__(self, attr: str) -> T:
        if attr == "x":
            return self._x
        raise AttributeError(attr)


print(isinstance(Eggs(42), Spam))
print(issubclass(Eggs, Spam))

Running on 3.10, 3.11 -> TypeError
Running on current main and this PR -> no TypeError

[sunmy2019]

I think one possible solution is that "do not let super().__subclasscheck()__ cache __subclasscheck__ result in this case".

[AlexWaygood]

@sunmy2019, I think we were thinking the same thought simultaneously :)

How does it look to you after 5f72b82? (It's basically a completely different approach now.)

[AlexWaygood]

This test fails on 3.11. We could consider backporting a version of this PR to 3.11, but I'm not sure if that would be a good idea or not. It's a somewhat invasive bugfix.

[AlexWaygood]

This new approach also has the advantage that it seems like it might provide a speedup relative to main, for isinstance checks that don't involve ABCMeta.register()? (But might just be noise.)

[sunmy2019]

This new approach looks better. Great Job!

[carljm]

LGTM

[AlexWaygood]

Thanks @JelleZijlstra for helping debug this, @sunmy2019 for spotting the flaws in my original approach, and everybody for reviewing!

[bedevere-bot]

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot AMD64 Windows11 Bigmem 3.x has failed when building commit b27fe67.

What do you need to do:

1. Don't panic.
2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/1079/builds/1325) and take a look at the build logs.
4. Check if the failure is related to this commit (b27fe67) or if it is a false positive.
5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/1079/builds/1325

Failed tests:

• test_hashlib

Failed subtests:

• test_case_md5_uintmax - test.test_hashlib.HashLibTestCase.test_case_md5_uintmax
• test_case_md5_huge - test.test_hashlib.HashLibTestCase.test_case_md5_huge

Summary of the results of the build (if available):

== Tests result: FAILURE then FAILURE ==

428 tests OK.

10 slowest tests:

• test_bigmem: 43 min 23 sec
• test_lzma: 34 min 43 sec
• test_bz2: 21 min 5 sec
• test_array: 7 min 22 sec
• test_zlib: 4 min 58 sec
• test_multiprocessing_spawn: 2 min 2 sec
• test_math: 1 min 27 sec
• test_hashlib: 1 min 22 sec
• test_pickle: 1 min 14 sec
• test_concurrent_futures: 1 min 13 sec

1 test failed:
test_hashlib

36 tests skipped:
test.test_asyncio.test_unix_events test_clinic test_curses
test_dbm_gnu test_dbm_ndbm test_devpoll test_epoll test_fcntl
test_fork1 test_gdb test_grp test_ioctl test_kqueue
test_multiprocessing_fork test_multiprocessing_forkserver test_nis
test_openpty test_ossaudiodev test_peg_generator
test_perf_profiler test_pipes test_poll test_posix test_pty
test_pwd test_readline test_resource test_spwd test_syslog
test_threadsignals test_wait3 test_wait4 test_xxlimited
test_xxtestfuzz test_zipfile64 test_zoneinfo

1 re-run test:
test_hashlib

Total duration: 47 min 52 sec

Click to see traceback logs

Traceback (most recent call last):
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\support\__init__.py", line 979, in wrapper
    return f(self, maxsize)
           ^^^^^^^^^^^^^^^^
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 540, in test_case_md5_uintmax
    self.check('md5', b'A'*size, '28138d306ff1b8281f1a9067e1a1a2b3')
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 394, in check
    self.check_file_digest(name, data, hexdigest)
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 407, in check_file_digest
    f.write(data)
OSError: [Errno 28] No space left on device


Traceback (most recent call last):
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\support\__init__.py", line 979, in wrapper
    return f(self, maxsize)
           ^^^^^^^^^^^^^^^^
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 535, in test_case_md5_huge
    self.check('md5', b'A'*size, 'c9af2dff37468ce5dfee8f2cfc0a9c6d')
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 394, in check
    self.check_file_digest(name, data, hexdigest)
  File "R:\buildarea\3.x.ambv-bb-win11.bigmem\build\Lib\test\test_hashlib.py", line 407, in check_file_digest
    f.write(data)
OSError: [Errno 28] No space left on device