[CSSolver] Refactor selectBestBindingDisjunction

Add an assert which verifies that all of the disjunction
choices has the same type variable on the left-hand side.

Based on that fact, refactor `selectBestBindingDisjunction`
to only check first choice to find suitable "bind" disjunctions
to favor.

Author: xedin

lib/Sema/CSSolver.cpp

@@ -1856,73 +1856,49 @@ static Constraint *selectBestBindingDisjunction(
   if (disjunctions.empty())
     return nullptr;
 
-  // Collect any disjunctions that simply attempt bindings for a
-  // type variable.
-  SmallVector<Constraint *, 8> bindingDisjunctions;
+  auto getAsTypeVar = [&cs](Type type) {
+    return cs.simplifyType(type)->getRValueType()->getAs<TypeVariableType>();
+  };
+
+  Constraint *firstBindDisjunction = nullptr;
   for (auto *disjunction : disjunctions) {
-    TypeVariableType *commonTypeVariable = nullptr;
-    if (llvm::all_of(
-            disjunction->getNestedConstraints(),
-            [&](Constraint *bindingConstraint) {
-              if (bindingConstraint->getKind() != ConstraintKind::Bind)
-                return false;
-
-              auto *tv = cs.simplifyType(bindingConstraint->getFirstType())
-                             ->getRValueType()
-                             ->getAs<TypeVariableType>();
-              // Only do this for simple type variable bindings, not for
-              // bindings like: ($T1) -> $T2 bind String -> Int
-              if (!tv)
-                return false;
-
-              // If we've seen a variable before, make sure that this is
-              // the same one.
-              if (commonTypeVariable == tv)
-                return true;
-              if (commonTypeVariable)
-                return false;
-
-              commonTypeVariable = tv;
-              return true;
-            })) {
-      bindingDisjunctions.push_back(disjunction);
-    }
-  }
+    auto choices = disjunction->getNestedConstraints();
+    assert(!choices.empty());
 
-  for (auto *disjunction : bindingDisjunctions) {
-    auto nested = disjunction->getNestedConstraints();
-    assert(!nested.empty());
-    auto *tv = cs.simplifyType(nested[0]->getFirstType())
-                   ->getRValueType()
-                   ->getAs<TypeVariableType>();
-    assert(tv);
+    auto *choice = choices.front();
+    if (choice->getKind() != ConstraintKind::Bind)
+      continue;
+
+    // We can judge disjunction based on the single choice
+    // because all of choices (of bind overload set) should
+    // have the same left-hand side.
+    // Only do this for simple type variable bindings, not for
+    // bindings like: ($T1) -> $T2 bind String -> Int
+    auto *typeVar = getAsTypeVar(choice->getFirstType());
+    if (!typeVar)
+      continue;
+
+    if (!firstBindDisjunction)
+      firstBindDisjunction = disjunction;
 
     llvm::SetVector<Constraint *> constraints;
     cs.getConstraintGraph().gatherConstraints(
-        tv, constraints, ConstraintGraph::GatheringKind::EquivalenceClass,
+        typeVar, constraints, ConstraintGraph::GatheringKind::EquivalenceClass,
         [](Constraint *constraint) {
           return constraint->getKind() == ConstraintKind::Conversion;
         });
 
     for (auto *constraint : constraints) {
-      auto toType =
-          cs.simplifyType(constraint->getSecondType())->getRValueType();
-      auto *toTV = toType->getAs<TypeVariableType>();
-      if (tv != toTV)
-        continue;
-
-      return disjunction;
+      if (typeVar == getAsTypeVar(constraint->getSecondType()))
+        return disjunction;
     }
   }
 
   // If we had any binding disjunctions, return the first of
   // those. These ensure that we attempt to bind types earlier than
   // trying the elements of other disjunctions, which can often mean
   // we fail faster.
-  if (!bindingDisjunctions.empty())
-    return bindingDisjunctions[0];
-
-  return nullptr;
+  return firstBindDisjunction;
 }
 
 Constraint *ConstraintSystem::selectDisjunction() {

lib/Sema/Constraint.cpp

@@ -722,6 +722,25 @@ Constraint *Constraint::createDisjunction(ConstraintSystem &cs,
     return constraints.front();
   }
 
+#ifndef NDEBUG
+  assert(!constraints.empty());
+  // Verify that all disjunction choices have the same left-hand side.
+  Type commonType;
+  assert(llvm::all_of(constraints, [&](const Constraint *choice) -> bool {
+    // if this disjunction is formed from "fixed"
+    // constraints let's not try to validate.
+    if (choice->HasRestriction || choice->getFix())
+      return true;
+
+    auto currentType = choice->getFirstType();
+    if (!commonType) {
+      commonType = currentType;
+      return true;
+    }
+    return commonType->isEqual(currentType);
+  }));
+#endif
+
   // Create the disjunction constraint.
   uniqueTypeVariables(typeVars);
   unsigned size = totalSizeToAlloc<TypeVariableType*>(typeVars.size());