Skip to content

[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

Merged
merged 1 commit into from
Nov 29, 2016

Conversation

xedin
Copy link
Contributor

@xedin xedin commented Nov 15, 2016

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.

@xedin
Copy link
Contributor Author

xedin commented Nov 15, 2016

/cc @rudkx

@rudkx rudkx self-assigned this Nov 16, 2016
@rudkx
Copy link
Contributor

rudkx commented Nov 28, 2016

@swift-ci Please smoke test

Copy link
Contributor

@rudkx rudkx left a 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
Copy link
Contributor

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
Copy link
Contributor

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,
Copy link
Contributor

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>.
@xedin
Copy link
Contributor Author

xedin commented Nov 28, 2016

@rudkx Fixed all of the typos (in commit message as well), sorry that this keeps happening...

@rudkx
Copy link
Contributor

rudkx commented Nov 28, 2016

@swift-ci Please smoke test and merge.

@xedin
Copy link
Contributor Author

xedin commented Nov 28, 2016

@rudkx looks like CI which was building macOS crashed :(

@rudkx
Copy link
Contributor

rudkx commented Nov 28, 2016

@xedin I'll try kicking it off again in a little while.

@rudkx
Copy link
Contributor

rudkx commented Nov 29, 2016

@swift-ci Please smoke test and merge

@swift-ci swift-ci merged commit fe1cd74 into swiftlang:master Nov 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants