[CSBindings] Apply more checking to transitively inferred bindings

This makes sure that we never run into infinite recursion situations
with transitive bindings that could have been aseembled and not
properly checked.

Author: xedin

include/swift/Sema/CSBindings.h

@@ -388,7 +388,7 @@ class BindingSet {
   BindingSet(const PotentialBindings &info)
       : CS(info.CS), TypeVar(info.TypeVar), Info(info) {
     for (const auto &binding : info.Bindings)
-      addBinding(binding);
+      addBinding(binding, /*isTransitive=*/false);
 
     for (auto *literal : info.Literals)
       addLiteralRequirement(literal);
@@ -479,7 +479,12 @@ class BindingSet {
   }
 
   /// Check if this binding is viable for inclusion in the set.
-  bool isViable(PotentialBinding &binding);
+  ///
+  /// \param binding The binding to validate.
+  /// \param isTransitive Indicates whether this binding has been
+  /// acquired through transitive inference and requires extra
+  /// checking.
+  bool isViable(PotentialBinding &binding, bool isTransitive);
 
   explicit operator bool() const {
     return hasViableBindings() || isDirectHole();
@@ -618,7 +623,13 @@ class BindingSet {
   void dump(llvm::raw_ostream &out, unsigned indent) const;
 
 private:
-  void addBinding(PotentialBinding binding);
+  /// Add a new binding to the set.
+  ///
+  /// \param binding The binding to add.
+  /// \param isTransitive Indicates whether this binding has been
+  /// acquired through transitive inference and requires validity
+  /// checking.
+  void addBinding(PotentialBinding binding, bool isTransitive);
 
   void addLiteralRequirement(Constraint *literal);
 

lib/Sema/CSBindings.cpp

@@ -27,6 +27,9 @@ using namespace swift;
 using namespace constraints;
 using namespace inference;
 
+static llvm::Optional<Type> checkTypeOfBinding(TypeVariableType *typeVar,
+                                               Type type);
+
 bool BindingSet::forClosureResult() const {
   return Info.TypeVar->getImpl().isClosureResultType();
 }
@@ -473,7 +476,7 @@ void BindingSet::inferTransitiveBindings(
 
                 // Copy the bindings over to the root.
                 for (const auto &binding : bindings.Bindings)
-                  addBinding(binding);
+                  addBinding(binding, /*isTransitive=*/true);
 
                 // Make a note that the key path root is transitively adjacent
                 // to contextual root type variable and all of its variables.
@@ -484,7 +487,8 @@ void BindingSet::inferTransitiveBindings(
               }
             } else {
               addBinding(
-                binding.withSameSource(inferredRootTy, BindingKind::Exact));
+                  binding.withSameSource(inferredRootTy, BindingKind::Exact),
+                  /*isTransitive=*/true);
             }
           }
         }
@@ -553,7 +557,8 @@ void BindingSet::inferTransitiveBindings(
       if (ConstraintSystem::typeVarOccursInType(TypeVar, type))
         continue;
 
-      addBinding(binding.withSameSource(type, BindingKind::Supertypes));
+      addBinding(binding.withSameSource(type, BindingKind::Supertypes),
+                 /*isTransitive=*/true);
     }
   }
 }
@@ -631,7 +636,8 @@ void BindingSet::finalize(
                   continue;
             }
 
-            addBinding({protocolTy, AllowedBindingKind::Exact, constraint});
+            addBinding({protocolTy, AllowedBindingKind::Exact, constraint},
+                       /*isTransitive=*/false);
           }
         }
       }
@@ -740,11 +746,11 @@ void BindingSet::finalize(
   }
 }
 
-void BindingSet::addBinding(PotentialBinding binding) {
+void BindingSet::addBinding(PotentialBinding binding, bool isTransitive) {
   if (Bindings.count(binding))
     return;
 
-  if (!isViable(binding))
+  if (!isViable(binding, isTransitive))
     return;
 
   SmallPtrSet<TypeVariableType *, 4> referencedTypeVars;
@@ -1165,14 +1171,17 @@ void PotentialBindings::addLiteral(Constraint *constraint) {
   Literals.insert(constraint);
 }
 
-bool BindingSet::isViable(PotentialBinding &binding) {
+bool BindingSet::isViable(PotentialBinding &binding, bool isTransitive) {
   // Prevent against checking against the same opened nominal type
   // over and over again. Doing so means redundant work in the best
   // case. In the worst case, we'll produce lots of duplicate solutions
   // for this constraint system, which is problematic for overload
   // resolution.
   auto type = binding.BindingType;
 
+  if (isTransitive && !checkTypeOfBinding(TypeVar, type))
+    return false;
+
   auto *NTD = type->getAnyNominal();
   if (!NTD)
     return true;

test/Sema/keypath_bidirectional_inference.swift

@@ -82,3 +82,29 @@ func testCollectionUpcastWithTupleLabelErasure() {
         .sorted(by: \.key.rawValue) // Ok
   }
 }
+
+do {
+  struct URL {
+    var path: String
+    func appendingPathComponent(_: String) -> URL { fatalError() }
+  }
+
+  struct EntryPoint {
+    var directory: URL { fatalError() }
+  }
+
+  func test(entryPoint: EntryPoint, data: [[String]]) {
+    let _ = data.map { suffixes in
+      let elements = ["a", "b"]
+      .flatMap { dir in
+        let directory = entryPoint.directory.appendingPathComponent(dir)
+        return suffixes.map { suffix in
+          directory.appendingPathComponent("\(suffix)")
+        }
+      }
+      .map(\.path) // Ok
+
+      return elements.joined(separator: ",")
+    }
+  }
+}