-
Notifications
You must be signed in to change notification settings - Fork 10.5k
[Diagnostics] Refactor diagnoseAmbiguityWithFixes. #29746
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
This will conflict with #29734 once merged |
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.
Looks good so far!
// TODO(diagnostics): The problem here is that `&` is interpreted as a binary operator, we need to re-think | ||
// how "missing member" fix is implemented because currently it finds N solutions with multiple fixes. | ||
takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{cannot invoke '' with no arguments}} | ||
takesP1AndP2([Swift.DoesNotExist & P1 & P2]()) // expected-error {{module 'Swift' has no member named 'DoesNotExist'}} |
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.
Nice!
@@ -186,7 +186,7 @@ func testMap() { | |||
} | |||
|
|||
// <rdar://problem/22414757> "UnresolvedDot" "in wrong phase" assertion from verifier | |||
[].reduce { $0 + $1 } // expected-error {{cannot invoke 'reduce' with an argument list of type '(_)'}} | |||
[].reduce { $0 + $1 } // expected-error {{missing argument for parameter #1 in call}} |
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.
An argument list of type... what?!
Nice improvement!
…ces between solutions in order to figure out the source of ambiguity.
fixes that appear in all solutions.
the ambiguity diagnostic by combining a few special errors into one.
…all" if the expression is not a function application.
…c parameters It's done by first retrieving all generic parameters from each solution, filtering boundings into distrinct set and diagnosing any differences. For example: ```swift func foo<T>(_: T, _: T) {} func bar(x: Int, y: Float) { foo(x, y) } ```
2278bdc
to
4d5c676
Compare
@hborla I have left all of your commits intact (expect for changes related to contextual result vs. operator ref), added |
…seConflictingGenericArguments`
@swift-ci please smoke test |
takeTuples((x, y), (x, y)) // expected-error{{cannot convert value of type '(Int, Float)' to expected argument type '(Float, Int)'}} | ||
|
||
takeTuples((x, y), (x, y)) | ||
// expected-error@-1 {{conflicting arguments to generic parameter 'U' ('Float' vs. 'Int')}} |
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.
Nice!
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.
@xedin I left 2 small comments, but otherwise your changes look great - especially the new "conflicting generic args" error message. Thank you!
@swift-ci please smoke test |
This change refactors
diagnoseAmbiguityWithFixes
to have a generalized strategy based on the differences between solutions in order to find the ambiguous overload, or other source of ambiguity. The new strategy will:FixKind
and locator.This change also introduces a new method on
CSFix
calleddiagnoseForAmbiguity
. This is called when there is no ambiguous overload, but the fix appears in each of the solutions.Resolves: rdar://problem/55605863