-
-
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,6 +6549,7 @@ 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()) | ||
| ): | ||
| h = literal_hash(expr) | ||
| if h is not None: | ||
|
|
@@ -6658,11 +6659,7 @@ def equality_type_narrowing_helper( | |
| # If we haven't been able to narrow types yet, we might be dealing with a | ||
| # explicit type(x) == some_type check | ||
|
Collaborator
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. Isn't this comment in the wrong place/wrong now?
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. I guess so. One of the commits I have on top of this PR gets rid of this function entirely, so won't touch for now |
||
| if_map, else_map = self.narrow_type_by_equality( | ||
| operator, | ||
| operands, | ||
| operand_types, | ||
| expr_indices, | ||
| narrowable_indices=narrowable_indices, | ||
| operator, operands, operand_types, expr_indices, narrowable_indices=narrowable_indices | ||
|
Collaborator
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. Any deeper reason behind why specifically this arg and none of the others is consistently passed via kwarg?
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. No particularly deep reason
|
||
| ) | ||
| if node is not 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. As above, it is more common to have things in
Collaborator
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.
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. Yup! I have this refactored in a later commit in my stack :-) |
||
| type_if_map, type_else_map = self.find_type_equals_check(node, expr_indices) | ||
|
|
@@ -6962,12 +6959,12 @@ def refine_identity_comparison_expression( | |
| else: | ||
| type_targets.append((i, TypeRange(expr_type, is_upper_bound=False))) | ||
|
|
||
| # print = lambda *a: None | ||
| # print() | ||
| # print("operands", operands) | ||
| # print("operand_types", operand_types) | ||
| # print("operator_specific_targets", operator_specific_targets) | ||
| # print("type_targets", type_targets) | ||
| 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 = [] | ||
|
|
||
|
|
@@ -7000,9 +6997,18 @@ 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) | ||
| # print("final_if_map", {str(k): str(v) for k, v in final_if_map.items()}) | ||
| # print("final_else_map", {str(k): str(v) for k, v in final_else_map.items()}) | ||
| 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 | ||
|
|
||
| def refine_away_none_in_comparison( | ||
|
|
||
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