[Type-checker] Improve diagnostic for keypath used without 'keyPath:' label

[artemcm]

Upon finding a keypath subscript without a prerequisite keyPath: label, continue type-checking as usual. If the keyPath type gets verified, it will eventually be caught with a "missing argument" error, which will suggest a fix-it of adding the missing label.

Resolves SR-12745
Resolves rdar://problem/62957095

[artemcm]

Oi! Just noticed that this issue is also being tackled in: #31671

[xedin]

At a glance I think this is actually a solution we want :) /cc @LucianoPAlmeida

[LucianoPAlmeida]

Humm... I think that is the way you suggested changing the implementation right @xedin ?

[varungandhi-apple]

What's going to happen when there is another subscript (with a different label) which also takes the same type of key path as an argument?

[xedin]

That's going to be an ambiguity failure but we should probably add a test-case regardless.

[artemcm]

See example in keypath_subscript_nolabel_ambiguous.swift, surprisingly, this does not result in an ambiguity error. It produces the exact same missing argument label 'keypath:' in subscript error. At the use site, trying both keyPath: and kp: works; but, without an explicit label, the compiler seems unaware of the ambiguity.

[artemcm]

I have a suspicion that this might be un-related to the fix though.

[xedin]

@artemcm I think example you have committed produces correct diagnostic, but as @varungandhi-apple mentioned it would be interesting if declared subscript would actually have an argument of (correct) key path type:

struct S {
    var x : Int = 0
    subscript(v v: KeyPath<S, Int>) -> Int { get { 0 } set(newValue) {} }
}
var s = S()
s[\.x] = 10

Correct key path type but missing label, should be diagnosed as - missing argument label 'v:' in subscript

struct S {
    var x : Int = 0
    subscript(v: KeyPath<S, String>) -> Int { get { 0 } set(newValue) {} }
}
var s = S()
s[\.x] = 10

No label but incorrect key path type, should be diagnosed as key path value type 'Int' cannot be converted to contextual type 'String'

I think if there is a subscript which has a key path parameter we should always prefer that over keyPath: one so there is probably only one way to make this ambiguous:

struct S {
    var x : Int = 0
    subscript(v: KeyPath<S, Double>) -> Int { get { 0 } set(newValue) {} }
    subscript(v: KeyPath<S, String>) -> Int { get { 0 } set(newValue) {} }
}
var s = S()
s[\.x] = 10

[artemcm]

Thanks @xedin, I was writing up similar examples and I am seeing that for the first code snippet:

struct S {
    var x : Int = 0
    subscript(v v: KeyPath<S, Int>) -> Int { get { 0 } set(newValue) {} }
}
var s = S()
s[\.x] = 10

This PR changes the produced diagnostic from:
missing argument label 'v:' in subscript
to
missing argument label 'keyPath:' in subscript
Which, as you suggest, isn't right, so I am trying to figure out why that may be changing here.

(This means that the corresponding test test/Sema/keypath_subscript_nolabel_overload.swift in this PR is currently failing.)

[artemcm]

Copying comment from the TODO inserted in the test here, as a summary of this problem:

This should actually be a diagnostic that correctly identifies that in the case of a missing label, there are two options for resolution: 'keyPath:' and 'v:' and to offer the user a choice. Today, the ExprTypeChecker identifies the disjunction with two of these possibilities, but filters out some of the terms based on label mismatch (but not implicit keypath terms, for example). It should probably not do that.

[artemcm]

@swift-ci test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 97e32ab

[shahmishal]

@swift-ci Please test