[CS] Solve all conjunctions in source order

Previously we would only do source ordering for
ClosureExprs, but other conjunctions need to have
their source location taken into account too, in
order to make sure we don't try and type-check e.g
a TapExpr in a second closure before we type-check
the first closure.

Also while here, switch to `std::min_element`
instead of sorting, and treat invalid source
locations as incomparable.

rdar://113326835

Author: hamishknight

lib/Sema/CSSolver.cpp

@@ -2438,25 +2438,31 @@ Constraint *ConstraintSystem::selectConjunction() {
 
   auto &SM = getASTContext().SourceMgr;
 
-  // All of the multi-statement closures should be solved in order of their
-  // apperance in the source.
-  llvm::sort(
-      conjunctions, [&](Constraint *conjunctionA, Constraint *conjunctionB) {
+  // Conjunctions should be solved in order of their apperance in the source.
+  // This is important because once a conjunction is solved, we don't re-visit
+  // it, so we need to make sure we don't solve it before another conjuntion
+  // that could provide it with necessary type information. Source order
+  // provides an easy to reason about and quick way of establishing this.
+  return *std::min_element(
+      conjunctions.begin(), conjunctions.end(),
+      [&](Constraint *conjunctionA, Constraint *conjunctionB) {
         auto *locA = conjunctionA->getLocator();
         auto *locB = conjunctionB->getLocator();
-
         if (!(locA && locB))
           return false;
 
-        auto *closureA = getAsExpr<ClosureExpr>(locA->getAnchor());
-        auto *closureB = getAsExpr<ClosureExpr>(locB->getAnchor());
+        auto anchorA = locA->getAnchor();
+        auto anchorB = locB->getAnchor();
+        if (!(anchorA && anchorB))
+          return false;
 
-        return closureA && closureB
-                   ? SM.isBeforeInBuffer(closureA->getLoc(), closureB->getLoc())
-                   : false;
-      });
+        auto slocA = anchorA.getStartLoc();
+        auto slocB = anchorB.getStartLoc();
+        if (!(slocA.isValid() && slocB.isValid()))
+          return false;
 
-  return conjunctions.front();
+        return SM.isBeforeInBuffer(slocA, slocB);
+      });
 }
 
 bool DisjunctionChoice::attempt(ConstraintSystem &cs) const {

test/Constraints/rdar113326835.swift

@@ -0,0 +1,46 @@
+// RUN: %target-typecheck-verify-swift
+
+// rdar://113326835 - Make sure we type-check the conjunctions in source order,
+// the first closure should be type-checked before we attempt the
+// TapExpr/SingleValueExpr conjunctions, since they rely on 'k' being resolved.
+
+func global<T>(_ x: T) -> String { "" }
+func global(_ x: Any.Type) -> String { "" }
+
+protocol P {
+  associatedtype X
+}
+
+struct Q<X>: P {
+  init() {}
+  func bar(_: String) -> Self { fatalError() }
+  func qux<U: P>(_: (X) -> U) -> Q<U.X> { fatalError() }
+}
+
+struct J<X>: P {
+  init(_: X) {}
+  func baz<T>(_ transform: (X) -> T) -> Q<T> { fatalError() }
+}
+
+func foo(a: Int) -> Q<String> {
+  J(a)
+    .baz { x in
+      ()
+      return a
+    }
+    .qux { k in
+      Q<String>().bar("\(k)")
+    }
+}
+
+func bar(a: Int) -> Q<String> {
+  J(a)
+    .baz { x in
+      ()
+      return a
+    }
+    .qux { k in
+      Q<String>().bar(if .random() { global(k) } else { global(k) })
+      // expected-error@-1 {{'if' may only be used as expression in return, throw, or as the source of an assignment}}
+    }
+}