[CS] Factor out BindingSet::favoredOverConjunction

And refactor the way we select the next step for
a ComponentStep. This should be NFC.

Author: hamishknight

include/swift/Sema/CSBindings.h

@@ -480,6 +480,9 @@ class BindingSet {
   /// if it has only concrete types or would resolve a closure.
   bool favoredOverDisjunction(Constraint *disjunction) const;
 
+  /// Check if this binding is favored over a conjunction.
+  bool favoredOverConjunction(Constraint *conjunction) const;
+
   /// Detect `subtype` relationship between two type variables and
   /// attempt to infer supertype bindings transitively e.g.
   ///

lib/Sema/CSBindings.cpp

@@ -1065,6 +1065,14 @@ bool BindingSet::favoredOverDisjunction(Constraint *disjunction) const {
   return !involvesTypeVariables();
 }
 
+bool BindingSet::favoredOverConjunction(Constraint *conjunction) const {
+  if (CS.shouldAttemptFixes() && isHole()) {
+    if (forClosureResult() || forGenericParameter())
+      return false;
+  }
+  return true;
+}
+
 BindingSet ConstraintSystem::getBindingsFor(TypeVariableType *typeVar,
                                             bool finalize) {
   assert(typeVar->getImpl().getRepresentative(nullptr) == typeVar &&

lib/Sema/CSStep.cpp

@@ -359,6 +359,7 @@ StepResult ComponentStep::take(bool prevFailed) {
   });
 
   auto *disjunction = CS.selectDisjunction();
+  auto *conjunction = CS.selectConjunction();
 
   if (CS.isDebugMode()) {
     if (!potentialBindings.empty()) {
@@ -393,33 +394,45 @@ StepResult ComponentStep::take(bool prevFailed) {
     }
   }
 
-  if (CS.shouldAttemptFixes()) {
-    if ((bestBindings &&
-         (bestBindings->forClosureResult() ||
-          bestBindings->forGenericParameter()) &&
-         bestBindings->isHole()) &&
-        !disjunction) {
-      if (auto *conjunction = CS.selectConjunction()) {
-        return suspend(
-            std::make_unique<ConjunctionStep>(CS, conjunction, Solutions));
-      }
+  enum class StepKind { Binding, Disjunction, Conjunction };
+
+  auto chooseStep = [&]() -> Optional<StepKind> {
+    // Bindings usually happen first, but sometimes we want to prioritize a
+    // disjunction or conjunction.
+    if (bestBindings) {
+      if (disjunction && !bestBindings->favoredOverDisjunction(disjunction))
+        return StepKind::Disjunction;
+
+      if (conjunction && !bestBindings->favoredOverConjunction(conjunction))
+        return StepKind::Conjunction;
+
+      return StepKind::Binding;
+    }
+    if (disjunction)
+      return StepKind::Disjunction;
+
+    if (conjunction)
+      return StepKind::Conjunction;
+
+    return None;
+  };
+
+  if (auto step = chooseStep()) {
+    switch (*step) {
+    case StepKind::Binding:
+      return suspend(
+          std::make_unique<TypeVariableStep>(*bestBindings, Solutions));
+    case StepKind::Disjunction:
+      return suspend(
+          std::make_unique<DisjunctionStep>(CS, disjunction, Solutions));
+    case StepKind::Conjunction:
+      return suspend(
+          std::make_unique<ConjunctionStep>(CS, conjunction, Solutions));
     }
+    llvm_unreachable("Unhandled case in switch!");
   }
 
-  if (bestBindings &&
-      (!disjunction || bestBindings->favoredOverDisjunction(disjunction))) {
-    // Produce a type variable step.
-    return suspend(
-        std::make_unique<TypeVariableStep>(*bestBindings, Solutions));
-  } else if (disjunction) {
-    // Produce a disjunction step.
-    return suspend(
-        std::make_unique<DisjunctionStep>(CS, disjunction, Solutions));
-  } else if (auto *conjunction = CS.selectConjunction()) {
-    return suspend(
-        std::make_unique<ConjunctionStep>(CS, conjunction, Solutions));
-  } else if (!CS.solverState->allowsFreeTypeVariables() &&
-             CS.hasFreeTypeVariables()) {
+  if (!CS.solverState->allowsFreeTypeVariables() && CS.hasFreeTypeVariables()) {
     // If there are no disjunctions or type variables to bind
     // we can't solve this system unless we have free type variables
     // allowed in the solution.