[ConstraintSystem] Re-activate constraints if, due to incorrect reference, member type has been bound before base type

If member has been bound before the base and the base was
incorrect at that (e.g. fallback to default `Any` type),
then we need to re-activate all of the constraints
associated with this member reference otherwise some of
the constraints could be left unchecked in inactive state.

This is especially important for key path expressions because
`key path` constraint can't be retired until all components
are simplified.

Resolves: rdar://78035829

Author: xedin

include/swift/Sema/ConstraintLocator.h

@@ -199,9 +199,13 @@ class ConstraintLocator : public llvm::FoldingSetNode {
   /// via key path dynamic member lookup.
   bool isForKeyPathDynamicMemberLookup() const;
 
-  /// Determine whether this locator points to one of the key path
-  /// components.
-  bool isForKeyPathComponent() const;
+  /// Determine whether this locator points to element inside
+  /// of a key path component.
+  bool isInKeyPathComponent() const;
+
+  /// Determine whether this locator points to a result type of
+  /// a key path component.
+  bool isForKeyPathComponentResult() const;
 
   /// Determine whether this locator points to the generic parameter.
   bool isForGenericParameter() const;

lib/Sema/CSBindings.cpp

@@ -1304,6 +1304,13 @@ bool TypeVarBindingProducer::computeNext() {
       }
     }
 
+    // There is a tailored fix for optional key path root references,
+    // let's not create ambiguity by attempting unwrap when it's
+    // not allowed.
+    if (binding.Kind != BindingKind::Subtypes &&
+        getLocator()->isKeyPathRoot() && type->getOptionalObjectType())
+      continue;
+
     // Allow solving for T even for a binding kind where that's invalid
     // if fixes are allowed, because that gives us the opportunity to
     // match T? values to the T binding by adding an unwrap fix.

lib/Sema/CSDiagnostics.cpp

@@ -3930,7 +3930,7 @@ bool AllowTypeOrInstanceMemberFailure::diagnoseAsError() {
     // If this is a reference to a static member by one of the key path
     // components, let's provide a tailored diagnostic and return because
     // that is unsupported so there is no fix-it.
-    if (locator->isForKeyPathComponent()) {
+    if (locator->isInKeyPathComponent()) {
       InvalidStaticMemberRefInKeyPath failure(getSolution(), Member, locator);
       return failure.diagnoseAsError();
     }

lib/Sema/CSDiagnostics.h

@@ -1543,7 +1543,7 @@ class InvalidMemberRefInKeyPath : public FailureDiagnostic {
                             ConstraintLocator *locator)
       : FailureDiagnostic(solution, locator), Member(member) {
     assert(member->hasName());
-    assert(locator->isForKeyPathComponent() ||
+    assert(locator->isInKeyPathComponent() ||
            locator->isForKeyPathDynamicMemberLookup());
   }
 

lib/Sema/CSSimplify.cpp

@@ -7371,8 +7371,21 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
 
       // Record a hole for memberTy to make it possible to form solutions
       // when contextual result type cannot be deduced e.g. `let _ = x.foo`.
-      if (auto *memberTypeVar = memberTy->getAs<TypeVariableType>())
-        recordPotentialHole(memberTypeVar);
+      if (auto *memberTypeVar = memberTy->getAs<TypeVariableType>()) {
+        if (getFixedType(memberTypeVar)) {
+          // If member has been bound before the base and the base was
+          // incorrect at that (e.g. fallback to default `Any` type),
+          // then we need to re-activate all of the constraints
+          // associated with this member reference otherwise some of
+          // the constraints could be left unchecked in inactive state.
+          // This is especially important for key path expressions because
+          // `key path` constraint can't be retired until all components
+          // are simplified.
+          addTypeVariableConstraintsToWorkList(memberTypeVar);
+        } else {
+          recordPotentialHole(memberTypeVar);
+        }
+      }
 
       return SolutionKind::Solved;
     };
@@ -8395,15 +8408,28 @@ ConstraintSystem::simplifyKeyPathConstraint(
       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.
+    // would either be marked as a potential hole or would have a fix.
     if (llvm::any_of(componentTypeVars, [&](TypeVariableType *tv) {
-          return tv->getImpl().getLocator()->isForKeyPathComponent() &&
+          auto *locator = tv->getImpl().getLocator();
+
+          // Result type of a component could be bound to a contextual
+          // (concrete) type if it's the last component in the chain,
+          // so the only way to detect errors is to check for fixes.
+          if (locator->isForKeyPathComponentResult()) {
+            auto path = locator->getPath();
+            auto *componentLoc =
+                getConstraintLocator(locator->getAnchor(), path.drop_back());
+
+            if (hasFixFor(componentLoc, FixKind::DefineMemberBasedOnUse) ||
+                hasFixFor(componentLoc, FixKind::UnwrapOptionalBase) ||
+                hasFixFor(componentLoc,
+                          FixKind::UnwrapOptionalBaseWithOptionalResult))
+              return true;
+          }
+
+          // If something inside of a component is marked as a hole,
+          // let's consider while component to be invalid.
+          return locator->isInKeyPathComponent() &&
                  tv->getImpl().canBindToHole();
         })) {
       return SolutionKind::Solved;

lib/Sema/ConstraintLocator.cpp

@@ -168,12 +168,16 @@ bool ConstraintLocator::isForKeyPathDynamicMemberLookup() const {
   return !path.empty() && path.back().isKeyPathDynamicMember();
 }
 
-bool ConstraintLocator::isForKeyPathComponent() const {
+bool ConstraintLocator::isInKeyPathComponent() const {
   return llvm::any_of(getPath(), [&](const LocatorPathElt &elt) {
     return elt.isKeyPathComponent();
   });
 }
 
+bool ConstraintLocator::isForKeyPathComponentResult() const {
+  return isLastElement<LocatorPathElt::KeyPathComponentResult>();
+}
+
 bool ConstraintLocator::isForGenericParameter() const {
   return isLastElement<LocatorPathElt::GenericParameter>();
 }

test/expr/unary/keypath/keypath.swift

@@ -1052,3 +1052,45 @@ func testSyntaxErrors() {
   _ = \A.a?;
   _ = \A.a!;
 }
+
+// SR-13364 - keypath missing optional crashes compiler: "Inactive constraints left over?"
+func sr13364() {
+  let _: KeyPath<String?, Int?> = \.utf8.count // expected-error {{no exact matches in reference to property 'count'}}
+  // expected-note@-1 {{found candidate with type 'Int'}}
+}
+
+// rdar://74711236 - crash due to incorrect member access in key path
+func rdar74711236() {
+  struct S {
+    var arr: [V] = []
+  }
+
+  struct V : Equatable {
+  }
+
+  enum Type {
+  case store
+  }
+
+  struct Context {
+    func supported() -> [Type] {
+      return []
+    }
+  }
+
+  func test(context: Context?) {
+    var s = S()
+
+    s.arr = {
+      // FIXME: Missing member reference is pattern needs a better diagnostic
+      if let type = context?.store { // expected-error {{type of expression is ambiguous without more context}}
+        // `isSupported` should be an invalid declaration to trigger a crash in `map(\.option)`
+        let isSupported = context!.supported().contains(type) // expected-error {{missing argument label 'where:' in call}} expected-error {{converting non-escaping value to '(Type) throws -> Bool' may allow it to escape}}
+        return (isSupported ? [type] : []).map(\.option)
+        // expected-error@-1 {{value of type 'Any' has no member 'option'}}
+        // expected-note@-2 {{cast 'Any' to 'AnyObject' or use 'as!' to force downcast to a more specific type to access members}}
+      }
+      return []
+    }()
+  }
+}