Better model runtime in isinstance and type checks #20675
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Primer here is a lie, exposes a reachability issue. Will put in a fix |
This comment has been minimized.
This comment has been minimized.
|
Okay great, #20677 fixes a lot of the false unreachability |
|
steam is an improvement (that I have a test case for) |
This comment has been minimized.
This comment has been minimized.
| yield values | ||
| raise | ||
|
|
||
| class A: ... |
There was a problem hiding this comment.
we don't change behaviour on f1,f2,g1,g2
but there is a latent pre-existing issue in conditional_types that could be exposed in the future, see discussion in #20677 (comment)
|
Diff from mypy_primer, showing the effect of this PR on open source code: beartype (https://github.com/beartype/beartype)
+ beartype/_util/hint/pep/proposal/pep484585/generic/pep484585genget.py:981: error: Item "object" of "object | Any" has no attribute "__qualname__" [union-attr]
+ beartype/_util/hint/pep/utilpepsign.py:481: error: Item "object" of "object | Any" has no attribute "__qualname__" [union-attr]
steam.py (https://github.com/Gobot1234/steam.py)
+ steam/_const.py:189: error: Unused "type: ignore" comment [unused-ignore]
static-frame (https://github.com/static-frame/static-frame)
+ static_frame/core/interface.py:1577: error: Argument "cls_interface" to "gen_from_accessor" of "InterfaceRecord" has incompatible type "type[object]"; expected "type[Interface]" [arg-type]
|
JukkaL
left a comment
JukkaL
left a comment
There was a problem hiding this comment.
Thanks! Can you add a description that explains the primary behavioral changes?
| # Type[A] means "any type that is a subtype of A" rather than "precisely type A" | ||
| # we indicate this by setting is_upper_bound flag | ||
| is_upper_bound = True | ||
| if isinstance(typ.item, NoneType): |
| return TypeRange(typ, is_upper_bound=False) | ||
| if isinstance(typ, Instance) and typ.type.fullname == "builtins.type": | ||
| object_type = Instance(typ.type.mro[-1], []) | ||
| return TypeRange(object_type, is_upper_bound=True) |
|
Thanks, I added some description. The final classes stuff is in code I'm not really changing. This diff is probably best reviewed with the "ignore whitespace" setting. In the big part of the diff the net new code is really just these two bits: https://github.com/python/mypy/pull/20675/files#diff-f96a2d6138bc6cdf2a07c4d37f6071cc25c1631afc107e277a28d5b59fc0ef04R7976-R7984 I had to split the function because the forms allowed in |
2 participants
We now more accurately model reachability for impossible checks. We also avoid allowing tuples and X | Y syntax in situations where they don't work at runtime.