Choose best type when working with unioned overloads#5242
Choose best type when working with unioned overloads#5242ilevkivskyi merged 4 commits intopython:masterfrom
Conversation
Michael0x2a
left a comment
Michael0x2a
left a comment
There was a problem hiding this comment.
Just a few minor nitpicks, but otherwise LGTM!
mypy/checkexpr.py
Outdated
| union_success = True | ||
|
|
||
| # Step 3: If the union math fails, or if there was no union in the argument types, | ||
| # we fall back to checking each branch one-by-one. |
There was a problem hiding this comment.
We should probably tweak this comment -- maybe just something simple like
# Step 3: We try checking each branch one-by-one
?
I don't think it matters too much, as long as we remove the "if the union math fails" bit.
mypy/checkexpr.py
Outdated
| if not unioned_errors.is_errors(): | ||
| # Success! Stop early. | ||
| return unioned_result | ||
| # Success! But we need to see maybe normal procedure gives a narrower type. |
There was a problem hiding this comment.
"need to see maybe normal procedure" -> "need to see if maybe the normal procedure"
mypy/checkexpr.py
Outdated
| # Success! Stop early. | ||
| return unioned_result | ||
| # Success! But we need to see maybe normal procedure gives a narrower type. | ||
| union_success = True |
There was a problem hiding this comment.
I think we can simplify this if statement to union_success = not unioned_errors.is_errors(). We'd also need to change the comment above to # Record if we succeeded. Next we need to see if maybe...
| assert unioned_result is not None | ||
| if is_subtype(inferred_result[0], unioned_result[0]): | ||
| return inferred_result | ||
| return unioned_result |
There was a problem hiding this comment.
Actually, just had another thought -- I think it would be clearer (and a little more efficient) if we added logic to return early if union_success is true somewhere around here. Currently, we return early only if the normal algorithm also succeeds.
(The logic currently does the right thing because step 5 happens to do what we want, but step 4 and down are supposed to be failure cases.)
2 participants
Fixes #5240
This is a simple straightforward fix.