-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[conflict] Fix #1855: Multiassign from Union #2154
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
7e043d0
9a50d73
5c4d86e
60cfbbb
5e9e0f2
71f8475
42b6e73
0560bd8
7683ee2
8f0316f
fde83d5
2fbf387
da3a516
ab60317
6bb5519
0d2c594
2c73db5
d2c6e8b
5c03ac9
c3e1b35
87f7d90
c752b42
5f0d02c
bec9454
a9fac0b
ea23ac0
4a34623
2db6a11
6151d20
4a457fc
c8a9b52
af0b24a
d5c5c02
640921b
20aea69
a3b6546
91a2275
bc4a390
2e6cb95
d6a7069
eb16d1d
dd71ddc
38651c4
9000099
f20f3d6
61be4e9
9830cb4
59dc8b7
7f304e4
ff1ca80
168087e
3f198cf
4fa059f
ab35a4c
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 |
---|---|---|
|
@@ -1178,12 +1178,11 @@ def check_multi_assign_from_union(self, lvalues: List[Expression], rvalue: Expre | |
union_types = tuple(join_type_list(col) for col in transposed) | ||
for union, lv in zip(union_types, lvalues): | ||
_1, _2, inferred = self.check_lvalue(lv) | ||
# self.binder.assign_type(lv, union, self.binder.get_declaration(lv)) | ||
if inferred: | ||
self.set_inferred_type(inferred, lv, union) | ||
else: | ||
self.store_type(lv, union) | ||
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. The binding of the joined type is "retrofitted". I am not sure this is the best way to do it - maybe inferred vars should simply accumulate through joins? 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. Retrofitting like this may not be a problem, but I suspect it might be. Accumulating through joins might not be any better. The most correct approach would be to infer the entire type for an lvalue before setting it, so that intermediate values can't leak. I don't know, but it may be possible that the intermediate lvalue types leak to the rvalue in some weird corner cases. I'd recommend not worrying about this too much, but I'd suggest adding a TODO comment about the potential issues. |
||
print(lv.literal) | ||
self.binder.assign_type(lv, union, self.binder.get_declaration(lv)) | ||
|
||
def check_multi_assign_from_tuple(self, lvalues: List[Lvalue], rvalue: Expression, | ||
rvalue_type: TupleType, context: Context, | ||
|
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.
You'll probably need to put the new type to the binder (maybe
self.binder.assign_type(...)
). Also add a test case for this. Example where this may make a difference: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.
The assignment to
a
is not accepted by mypy. I opened #2164There 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.
Simple
assign_type
did not work; I needed something more intrusive than that. I don't like this solution, to be honest. I want to ask the type to split itself, without side effects, but that's not how things works.