Repository navigation
Typing: runtime-checkable protocols are broken on main #104555
Description
Activity
- addedtype-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error
on May 16, 2023 (I'm working on a fix)
Reacted by Ken JinThanks for catching! I suppose this is about
__type_params__?Thanks for catching! I suppose this is about
__type_params__?Yes.
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
TypeErrorwasn'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()
Anyway, I'll submit a PR without that test for now...
I'll look into this more.
Reacted by Alex WaygoodOkay, cool. I'll enable automerge on my PR for now, as it definitely gets us closer to correct behaviour.
Reacted by Jelle ZijlstraThe issue you saw also reproduces for me without PEP 695 syntax. It seems to be due to the
_allow_reckless_class_checkscheck, which somehow thinks the caller isabcif we're in a test, but not if we're in the REPL.Aha, so we've discovered a bug, it just isn't a new bug.
It's possible this bug was introduced by the fact that Generic is now a C class, haven't tried on 3.11 yet.
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()
6 remaining items
47753ecde21b79b5c5f11d883946fda2a340e427 is the first bad commit commit 47753ecde21b79b5c5f11d883946fda2a340e427 Author: Alex Waygood <Alex.Waygood@Gmail.com> 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(-)Reacted by Alex Waygood and Kirill Podoprigora47753ecde21b79b5c5f11d883946fda2a340e427 is the first bad commit commit 47753ecde21b79b5c5f11d883946fda2a340e427 Author: Alex Waygood <Alex.Waygood@Gmail.com> 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 :)
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
@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
depthvalue we pass to_allow_reckless_class_checks()?Oh sorry, ignore my previous comment.
Reacted by Alex WaygoodConfirmed that the code path that does not raise TypeError goes through L606.
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!
Reacted by Alex Waygood@sunmy2019 thanks for checking!
Oh, now I see the whole picture!
abc.ABCMetacache__subclasscheck__results if no exception occurs.Protocolassigns 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).
Reacted by Alex WaygoodMore details for (2) (3)
The
__subclasshook__inProtocolutilizes_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()=FalseIt 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, 2023 - added a commit that references this issue
on May 17, 2023
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:
And here's the behaviour you get on
mainwith protocols that use PEP 695 syntax, which is incorrect:Linked PRs
isinstance()influence whetherissubclass()raises an exception #104559