[ConstraintSystem] NFC: Standardize the way of type holefication

Introduce `ConstraintSystem::recordTypeVariablesAsHoles` as a
standard way to record that unbound type variables found in a
type are holes in the given constraint system.

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -3337,6 +3337,13 @@ class ConstraintSystem {
   void recordPotentialHole(TypeVariableType *typeVar);
   void recordAnyTypeVarAsPotentialHole(Type type);
 
+  /// Record all unbound type variables that occur in the given type
+  /// as being bound to "hole" type represented by \c PlaceholderType
+  /// in this constraint system.
+  ///
+  /// \param type The type on which to holeify.
+  void recordTypeVariablesAsHoles(Type type);
+
   void recordMatchCallArgumentResult(ConstraintLocator *locator,
                                      MatchCallArgumentResult result);
 

lib/Sema/BuilderTransform.cpp

@@ -1174,13 +1174,7 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType,
 
       if (auto *closure =
               getAsExpr<ClosureExpr>(fn.getAbstractClosureExpr())) {
-        auto closureTy = getClosureType(closure);
-        simplifyType(closureTy).visit([&](Type componentTy) {
-          if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
-            assignFixedType(typeVar,
-                            PlaceholderType::get(getASTContext(), typeVar));
-          }
-        });
+        recordTypeVariablesAsHoles(getClosureType(closure));
       }
 
       return getTypeMatchSuccess();

lib/Sema/CSSimplify.cpp

@@ -6159,14 +6159,7 @@ bool ConstraintSystem::repairFailures(
     // unrelated fixes, let's proactively bind all of the pattern
     // elemnts to holes here.
     if (lhs->isAny()) {
-      rhs.visit([&](Type type) {
-        if (auto *typeVar = type->getAs<TypeVariableType>()) {
-          if (!getFixedType(typeVar)) {
-            assignFixedType(typeVar,
-                            PlaceholderType::get(getASTContext(), typeVar));
-          }
-        }
-      });
+      recordTypeVariablesAsHoles(rhs);
     }
 
     conversionsOrFixes.push_back(CollectionElementContextualMismatch::create(
@@ -10404,8 +10397,7 @@ ConstraintSystem::SolutionKind ConstraintSystem::simplifyMemberConstraint(
             // equality instead of argument application constraint, so allowing
             // them to bind member could mean missing valid hole positions in
             // the pattern.
-            assignFixedType(memberTypeVar, PlaceholderType::get(getASTContext(),
-                                                                memberTypeVar));
+            recordTypeVariablesAsHoles(memberTypeVar);
           } else {
             recordPotentialHole(memberTypeVar);
           }
@@ -14010,8 +14002,25 @@ void ConstraintSystem::recordAnyTypeVarAsPotentialHole(Type type) {
     return;
 
   type.visit([&](Type type) {
-    if (auto *typeVar = type->getAs<TypeVariableType>())
+    if (auto *typeVar = type->getAs<TypeVariableType>()) {
       typeVar->getImpl().enableCanBindToHole(getSavedBindings());
+    }
+  });
+}
+
+void ConstraintSystem::recordTypeVariablesAsHoles(Type type) {
+  // Let's simplify the type first because bound type variables could
+  // reference other type variables in their bindings.
+  simplifyType(type).visit([&](Type componentTy) {
+    if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
+      // Ignore bound type variables. This can happen if a type variable
+      // occurs in multiple positions.
+      if (typeVar->getImpl().hasRepresentativeOrFixed())
+        return;
+
+      assignFixedType(typeVar,
+                      PlaceholderType::get(getASTContext(), typeVar));
+    }
   });
 }
 

lib/Sema/CSStep.cpp

@@ -1103,13 +1103,6 @@ void ConjunctionStep::SolverSnapshot::applySolution(const Solution &solution) {
   if (score.Data[SK_Fix] == 0)
     return;
 
-  auto holeify = [&](Type componentTy) {
-    if (auto *typeVar = componentTy->getAs<TypeVariableType>()) {
-      CS.assignFixedType(
-          typeVar, PlaceholderType::get(CS.getASTContext(), typeVar));
-    }
-  };
-
   // If this conjunction represents a closure and inference
   // has failed, let's bind all of unresolved type variables
   // in its interface type to holes to avoid extraneous
@@ -1118,14 +1111,13 @@ void ConjunctionStep::SolverSnapshot::applySolution(const Solution &solution) {
   if (locator->directlyAt<ClosureExpr>()) {
     auto closureTy =
         CS.getClosureType(castToExpr<ClosureExpr>(locator->getAnchor()));
-
-    CS.simplifyType(closureTy).visit(holeify);
+    CS.recordTypeVariablesAsHoles(closureTy);
   }
 
   // Same for a SingleValueStmtExpr, turn any unresolved type variables present
   // in its type into holes.
   if (locator->isForSingleValueStmtConjunction()) {
     auto *SVE = castToExpr<SingleValueStmtExpr>(locator->getAnchor());
-    CS.simplifyType(CS.getType(SVE)).visit(holeify);
+    CS.recordTypeVariablesAsHoles(CS.getType(SVE));
   }
 }