Typing: runtime-checkable protocols are broken on main

[AlexWaygood]

PEP-695 protocols don't work as intended:

Here's the behaviour you get with protocols that use pre-PEP 695 syntax, which is correct:

>>> from typing import Protocol, runtime_checkable, TypeVar
>>> T_co = TypeVar("T_co", covariant=True)
>>> @runtime_checkable
... class SupportsAbsOld(Protocol[T_co]):
...     def __abs__(self) -> T_co:
...         ...
...
>>> isinstance(0, SupportsAbsOld)
True
>>> issubclass(float, SupportsAbsOld)
True

And here's the behaviour you get on main with protocols that use PEP 695 syntax, which is incorrect:

>>> @runtime_checkable
... class SupportsAbsNew[T_co](Protocol):
...     def __abs__(self) -> T_co:
...         ...
...
>>> isinstance(0, SupportsAbsNew)
False
>>> issubclass(float, SupportsAbsNew)
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()

Linked PRs

• gh-104555: Fix isinstance() and issubclass() for runtime-checkable protocols that use PEP 695 #104556
• gh-104555: Runtime-checkable protocols: Don't let previous calls to isinstance() influence whether issubclass() raises an exception #104559

[AlexWaygood]

(I'm working on a fix)

[JelleZijlstra]

Thanks for catching! I suppose this is about __type_params__?

[AlexWaygood]

Thanks for catching! I suppose this is about __type_params__?

Yes.

[AlexWaygood]

I'm getting very confused here. The fix for the original issue is simple, and I've fixed it locally. But as part of the PR, I tried adding this test, and the last assertion fails (the test reports that TypeError wasn't raised):

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

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

        self.assertIsInstance(Eggs(42), Spam)
        with self.assertRaises(TypeError):
            issubclass(Eggs, Spam)

But I can't reproduce the failure in the REPL:

>>> from typing import *
>>> @runtime_checkable
... class Spam[T](Protocol):
...     x: T
... 
...     
>>> class Eggs[T]:
...     def __init__(self, x: T) -> None:
...         self.x = x
... 
...         
>>> issubclass(Eggs, Spam)
Traceback (most recent call last):
  File "<pyshell#5>", line 1, in <module>
    issubclass(Eggs, Spam)
  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()

[AlexWaygood]

Anyway, I'll submit a PR without that test for now...

[JelleZijlstra]

I'll look into this more.

[AlexWaygood]

Okay, cool. I'll enable automerge on my PR for now, as it definitely gets us closer to correct behaviour.

[JelleZijlstra]

The issue you saw also reproduces for me without PEP 695 syntax. It seems to be due to the _allow_reckless_class_checks check, which somehow thinks the caller is abc if we're in a test, but not if we're in the REPL.

[AlexWaygood]

Aha, so we've discovered a bug, it just isn't a new bug.

[JelleZijlstra]

It's possible this bug was introduced by the fact that Generic is now a C class, haven't tried on 3.11 yet.

[AlexWaygood]

Yes, looks like the bug is new in 3.12. Running this test file passes on 3.11, but not on main:

import unittest
from typing import runtime_checkable, Generic, Protocol, TypeVar

T = TypeVar("T")

class TestProtocols(unittest.TestCase):
    def test_pep695_generic_protocol_noncallable_members(self):
        @runtime_checkable
        class Spam(Protocol[T]):
            x: T

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

        self.assertIsInstance(Eggs(42), Spam)
        with self.assertRaises(TypeError):
            issubclass(Eggs, Spam)


if __name__ == "__main__":
    unittest.main()

[AlexWaygood]

It's easier to keep the discussion in one place, so I'm reopening so we can continue discussing the other bug

[JelleZijlstra]

A few things I found:

• The reason why Alex saw different behavior in the REPL and a test is that it matters whether you call isinstance() first. The issue shows up only if call isinstance() before you call issubclass().
• The issue reproduces on a commit on main before PEP 695 was merged, so it's not due to PEP 695.
• I'm bisecting now to find the culprit.

[JelleZijlstra]

47753ecde21b79b5c5f11d883946fda2a340e427 is the first bad commit
commit 47753ecde21b79b5c5f11d883946fda2a340e427
Author: Alex Waygood <[email protected]>
Date:   Wed Apr 5 10:07:30 2023 +0100

    gh-74690: typing: Simplify and optimise `_ProtocolMeta.__instancecheck__` (#103159)

 Lib/typing.py | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

47753ec

[AlexWaygood]

47753ecde21b79b5c5f11d883946fda2a340e427 is the first bad commit
commit 47753ecde21b79b5c5f11d883946fda2a340e427
Author: Alex Waygood <[email protected]>
Date:   Wed Apr 5 10:07:30 2023 +0100

    gh-74690: typing: Simplify and optimise `_ProtocolMeta.__instancecheck__` (#103159)

 Lib/typing.py | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

(I am, again, working on a fix :)

[AlexWaygood]

Incredible stuff:

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

[AlexWaygood]

@sunmy2019 I was just about to file 2601138 as a PR, which fixes things, but do you think we could fix it by just adjusting the depth value we pass to _allow_reckless_class_checks()?

[sunmy2019]

Oh sorry, ignore my previous comment.

[sunmy2019]

Confirmed that the code path that does not raise TypeError goes through L606.

cpython/Modules/_abc.c

Lines 589 to 608 in 03e3c34

	/* 2. Check negative cache; may have to invalidate. */
	if (impl->_abc_negative_cache_version < abc_invalidation_counter) {
	/* Invalidate the negative cache. */
	if (impl->_abc_negative_cache != NULL &&
	PySet_Clear(impl->_abc_negative_cache) < 0)
	{
	goto end;
	}
	impl->_abc_negative_cache_version = abc_invalidation_counter;
	}
	else {
	incache = _in_weak_set(impl->_abc_negative_cache, subclass);
	if (incache < 0) {
	goto end;
	}
	if (incache > 0) {
	result = Py_False;
	goto end;
	}
	}

@AlexWaygood You are right!

[AlexWaygood]

@sunmy2019 thanks for checking!

[sunmy2019]

Oh, now I see the whole picture!

1. abc.ABCMeta cache __subclasscheck__ results if no exception occurs.
2. Protocol assigns a __subclasshook__, which raises or does not raise exceptions depending on the caller.
3. _ProtocolMeta.__instancecheck__ calls the __subclasshook__ in (2) (not raising exceptions), and the result is cached by (1)
4. User call (1), and accidentally get the cached result (not the exception).

[sunmy2019]

More details for (2) (3)

The __subclasshook__ in Protocol utilizes _allow_reckless_class_checks(). In normal cases, it is

_caller(0)=typing  _allow_reckless_class_checks
_caller(1)=typing  Protocol. _proto_hook
_caller(2)=abc     ABCMeta.__subclasscheck__
_caller(3)=__main__         <------- _allow_reckless_class_checks()=False

It then raises an exception.

In the problematic scenarios (3):

_caller(0)=typing   _allow_reckless_class_checks
_caller(1)=typing   Protocol. _proto_hook
_caller(2)=abc      ABCMeta.__subclasscheck__
_caller(3)=abc      ABCMeta.__instancecheck__              <------- _allow_reckless_class_checks()=True
_caller(4)=typing   _ProtocolMeta.__instancecheck__

It does raise an exception.