[Diagnostics] Prioritize type mismatches over labeling failures for c… #36267
Conversation
…alls Since labels are considered part of the name, mismatches in labeling should be invalidate overload choices. Let's prefer an overload with correct labels but incorrect types over the one with incorrect labels. This also means that it's possible to restore performance optimizations related to early filtering in diagnostic mode, which is important for deeply nested code i.e. SwiftUI views.
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS platform |
|
@swift-ci please smoke test Windows platform |
| // Don't attempt further optimization in "diagnostic mode" because | ||
| // in such mode we'd like to attempt all of the available overloads | ||
| // regardless of problems related to missing or extraneous labels | ||
| // and/or arguments. |
There was a problem hiding this comment.
I was thinking about this a little more and wondering - this is going to make the solver totally skip over overloads with labeling failures if it's able to fix up a different overload with correct labels, right? What's going to happen now if there's 1 label mismatch and N type mismatches in the expression?
There was a problem hiding this comment.
If there are overloads that have correct labels in expected positions than they'd be checked and ranked based on the number/kind of type failures they have, the rest is skipped, if there are no matching overloads - all of the overloads are going to be checked to determine what is going on (maybe there are out-of-order arguments or extraneous labels). The reasoning behind this is based on to the fact that name with incorrect labels shouldn't be found by the lookup anyway since labels are part of the name.
There was a problem hiding this comment.
The reasoning behind this is based on to the fact that name with incorrect labels shouldn't be found by the lookup anyway since labels are part of the name.
That's true for a compound name, but not a function call. I believe lookup for a function call just uses the bare name (e.g. to get function calls working with $ labels for SE-0293, all I had to do was change argument-to-parameter label matching)
There was a problem hiding this comment.
It's the case for all the names, since labels attached to the name in ABI.
There was a problem hiding this comment.
That said lookup works on the bare name still and most likely will do so in the future.
There was a problem hiding this comment.
I understand that labels are part of the name for all names. I'm saying that lookup sometimes does not include the labels, and instead relies on argument-to-parameter matching.
There was a problem hiding this comment.
What I am asking is - what diagnostic will be produced for code like this, which has only 1 label mismatch but several type mismatches?
func test(arg1: Int, arg2: Int) {}
func test(arg: String, arg2: String) {}
test(arg1: "", arg2: "")There was a problem hiding this comment.
I understand what the concern is here, that's why it is working the way it is :) The idea behind these changes is that matching labels is strong enough indication that developer wanted a particular overload even though the types might not line up at the moment (e.g. they have been filled by completion with holes), so in your example it's going to be two type failures related to arguments instead of one label failure.
That said - there is still a possibility (and that's what your example indicates as well) that one of the labels has a typo, I think we could detect that during label matching and allow such overloads to be attempted in the diagnostic mode together with the ones where labels line up exactly.
|
@swift-ci please smoke test |
|
@swift-ci please smoke test macOS platform |
|
@swift-ci please smoke test Linux platform |
…tion choices" This reverts commit 2df4ba7 from swiftlang#36300 which conflicts with swiftlang#36267 and was missed because of classic "time of test was too old before merging".
| @@ -648,7 +648,7 @@ let arr = [BottleLayout]() | |||
| let layout = BottleLayout(count:1) | |||
| let ix = arr.firstIndex(of:layout) // expected-error {{referencing instance method 'firstIndex(of:)' on 'Collection' requires that 'BottleLayout' conform to 'Equatable'}} | |||
|
|
|||
| let _: () -> UInt8 = { .init("a" as Unicode.Scalar) } // expected-error {{missing argument label 'ascii:' in call}} | |||
| let _: () -> UInt8 = { .init("a" as Unicode.Scalar) } // expected-error {{initializer 'init(_:)' requires that 'Unicode.Scalar' conform to 'BinaryInteger'}} | |||
There was a problem hiding this comment.
😢 This change is a significant QoL regression for this diagnostic; the previous error was actually useful to the user, whereas this one is...not. Worse, a missing label is likely to be harder for a human being to discover on their own.
This is coming to my attention, incidentally, because #33889 will also cause this particular message to change; it's going to be brittle because where an initializer is implemented (default implementation versus on the concrete type) now seems to change the diagnostic message.
There was a problem hiding this comment.
I am working on fixing this!
…ific mismatched type (see swiftlang#36267)
…alls
Since labels are considered part of the name, mismatches in labeling
should be invalidate overload choices. Let's prefer an overload with
correct labels but incorrect types over the one with incorrect labels.
This also means that it's possible to restore performance optimizations
related to early filtering in diagnostic mode, which is important for
deeply nested code i.e. SwiftUI views.
Resolves: rdar://72771072