-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Rework narrowing logic for equality and identity #20492
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
Changes from 1 commit
28fefe6
cce6a71
cdbd737
4dbd107
4a552d5
ca5120d
94c5be7
92051d9
586d2a1
7bc5dac
1ef7ab7
5cc76be
54b4c8e
ce2728e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6549,7 +6549,10 @@ def comparison_type_narrowing_helper(self, node: ComparisonExpr) -> tuple[TypeMa | |
| and not is_false_literal(expr) | ||
| and not is_true_literal(expr) | ||
| and not self.is_literal_enum(expr) | ||
| and not (isinstance(expr_type, CallableType) and expr_type.is_type_obj()) | ||
| and not ( | ||
| isinstance(p_expr := get_proper_type(expr_type), CallableType) | ||
| and p_expr.is_type_obj() | ||
| ) | ||
| ): | ||
| h = literal_hash(expr) | ||
| if h is not None: | ||
|
|
@@ -6959,13 +6962,6 @@ def refine_identity_comparison_expression( | |
| else: | ||
| type_targets.append((i, TypeRange(expr_type, is_upper_bound=False))) | ||
|
|
||
| if False: | ||
| print() | ||
| print("operands", operands) | ||
| print("operand_types", operand_types) | ||
| print("operator_specific_targets", operator_specific_targets) | ||
| print("type_targets", type_targets) | ||
|
|
||
| partial_type_maps = [] | ||
|
|
||
| if operator_specific_targets: | ||
|
|
@@ -6997,19 +6993,7 @@ def refine_identity_comparison_expression( | |
| else_map = {} | ||
| partial_type_maps.append((if_map, else_map)) | ||
|
|
||
| final_if_map, final_else_map = reduce_conditional_maps( | ||
| partial_type_maps, use_meet=len(operands) > 2 | ||
| ) | ||
| if False: | ||
| print( | ||
| "final_if_map", | ||
| {str(k): str(v) for k, v in final_if_map.items()} if final_if_map else None, | ||
| ) | ||
| print( | ||
| "final_else_map", | ||
| {str(k): str(v) for k, v in final_else_map.items()} if final_else_map else None, | ||
| ) | ||
| return final_if_map, final_else_map | ||
| return reduce_conditional_maps(partial_type_maps, use_meet=len(operands) > 2) | ||
|
|
||
| def refine_away_none_in_comparison( | ||
| self, | ||
|
|
@@ -8534,9 +8518,10 @@ def and_conditional_maps(m1: TypeMap, m2: TypeMap, use_meet: bool = False) -> Ty | |
| for n1 in m1: | ||
| for n2 in m2: | ||
| if literal_hash(n1) == literal_hash(n2): | ||
| result[n1] = meet_types(m1[n1], m2[n2]) | ||
| if isinstance(result[n1], UninhabitedType): | ||
| meet_result = meet_types(m1[n1], m2[n2]) | ||
| if isinstance(meet_result, UninhabitedType): | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was a preexisting bug that gets exposed. We now use this logic for chained comparisons, in place of the bespoke logic previously |
||
| return None | ||
| result[n1] = meet_result | ||
| return result | ||
|
|
||
|
|
||
|
|
@@ -8614,7 +8599,7 @@ def is_singleton_value(t: Type) -> bool: | |
| "builtins.bytes", | ||
| "builtins.bytearray", | ||
| "builtins.memoryview", | ||
| "builtins.tuple", | ||
| # for Collection[Never] | ||
| "builtins.list", | ||
| "builtins.dict", | ||
| "builtins.set", | ||
|
|
@@ -8627,7 +8612,10 @@ def has_custom_eq_checks(t: Type) -> bool: | |
| or custom_special_method(t, "__ne__", check_all=False) | ||
| # TODO: make the hack more principled. explain what and why we're doing this though | ||
| # custom_special_method has special casing for builtins | ||
| or (isinstance(t, Instance) and t.type.fullname in BUILTINS_CUSTOM_EQ_CHECKS) | ||
| or ( | ||
| isinstance(pt := get_proper_type(t), Instance) | ||
| and pt.type.fullname in BUILTINS_CUSTOM_EQ_CHECKS | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe there is a better way to do this, at least for collections. Will explore |
||
| ) | ||
| ) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,6 +9,7 @@ | |
|
|
||
| import argparse | ||
| import sys | ||
| from typing import Any, cast | ||
|
|
||
| from mypy.main import infer_python_executable, process_options | ||
| from mypy.options import Options | ||
|
|
@@ -63,7 +64,7 @@ def test_executable_inference(self) -> None: | |
|
|
||
| # first test inferring executable from version | ||
| options = Options() | ||
| options.python_executable = None | ||
| options.python_executable = cast(Any, None) | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A little unfortunate, because |
||
| options.python_version = sys.version_info[:2] | ||
| infer_python_executable(options, special_opts) | ||
| assert options.python_version == sys.version_info[:2] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -194,12 +194,12 @@ def test_generic_function_type(self) -> None: | |
|
|
||
| def test_type_alias_expand_once(self) -> None: | ||
| A, target = self.fx.def_alias_1(self.fx.a) | ||
| assert get_proper_type(A) == target | ||
| assert get_proper_type(target) == target | ||
| assert get_proper_type(A) == target | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fun, from this assert mypy now knows that |
||
|
|
||
| A, target = self.fx.def_alias_2(self.fx.a) | ||
| assert get_proper_type(A) == target | ||
| assert get_proper_type(target) == target | ||
| assert get_proper_type(A) == target | ||
|
|
||
| def test_recursive_nested_in_non_recursive(self) -> None: | ||
| A, _ = self.fx.def_alias_1(self.fx.a) | ||
|
|
||
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.
Shouldn't these be
LITERAL_YES? I guess maybe they aren't now, but that seems like the right distinction.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.
Yeah, good point. Good thing to explore in a follow up PR