GeezerVison is showing github.com — Typing: runtime-checkable protocols are broken on `main` · Issue #104555 · python/cpython
Typing: runtime-checkable protocols are broken on main #104555DescriptionPEP-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)
TrueAnd 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
ActivityAlexWaygood commented on May 16, 2023 JelleZijlstra commented on May 16, 2023 Thanks for catching! I suppose this is about __type_params__? AlexWaygood commented on May 16, 2023
Thanks for catching! I suppose this is about __type_params__?
Yes. AlexWaygood commented on May 16, 2023 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 commented on May 16, 2023 Anyway, I'll submit a PR without that test for now... JelleZijlstra commented on May 16, 2023 I'll look into this more. AlexWaygood commented on May 16, 2023 Okay, cool. I'll enable automerge on my PR for now, as it definitely gets us closer to correct behaviour. Reacted by Jelle Zijlstra JelleZijlstra commented on May 16, 2023 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 commented on May 16, 2023 Aha, so we've discovered a bug, it just isn't a new bug. JelleZijlstra commented on May 16, 2023 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 commented on May 16, 2023 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()JelleZijlstra commented on May 16, 2023 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 Reacted by Alex Waygood and Kirill Podoprigora AlexWaygood commented on May 16, 2023
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 commented on May 16, 2023 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)
Falsesunmy2019 commented on May 16, 2023 AlexWaygood commented on May 16, 2023 @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 commented on May 16, 2023 Oh sorry, ignore my previous comment. sunmy2019 commented on May 16, 2023 Confirmed that the code path that does not raise TypeError goes through L606.
|
/* 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 commented on May 16, 2023 sunmy2019 commented on May 16, 2023 Oh, now I see the whole picture!
abc.ABCMeta cache __subclasscheck__ results if no exception occurs.Protocol assigns a __subclasshook__, which raises or does not raise exceptions depending on the caller._ProtocolMeta.__instancecheck__ calls the __subclasshook__ in (2) (not raising exceptions), and the result is cached by (1)- User call (1), and accidentally get the cached result (not the exception).
sunmy2019 commented on May 16, 2023 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. added a commit that references this issue on May 17, 2023gh-104555: Runtime-checkable protocols: Don't let previous calls to `…
MetadataMetadataDevelopmentNo branches or pull requestsIssue actions
|