[red-knot] Binary operator inference for union types#16601
Merged
Conversation
Contributor
|
This comment was marked as resolved.
This comment was marked as resolved.
sharkdp
force-pushed
the
david/binary-operator-unions
branch
from
0303719 to
2273374
Compare
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
sharkdp
added a commit
that referenced
this pull request
Mar 10, 2025
## Summary Strip ANSI codes in the mypy_primer diff before uploading. ## Test Plan Successful run here: #16601
sharkdp
force-pushed
the
david/binary-operator-unions
branch
from
2273374 to
86933b9
Compare
This comment was marked as resolved.
This comment was marked as resolved.
AlexWaygood
pushed a commit
that referenced
this pull request
Mar 10, 2025
## Summary Strip ANSI codes in the mypy_primer diff before uploading. ## Test Plan Successful run here: #16601
sharkdp
force-pushed
the
david/binary-operator-unions
branch
from
86933b9 to
e06ac75
Compare
sharkdp
force-pushed
the
david/binary-operator-unions
branch
from
e06ac75 to
760bbd4
Compare
sharkdp
commented
Mar 11, 2025
| (Type::Union(lhs_union), rhs, _) => { | ||
| let mut union = UnionBuilder::new(self.db()); | ||
| for lhs in lhs_union.elements(self.db()) { | ||
| let result = self.infer_binary_expression_type(*lhs, rhs, op)?; |
Contributor
Author
There was a problem hiding this comment.
Instead of short-circuiting here, we could probably also build a better diagnostic, telling you which exact element-combination didn't work. But it seems low priority and doesn't necessarily have to be included here, I think.
sharkdp
requested review from
AlexWaygood,
MichaReiser and
carljm
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This fixes a bug I noticed while looking at ecosystem results. The MRE version of it is this:
It's yet another instance of the general problem that our way of representing union types does not prevent us from handling unions (and intersections) incorrectly. The pattern is usually this: we implement operation X by explicitly matching on some cases that we handle explicitly (e.g. literals). For all other types, we fall back to some generic behavior (calling some functions that work for every
Type). That generic behavior also compiles forType::Union/Type::Intersection, but what we really want is to recursively call X for each element, which also gives us that special behavior for elements of the union.For intersections, I'll open a ticket.
Test Plan