-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[red-knot] Narrowing for type(x) is C
checks
#14432
Conversation
Add type narrowing for `type(x) is C` conditions: ```py if type(x) is A: reveal_type(x) # revealed: A else: reveal_type(x) # revealed: A | B ```
ce53505
to
1143efd
Compare
x = get_a_or_b() | ||
|
||
if type(x) == A: | ||
reveal_type(x) # revealed: A | B |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Both pyright and mypy are less strict here and infer A
instead of A | B
. This is unsound, as can be demonstrated by returning B()
from get_a_or_b()
, and observing that this branch is taken.
CodSpeed Performance ReportMerging #14432 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
crates/red_knot_python_semantic/resources/mdtest/narrow/type.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/narrow/type.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/narrow/type.md
Outdated
Show resolved
Hide resolved
crates/red_knot_python_semantic/resources/mdtest/narrow/type.md
Outdated
Show resolved
Hide resolved
|
1f888e2
to
fefd47a
Compare
Ah, the performance regression is probably from removing this (now insufficient) check: if !left.is_name_expr() && comparators.iter().all(|c| !c.is_name_expr()) {
// If none of the comparators are name expressions,
// we have no symbol to narrow down the type of.
return None;
} I'll look into it. Edit: fixed now by introducing a similar check. |
Summary
Add type narrowing for
type(x) is C
conditions (andelse
clauses oftype(x) is not C
conditionals):closes: #14431, part of: #13694
Test Plan
New Markdown-based tests.