[CSBindings] Prevent determineBestBindings from selecting unresolved key path type

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() ||