gh-113320: Reduce the number of dangerous getattr() calls when constructing protocol classes
#113401
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n constructing protocol classes
AlexWaygood
commented
Dec 23, 2023
|
Runtime protocols with members that raised when accessed on the class object have never worked — not even on Python <=3.11 (see #113320 (comment)) — so I don't think there are any backwards-compatibility issues here w.r.t. raising an error if someone tries to create one |
|
Thanks for the review! |
|
Thanks @AlexWaygood for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12. |
|
Sorry, @AlexWaygood, I could not cleanly backport this to |
|
GH-113722 is a backport of this pull request to the 3.12 branch. |
AlexWaygood
added a commit
to AlexWaygood/cpython
that referenced
this pull request
Jan 5, 2024
…lls when constructing protocol classes (python#113401) - Only attempt to figure out whether protocol members are "method members" or not if the class is marked as a runtime protocol. This information is irrelevant for non-runtime protocols; we can safely skip the risky introspection for them. - Only do the risky getattr() calls in one place (the runtime_checkable class decorator), rather than in three places (_ProtocolMeta.__init__, _ProtocolMeta.__instancecheck__ and _ProtocolMeta.__subclasscheck__). This reduces the number of locations in typing.py where the risky introspection could go wrong. - For runtime protocols, if determining whether a protocol member is callable or not fails, give a better error message. I think it's reasonable for us to reject runtime protocols that have members which raise strange exceptions when you try to access them. PEP-544 clearly states that all protocol member must be callable for issubclass() calls against the protocol to be valid -- and if a member raises when we try to access it, there's no way for us to figure out whether it's a callable member or not! (cherry-picked from commit ed6ea3e)
AlexWaygood
added a commit
that referenced
this pull request
Jan 5, 2024
…en constructing protocol classes (#113401) (#113722) - Only attempt to figure out whether protocol members are "method members" or not if the class is marked as a runtime protocol. This information is irrelevant for non-runtime protocols; we can safely skip the risky introspection for them. - Only do the risky getattr() calls in one place (the runtime_checkable class decorator), rather than in three places (_ProtocolMeta.__init__, _ProtocolMeta.__instancecheck__ and _ProtocolMeta.__subclasscheck__). This reduces the number of locations in typing.py where the risky introspection could go wrong. - For runtime protocols, if determining whether a protocol member is callable or not fails, give a better error message. I think it's reasonable for us to reject runtime protocols that have members which raise strange exceptions when you try to access them. PEP-544 clearly states that all protocol member must be callable for issubclass() calls against the protocol to be valid -- and if a member raises when we try to access it, there's no way for us to figure out whether it's a callable member or not! (cherry-picked from commit ed6ea3e)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
getattr()calls in one place (theruntime_checkableclass decorator), rather than in three places (_ProtocolMeta.__init__,_ProtocolMeta.__instancecheck__and_ProtocolMeta.__subclasscheck__). This reduces the number of locations intyping.pywhere the risky introspection could go wrong.issubclass()calls against the protocol to be valid -- and if a member raises when we try to access it, there's no way for us to figure out whether it's a callable member or not!Cc. @Gobot1234