[CS] Don't leave key path with holes unsolved

We currently leave a key path constraint unsolved
if one of its components hasn't yet had its
overload resolved. However, for e.g a missing
member component, the overload type variable will
be bound to a hole and an overload will never be
resolved.

Tweak the logic to consider the key path constraint
trivially solved if one of its components has been
marked as a hole, which will allow the key path
type itself to be marked as a hole.

Resolves SR-12437 & SR-12823.
Resolves rdar://62201037.

Author: hamishknight

lib/Sema/CSSimplify.cpp

@@ -7697,10 +7697,30 @@ ConstraintSystem::simplifyKeyPathConstraint(
     return true;
   };
 
-  // We have a hole, the solver can't infer the key path type. So let's
-  // just assume this is solved.
-  if (shouldAttemptFixes() && keyPathTy->isHole()) {
-    return SolutionKind::Solved;
+  // If we have a hole somewhere in the key path, the solver won't be able to
+  // infer the key path type. So let's just assume this is solved.
+  if (shouldAttemptFixes()) {
+    if (keyPathTy->isHole())
+      return SolutionKind::Solved;
+
+    // If the root type has been bound to a hole, we cannot infer it.
+    if (getFixedTypeRecursive(rootTy, /*wantRValue*/ true)->isHole())
+      return SolutionKind::Solved;
+
+    // If we have e.g a missing member somewhere, a component type variable
+    // will have been marked as a potential hole.
+    // FIXME: This relies on the fact that we only mark an overload type
+    // variable as a potential hole once we've added a corresponding fix. We
+    // can't use 'isHole' instead, as that doesn't handle cases where the
+    // overload type variable gets bound to another type from the context rather
+    // than a hole. We need to come up with a better way of handling the
+    // relationship between key paths and overloads.
+    if (llvm::any_of(componentTypeVars, [&](TypeVariableType *tv) {
+          return tv->getImpl().getLocator()->isForKeyPathComponent() &&
+                 tv->getImpl().canBindToHole();
+        })) {
+      return SolutionKind::Solved;
+    }
   }
 
   // If we're fixed to a bound generic type, trying harvesting context from it.
@@ -7751,34 +7771,11 @@ ConstraintSystem::simplifyKeyPathConstraint(
       // to determine whether the result will be a function type vs BGT KeyPath
       // type, so continue through components to create new constraint at the
       // end.
-      if (!overload || anyComponentsUnresolved) {
+      if (!overload) {
         if (flags.contains(TMF_GenerateConstraints)) {
           anyComponentsUnresolved = true;
           continue;
         }
-
-        if (shouldAttemptFixes()) {
-          auto typeVar =
-              llvm::find_if(componentTypeVars, [&](TypeVariableType *typeVar) {
-                auto *locator = typeVar->getImpl().getLocator();
-                auto elt = locator->findLast<LocatorPathElt::KeyPathComponent>();
-                return elt && elt->getIndex() == i;
-              });
-
-          // If one of the components haven't been resolved, let's check
-          // whether it has been determined to be a "hole" and if so,
-          // let's allow component validation to contiunue.
-          //
-          // This helps to, for example, diagnose problems with missing
-          // members used as part of a key path.
-          if (typeVar != componentTypeVars.end() &&
-              (*typeVar)->getImpl().canBindToHole()) {
-            anyComponentsUnresolved = true;
-            capability = ReadOnly;
-            continue;
-          }
-        }
-
         return SolutionKind::Unsolved;
       }
 

test/Constraints/rdar62201037.swift

@@ -0,0 +1,31 @@
+// RUN: %target-swift-frontend %s -verify -emit-sil -o /dev/null
+
+struct R<T> {
+  var str: String?
+}
+
+func map<A, B>(e: (A) -> B) -> () -> R<B> {
+  fatalError()
+}
+func map<A, B>(_ : (A) -> B) -> (A?) -> B? {
+  fatalError()
+}
+
+infix operator |>
+func |> <A, B> (g: A, h: (A) -> B) -> B { h(g) }
+
+infix operator ^^^
+func ^^^ <A, B, C>(j: ((B) -> C) -> A, k: String) {}
+
+extension WritableKeyPath {
+  static func ^^^ (l: WritableKeyPath, m: Value) -> (Root) -> Root {
+    fatalError()
+  }
+}
+
+func foo<T>(_ s: String, _ rt: R<T>?) -> String? {
+  return rt.flatMap { _ in
+    rt |> map(\.str ^^^ s)
+  }
+  .flatMap(\.str)
+}

test/expr/unary/keypath/keypath.swift

@@ -895,11 +895,40 @@ struct SR_12290 {
 
 func testKeyPathHole() {
   _ = \.x // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{8-8=<#Root#>}}
+  _ = \.x.y // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{8-8=<#Root#>}}
+
   let _ : AnyKeyPath = \.x 
   // expected-error@-1 {{'AnyKeyPath' does not provide enough context for root type to be inferred; consider explicitly specifying a root type}} {{25-25=<#Root#>}}
+  let _ : AnyKeyPath = \.x.y
+  // expected-error@-1 {{'AnyKeyPath' does not provide enough context for root type to be inferred; consider explicitly specifying a root type}} {{25-25=<#Root#>}}
 
   func f(_ i: Int) {}
   f(\.x) // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{6-6=<#Root#>}}
+  f(\.x.y) // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{6-6=<#Root#>}}
+
+  // FIXME(SR-12827): Instead of "generic parameter 'T' could not be inferred",
+  // we should offer the same diagnostic as above.
+  func provideValueButNotRoot<T>(_ fn: (T) -> String) {} // expected-note 2{{in call to function 'provideValueButNotRoot'}}
+  provideValueButNotRoot(\.x) // expected-error {{generic parameter 'T' could not be inferred}}
+  provideValueButNotRoot(\.x.y) // expected-error {{generic parameter 'T' could not be inferred}}
+  provideValueButNotRoot(\String.foo) // expected-error {{value of type 'String' has no member 'foo'}}
+
+  func provideKPValueButNotRoot<T>(_ kp: KeyPath<T, String>) {} // expected-note 3{{in call to function 'provideKPValueButNotRoot'}}
+  provideKPValueButNotRoot(\.x) // expected-error {{generic parameter 'T' could not be inferred}}
+  provideKPValueButNotRoot(\.x.y) // expected-error {{generic parameter 'T' could not be inferred}}
+  provideKPValueButNotRoot(\String.foo)
+  // expected-error@-1 {{value of type 'String' has no member 'foo'}}
+  // expected-error@-2 {{generic parameter 'T' could not be inferred}}
+}
+
+func testMissingMember() {
+  let _: KeyPath<String, String> = \.foo // expected-error {{value of type 'String' has no member 'foo'}}
+  let _: KeyPath<String, String> = \.foo.bar // expected-error {{value of type 'String' has no member 'foo'}}
+
+  let _: PartialKeyPath<String> = \.foo // expected-error {{value of type 'String' has no member 'foo'}}
+  let _: PartialKeyPath<String> = \.foo.bar // expected-error {{value of type 'String' has no member 'foo'}}
+
+  _ = \String.x.y // expected-error {{value of type 'String' has no member 'x'}}
 }
 
 func testSyntaxErrors() { // expected-note{{}}