Skip to content

Safer treatment of @runtime protocols in isinstance() #3827

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.

Sign up for GitHub

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

Jump to bottom
Closed
ilevkivskyi opened this issue Aug 14, 2017 · 4 comments
Closed

Safer treatment of @runtime protocols in isinstance() #3827

ilevkivskyi opened this issue Aug 14, 2017 · 4 comments
Labels
bug mypy got something wrong priority-0-high topic-protocols topic-reachability Detecting unreachable code

Comments

@ilevkivskyi
Copy link
Member

Currently, isinstance with protocols uses the same logic as with nominal classes, i.e. only erases the type variables, but sometimes this may be not safe (as discussed in PEP 544), for example:

@runtime
class P(Protocol):
    attr: int

class C:
    attr = 'Hi!'

if isinstance(C(), P):
    # this branch is considered unreachable by mypy, but will be executed

The idea is to perform a "deeper" erasure (setting all attribute types to Any) when considering isinstance() with protocols, thus mimicking the runtime behaviour.
@ilevkivskyi
Copy link
Member Author

This is a follow-up for #3132

@ilevkivskyi ilevkivskyi self-assigned this Aug 14, 2017
@ilevkivskyi ilevkivskyi mentioned this issue Aug 15, 2017
Merged
@ilevkivskyi ilevkivskyi mentioned this issue Sep 1, 2017
Merged
@gvanrossum
Copy link
Member

I'm curious how far this should go. With the @runtime decorator IIUC an object with a method that has the same name as the protocol but a different signature may be considered an instance of that protocol. Are you proposing to erase this too? (I guess it's fine, but your example and wording aren't very clear about it -- "attributes" is sometimes assumed not to include methods.)

@ilevkivskyi
Copy link
Member Author

I'm curious how far this should go.

TBH I am not sure myself, this is why "discussion needed" label :-) One possible option is to erase types of all attributes (both methods and variables) completely. But maybe this is too radical, and we can for example only erase types of instance variables, but keep the fact that methods are callable (so that they will become Callable[..., Any] after erasure).

@ilevkivskyi ilevkivskyi mentioned this issue Apr 2, 2018
Merged
@gvanrossum gvanrossum mentioned this issue Apr 3, 2018
Closed
@ilevkivskyi ilevkivskyi added bug mypy got something wrong priority-0-high and removed priority-1-normal labels Apr 7, 2018
@ilevkivskyi ilevkivskyi mentioned this issue Jul 5, 2018
Closed
@ilevkivskyi ilevkivskyi mentioned this issue Nov 12, 2019
Closed
@ilevkivskyi ilevkivskyi removed their assignment Dec 9, 2019
@AlexWaygood
Copy link
Member

Mypy no longer considers the code in the original example unreachable, so I'm closing this issue. (If there's more to be done, feel free to reopen.)

@AlexWaygood AlexWaygood added topic-reachability Detecting unreachable code and removed needs discussion labels Mar 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug mypy got something wrong priority-0-high topic-protocols topic-reachability Detecting unreachable code
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gvanrossum @ilevkivskyi @AlexWaygood