-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Diagnostics] Improve diagnostics of overloaded mutating methods #5808
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
Conversation
/cc @rudkx |
@swift-ci Please smoke test |
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.
A few typos that would be great to fix before merging, but otherwise LGTM.
// let answer: Int = 42 | ||
// S().f(answer) | ||
// | ||
// Constraint system generator is going to peek `f(_ f: Float)` as |
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.
typo: peek instead of pick
// Such leads to incorrect contextual conversion failure diagnostic because | ||
// type of the argument is going to resolved as (Int) no matter what. | ||
// To workaround that fact and improve diagnostic of such cases we are going | ||
// to try and collect all unviable canddidates for a given call and check if |
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.
typo: canddidates
// already established type of the argument expression. | ||
unviableCandidates.filterList(argType, argLabels); | ||
|
||
// If one of the inviable candidates matches arguments exactly, |
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.
typo: inviable
Add special logic to FailureDiagnosis::visitApplyExpr to handle situation like following: struct S { mutating func f(_ i: Int) {} func f(_ f: Float) {} } Given struct has an overloaded method "f" with a single argument of multiple different types, one of the overloads is marked as "mutating", which means it can only be applied on LValue base type. So when struct is used like this: let answer: Int = 42 S().f(answer) Constraint system generator is going to pick `f(_ f: Float)` as only possible overload candidate because "base" of the call is immutable and contextual information about argument type is not available yet. Such leads to incorrect contextual conversion failure diagnostic because type of the argument is going to resolved as (Int) no matter what. To workaround that fact and improve diagnostic of such cases we are going to try and collect all unviable candidates for a given call and check if at least one of them matches established argument type before even trying to re-check argument expression. Resolves: <rdar://problem/28051973>.
@rudkx Fixed all of the typos (in commit message as well), sorry that this keeps happening... |
@swift-ci Please smoke test and merge. |
@rudkx looks like CI which was building macOS crashed :( |
@xedin I'll try kicking it off again in a little while. |
@swift-ci Please smoke test and merge |
Add special logic to FailureDiagnosis::visitApplyExpr to
handle situation like following:
Given struct has an overloaded method "f" with a single argument of
multiple different types, one of the overloads is marked as
"mutating", which means it can only be applied on LValue base type.
So when struct is used like this:
Constraint system generator is going to pick
f(_ f: Float)
asonly possible overload candidate because "base" of the call is immutable
and contextual information about argument type is not available yet.
Such leads to incorrect contextual conversion failure diagnostic because
type of the argument is going to resolved as (Int) no matter what.
To workaround that fact and improve diagnostic of such cases we are going
to try and collect all unviable candidates for a given call and check if
at least one of them matches established argument type before even trying
to re-check argument expression.
Resolves: rdar://problem/28051973.