Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 12 commits into from
May 17, 2023
36 changes: 36 additions & 0 deletions Lib/test/test_typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -2654,6 +2654,42 @@ class D(PNonCall): ...
with self.assertRaises(TypeError):
issubclass(D, PNonCall)

def test_no_weird_caching_with_issubclass_after_isinstance(self):
@runtime_checkable
class Spam(Protocol):
x: int

class Eggs:
def __init__(self) -> None:
self.x = 42

# gh-104555: ABCMeta might cache the result of this isinstance check
# if we called super().__instancecheck__ in the wrong place
# in _ProtocolMeta.__instancecheck__...
self.assertIsInstance(Eggs(), Spam)

# ...and if it did, then TypeError wouldn't be raised here!
with self.assertRaises(TypeError):
issubclass(Eggs, Spam)

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

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

# gh-104555: ABCMeta might cache the result of this isinstance check
# if we called super().__instancecheck__ in the wrong place
# in _ProtocolMeta.__instancecheck__...
self.assertIsInstance(Eggs(42), Spam)

# ...and if it did, then TypeError wouldn't be raised here!
with self.assertRaises(TypeError):
issubclass(Eggs, Spam)

def test_protocols_isinstance(self):
T = TypeVar('T')

Expand Down
10 changes: 8 additions & 2 deletions Lib/typing.py
Original file line number Diff line number Diff line change
Expand Up @@ -1798,7 +1798,13 @@ def __instancecheck__(cls, instance):
raise TypeError("Instance and class checks can only be used with"
" @runtime_checkable protocols")

if super().__instancecheck__(instance):
# gh-104555: Don't call super().__instancecheck__ here,
# ABCMeta.__instancecheck__ would erroneously use it to populate the cache,
# which would cause incorrect results for *issubclass()* calls
if type.__instancecheck__(cls, instance):
return True

if cls.__callable_proto_members_only__ and issubclass(type(instance), cls):
return True
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


if is_protocol_cls:
Expand All @@ -1813,7 +1819,7 @@ def __instancecheck__(cls, instance):
else:
return True

return False
return super().__instancecheck__(instance)


class Protocol(Generic, metaclass=_ProtocolMeta):
Expand Down