[ConstraintSystem] Key path capability inference should indicate whether key path is invalid

This flag makes it easier to determine what binding to produce
from the default. In cases where some of the member references
are invalid it's better to produce a placeholder for a key
path type instead of letting the solver to attempt to fix more
contextual problems for a broken key path.

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -5524,9 +5524,9 @@ class ConstraintSystem {
   ///
   /// \param keyPathType The type variable that represents the key path literal.
   ///
-  /// \returns Capability if key path is sufficiently resolved and None
-  /// otherwise.
-  llvm::Optional<KeyPathCapability>
+  /// \returns `bool` to indicate whether key path is valid or not,
+  /// and capability if it could be determined.
+  std::pair</*isValid=*/bool, llvm::Optional<KeyPathCapability>>
   inferKeyPathLiteralCapability(TypeVariableType *keyPathType);
 
   SWIFT_DEBUG_DUMP;

lib/Sema/CSBindings.cpp

@@ -889,14 +889,32 @@ void BindingSet::addDefault(Constraint *constraint) {
 
   if (TypeVar->getImpl().isKeyPathType() && Bindings.empty()) {
     if (constraint->getKind() == ConstraintKind::FallbackType) {
-      if (auto capability = CS.inferKeyPathLiteralCapability(TypeVar)) {
+      auto &ctx = CS.getASTContext();
+
+      bool isValid;
+      llvm::Optional<KeyPathCapability> capability;
+
+      std::tie(isValid, capability) = CS.inferKeyPathLiteralCapability(TypeVar);
+
+      if (!isValid) {
+        // If one of the references in a key path is invalid let's add
+        // a placeholder binding in diagnostic mode to indicate that
+        // the key path cannot be properly resolved.
+        if (CS.shouldAttemptFixes()) {
+          addBinding({PlaceholderType::get(ctx, TypeVar),
+                      AllowedBindingKind::Exact, constraint});
+        }
+
+        // During normal solving the set has to stay empty.
+        return;
+      }
+
+      if (capability) {
         auto *keyPathType = defaultTy->castTo<BoundGenericType>();
 
         auto root = keyPathType->getGenericArgs()[0];
         auto value = keyPathType->getGenericArgs()[1];
 
-        auto &ctx = CS.getASTContext();
-
         switch (*capability) {
         case KeyPathCapability::ReadOnly:
           break;
@@ -914,8 +932,11 @@ void BindingSet::addDefault(Constraint *constraint) {
         }
 
         addBinding({keyPathType, AllowedBindingKind::Exact, constraint});
-        return;
       }
+
+      // If key path is not yet sufficiently resolved, don't add any
+      // bindings.
+      return;
     }
   }
 

lib/Sema/CSSimplify.cpp

@@ -11736,13 +11736,6 @@ bool ConstraintSystem::resolveKeyPath(TypeVariableType *typeVar,
         contextualType = BoundGenericType::get(
             keyPathTy->getDecl(), keyPathTy->getParent(), {root, value});
       }
-    } else if (contextualType->isPlaceholder()) {
-      auto root = simplifyType(getKeyPathRootType(keyPath));
-      if (!(root->isTypeVariableOrMember() || root->isPlaceholder())) {
-        auto value = getKeyPathValueType(keyPath);
-        contextualType =
-            BoundGenericType::get(ctx.getKeyPathDecl(), Type(), {root, value});
-      }
     }
   }
 

lib/Sema/ConstraintSystem.cpp

@@ -7379,7 +7379,7 @@ ConstraintSystem::lookupConformance(Type type, ProtocolDecl *protocol) {
   return conformance;
 }
 
-llvm::Optional<KeyPathCapability>
+std::pair<bool, llvm::Optional<KeyPathCapability>>
 ConstraintSystem::inferKeyPathLiteralCapability(TypeVariableType *keyPathType) {
   auto *typeLocator = keyPathType->getImpl().getLocator();
   assert(typeLocator->isLastElement<LocatorPathElt::KeyPathType>());
@@ -7390,6 +7390,19 @@ ConstraintSystem::inferKeyPathLiteralCapability(TypeVariableType *keyPathType) {
 
   bool didOptionalChain = false;
 
+  auto fail = []() -> std::pair<bool, llvm::Optional<KeyPathCapability>> {
+    return std::make_pair(false, llvm::None);
+  };
+
+  auto delay = []() -> std::pair<bool, llvm::Optional<KeyPathCapability>> {
+    return std::make_pair(true, llvm::None);
+  };
+
+  auto success = [](KeyPathCapability capability)
+      -> std::pair<bool, llvm::Optional<KeyPathCapability>> {
+    return std::make_pair(true, capability);
+  };
+
   for (unsigned i : indices(keyPath->getComponents())) {
     auto &component = keyPath->getComponents()[i];
 
@@ -7411,7 +7424,7 @@ ConstraintSystem::inferKeyPathLiteralCapability(TypeVariableType *keyPathType) {
       auto *calleeLoc = getCalleeLocator(componentLoc);
       auto overload = findSelectedOverloadFor(calleeLoc);
       if (!overload)
-        return llvm::None;
+        return delay();
 
       // tuple elements do not change the capability of the key path
       auto choice = overload->choice;
@@ -7421,23 +7434,16 @@ ConstraintSystem::inferKeyPathLiteralCapability(TypeVariableType *keyPathType) {
 
       // Discarded unsupported non-decl member lookups.
       if (!choice.isDecl())
-        return llvm::None;
+        return fail();
 
       auto storage = dyn_cast<AbstractStorageDecl>(choice.getDecl());
 
-      if (hasFixFor(calleeLoc, FixKind::AllowInvalidRefInKeyPath)) {
-        if (!shouldAttemptFixes())
-          return llvm::None;
-
-        // If this was a method reference let's mark it as read-only.
-        if (!storage) {
-          capability = KeyPathCapability::ReadOnly;
-          continue;
-        }
-      }
+      if (hasFixFor(calleeLoc, FixKind::AllowInvalidRefInKeyPath) ||
+          hasFixFor(calleeLoc, FixKind::UnwrapOptionalBase))
+        return fail();
 
       if (!storage)
-        return llvm::None;
+        return fail();
 
       if (isReadOnlyKeyPathComponent(storage, component.getLoc())) {
         capability = KeyPathCapability::ReadOnly;
@@ -7481,7 +7487,7 @@ ConstraintSystem::inferKeyPathLiteralCapability(TypeVariableType *keyPathType) {
   if (didOptionalChain)
     capability = KeyPathCapability::ReadOnly;
 
-  return capability;
+  return success(capability);
 }
 
 TypeVarBindingProducer::TypeVarBindingProducer(BindingSet &bindings)