[red-knot] Support for not equal narrowing #13749
Conversation
Lexxxzy
requested review from
carljm,
MichaReiser and
AlexWaygood
as code owners
|
I have a question: Why isn’t there a case for (1 positive, 1 negative) in the match statement of InnerIntersectionBuilder::build? It seems like (1 & ~0) is essentially just 1. Asking because for this example: x = None if flag else 1
y = 0
if x != 1:
y = x
reveal_type(y) # revealed: None & ~Literal[1]
reveal_type(y) # revealed: Literal[0] | None & ~Literal[1]
|
|
| x = None if flag else 1 | ||
| y = 0 | ||
| if x != None: | ||
| y = x | ||
|
|
||
| reveal_type(x) # revealed: None | Literal[1] | ||
| reveal_type(y) # revealed: Literal[0, 1] |
There was a problem hiding this comment.
With the new test framework, we can now write this in a shorter way, without using additional variables:
| x = None if flag else 1 | |
| y = 0 | |
| if x != None: | |
| y = x | |
| reveal_type(x) # revealed: None | Literal[1] | |
| reveal_type(y) # revealed: Literal[0, 1] | |
| x = None if flag else 1 | |
| if x != None: | |
| reveal_type(x) # revealed: Literal[1] | |
| reveal_type(x) # revealed: None | Literal[1] |
| @@ -155,7 155,7 @@ impl<'db> NarrowingConstraintsBuilder<'db> { | |||
| let inference = infer_expression_types(self.db, expression); | |||
| for (op, comparator) in std::iter::zip(&**ops, &**comparators) { | |||
| let comp_ty = inference.expression_ty(comparator.scoped_ast_id(self.db, scope)); | |||
| if matches!(op, ast::CmpOp::IsNot) { | |||
| if matches!(op, ast::CmpOp::IsNot | ast::CmpOp::NotEq) { | |||
There was a problem hiding this comment.
This branch needs the same fix that I did for the IsNot branch here: #13758. If you re-apply this | ast::CmpOp::NotEq to the current version on main, that should work automatically.
If we don't filter out non-singleton types, you would get a type of list[int] & ~list[int] == Never in a case like this:
x = [1]
y = [1, 2]
if x != y:
reveal_type(x) # should still have type list[int]!It might make sense to add an explicit test for this as well, even though we have one for is not already. Just in case we ever split the IsNot/NotEq branches for some reason.
| @@ -0,0 1,15 @@ | |||
| # Conditionals | |||
There was a problem hiding this comment.
Can you please move this test to crates/red_knot_python_semantic/resources/mdtest/narrow/conditionals_not_equals.md and adapt the structure to the what the other files in this folder have?
|
|
||
| ## Narrowing | ||
|
|
||
| ### `is != None` |
There was a problem hiding this comment.
| ### `is != None` | |
| ### `x != None` |
There is a case for that. The match (self.positive.len(), self.negative.len()) {
(0, 0) => Type::Never,
(1, 0) => self.positive[0],
_ => {
self.positive.shrink_to_fit();
self.negative.shrink_to_fit();
Type::Intersection(IntersectionType::new(db, self.positive, self.negative))
}
}There's still the question on how to simplify those intersection types. For example,
It is. This simplification is exactly what the
If we ignore the problem that |
Summary
Planning to support type narrowing for
!=expression as stated in #13694.Test Plan
Add tests in new md format.