Merge pull request #70662 from xedin/fix-crash-in-simplifyKeyPathConstraint

[ConstraintSystem] Teach inference to never select unresolved key path types

Author: xedin

include/swift/Sema/CSBindings.h

@@ -576,7 +576,10 @@ class BindingSet {
 
   /// Finalize binding computation for this type variable by
   /// inferring bindings from context e.g. transitive bindings.
-  void finalize(
+  ///
+  /// \returns true if finalization successful (which makes binding set viable),
+  /// and false otherwise.
+  bool finalize(
       llvm::SmallDenseMap<TypeVariableType *, BindingSet> &inferredBindings);
 
   static BindingScore formBindingScore(const BindingSet &b);

lib/Sema/CSBindings.cpp

@@ -600,7 +600,7 @@ static Type getKeyPathType(ASTContext &ctx, KeyPathCapability capability,
   return keyPathTy;
 }
 
-void BindingSet::finalize(
+bool BindingSet::finalize(
     llvm::SmallDenseMap<TypeVariableType *, BindingSet> &inferredBindings) {
   inferTransitiveBindings(inferredBindings);
 
@@ -654,9 +654,10 @@ void BindingSet::finalize(
 
       std::tie(isValid, capability) = CS.inferKeyPathLiteralCapability(TypeVar);
 
-      // Key path literal is not yet sufficiently resolved.
+      // Key path literal is not yet sufficiently resolved, this binding
+      // set is not viable.
       if (isValid && !capability)
-        return;
+        return false;
 
       // If the key path is sufficiently resolved we can add inferred binding
       // to the set.
@@ -728,7 +729,7 @@ void BindingSet::finalize(
       Bindings = std::move(updatedBindings);
       Defaults.clear();
 
-      return;
+      return true;
     }
 
     if (CS.shouldAttemptFixes() &&
@@ -752,6 +753,8 @@ void BindingSet::finalize(
         });
       }
     }
+
+    return true;
   }
 }
 
@@ -1030,7 +1033,8 @@ llvm::Optional<BindingSet> ConstraintSystem::determineBestBindings(
     // produce a default type.
     bool isViable = isViableForRanking(bindings);
 
-    bindings.finalize(cache);
+    if (!bindings.finalize(cache))
+      continue;
 
     if (!bindings || !isViable)
       continue;

lib/Sema/CSSimplify.cpp

@@ -12207,13 +12207,6 @@ ConstraintSystem::simplifyKeyPathConstraint(
       assert(contextualTy);
     }
 
-    // If there are no other options the solver might end up picking
-    // `AnyKeyPath` or `PartialKeyPath` based on a contextual conversion.
-    // This is an error during normal type-checking but okay in
-    // diagnostic mode because root and value are allowed to be holes.
-    if (contextualTy->isAnyKeyPath() || contextualTy->isPartialKeyPath())
-      return shouldAttemptFixes();
-
     if (auto bgt = contextualTy->getAs<BoundGenericType>()) {
       // We can get root and value from a concrete key path type.
       assert(bgt->isKeyPath() || bgt->isWritableKeyPath() ||

lib/Sema/ConstraintSystem.cpp

@@ -7648,6 +7648,13 @@ ConstraintSystem::inferKeyPathLiteralCapability(KeyPathExpr *keyPath) {
   if (keyPath->hasSingleInvalidComponent())
     return fail();
 
+  // If root is determined to be a hole it means that none of the components
+  // are resolvable and key path is not viable.
+  auto rootTy =
+      getFixedTypeRecursive(getKeyPathRootType(keyPath), /*wantRValue=*/false);
+  if (rootTy->isPlaceholder())
+    return fail();
+
   auto mutability = KeyPathMutability::Writable;
   for (unsigned i : indices(keyPath->getComponents())) {
     auto &component = keyPath->getComponents()[i];

test/expr/unary/keypath/keypath.swift

@@ -914,13 +914,13 @@ func testKeyPathHole() {
 
   func f(_ i: Int) {}
   f(\.x) // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{6-6=<#Root#>}}
-  // expected-error@-1 {{cannot convert value of key path type to expected argument type 'Int'}}
   f(\.x.y) // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}} {{6-6=<#Root#>}}
-  // expected-error@-1 {{cannot convert value of key path type to expected argument type 'Int'}}
 
-  func provideValueButNotRoot<T>(_ fn: (T) -> String) {} 
+func provideValueButNotRoot<T>(_ fn: (T) -> String) {} // expected-note 2 {{in call to function 'provideValueButNotRoot'}}
   provideValueButNotRoot(\.x) // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}}
+  // expected-error@-1 {{generic parameter 'T' could not be inferred}}
   provideValueButNotRoot(\.x.y) // expected-error {{cannot infer key path type from context; consider explicitly specifying a root type}}
+// expected-error@-1 {{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>) {} 
@@ -1237,3 +1237,9 @@ func test_keypath_coercion_to_function() {
 func test_keypath_application_with_composition(v: String, kp: any KeyPath<String, Int> & PP) {
   _ = v[keyPath: kp] // Ok
 }
+
+func test_leading_dot_key_path_without_context() {
+  func test(_: AnyKeyPath?) {}
+  test(\.utf8)
+  // expected-error@-1 {{cannot infer key path type from context; consider explicitly specifying a root type}}
+}

validation-test/Sema/SwiftUI/rdar84580119.swift

@@ -16,7 +16,6 @@ extension EnvironmentValues {
     set { self[\.MyHorizontalAlignmentEnvironmentKey.self] = newValue }
     // expected-error@-1 {{generic parameter 'K' could not be inferred}}
     // expected-error@-2 {{cannot infer key path type from context; consider explicitly specifying a root type}}
-    // expected-error@-3 {{cannot convert value of key path type to expected argument type 'K.Type'}}
   }
 }