[CSStep] Conjunction: integrate isolation scope into snapshot

It helps to simply handling of outer constrants because they have
to be added to the constraint system before scope is created but
constraint graph have to get updated after to make sure that
incremental binding inference already knows about types inferred
from conjunction.

Author: xedin

Diff for: lib/Sema/CSStep.cpp

@@ -821,9 +821,7 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
   // Return from the follow-up splitter step that
   // attempted to apply information gained from the
   // isolated constraint to the outer context.
-  if (bool(IsolationScope)) {
-    IsolationScope.reset();
-
+  if (Snapshot && Snapshot->isScoped()) {
     if (CS.isDebugMode())
       getDebugLogger() << ")\n";
 
@@ -870,18 +868,6 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
   // solving along the current path until complete
   // solution is reached.
   if (Producer.isExhausted()) {
-    // Restore constraint system state before conjunction.
-    //
-    // Note that this doesn't include conjunction constraint
-    // itself because we don't want to re-solve it at this
-    // point.
-    if (Conjunction->isIsolated()) {
-      assert(
-          Snapshot &&
-          "Isolated conjunction requires a snapshot of the constraint system");
-      Snapshot.reset();
-    }
-
     // If one of the elements failed, that means while
     // conjunction failed with it.
     if (HadFailure)
@@ -901,12 +887,15 @@ StepResult ConjunctionStep::resume(bool prevFailed) {
         log << "(applying conjunction result to outer context\n";
       }
 
-      // Establish isolation scope so that conjunction solution
-      // and follow-up steps could be rolled back.
-      IsolationScope = std::make_unique<Scope>(CS);
-
-      // Apply solution inferred for the conjunction.
-      CS.applySolution(Solutions.pop_back_val());
+      // Restore constraint system state before conjunction.
+      //
+      // Note that this doesn't include conjunction constraint
+      // itself because we don't want to re-solve it at this
+      // point.
+      assert(
+          Snapshot &&
+          "Isolated conjunction requires a snapshot of the constraint system");
+      Snapshot->setupOuterContext(Solutions.pop_back_val());
 
       // Restore best score, since upcoming step is going to
       // work with outer scope in relation to the conjunction.

Diff for: lib/Sema/CSStep.h

@@ -775,6 +775,12 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
     llvm::SetVector<TypeVariableType *> TypeVars;
     ConstraintList Constraints;
 
+    /// If this conjunction has to be solved in isolation,
+    /// this scope would be initialized once all of the
+    /// elements are successfully solved to continue solving
+    /// along the current path as-if there was no conjunction.
+    std::unique_ptr<Scope> IsolationScope = nullptr;
+
   public:
     SolverSnapshot(ConstraintSystem &cs)
         : CS(cs), TypeVars(std::move(cs.TypeVariables)) {
@@ -786,12 +792,47 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
         CG.removeConstraint(&constraint);
     }
 
+    void setupOuterContext(Solution solution) {
+      // Re-add type variables and constraints back
+      // to the constraint system.
+      restore();
+
+      // Establish isolation scope so that conjunction solution
+      // and follow-up steps could be rolled back.
+      IsolationScope = std::make_unique<Scope>(CS);
+
+      // Apply solution inferred for the conjunction.
+      CS.applySolution(solution);
+
+      // Add constraints to the graph after solution
+      // has been applied to make sure that all type
+      // information is available to incremental inference.
+      for (auto &constraint : CS.InactiveConstraints)
+        CS.CG.addConstraint(&constraint);
+    }
+
+    bool isScoped() const { return bool(IsolationScope); }
+
     ~SolverSnapshot() {
-      auto &CG = CS.getConstraintGraph();
+      if (!IsolationScope)
+        restore();
+
+      IsolationScope.reset();
+      // Re-add all of the constraint to the constraint
+      // graph after scope has been rolled back, to make
+      // make sure the original (before conjunction)
+      // state is completely restored.
+      updateConstraintGraph();
+    }
 
+  private:
+    void restore() {
       CS.TypeVariables = std::move(TypeVars);
       CS.InactiveConstraints.splice(CS.InactiveConstraints.end(), Constraints);
+    }
 
+    void updateConstraintGraph() {
+      auto &CG = CS.getConstraintGraph();
       for (auto &constraint : CS.InactiveConstraints)
         CG.addConstraint(&constraint);
     }
@@ -812,12 +853,6 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
   /// Indicates that one of the elements failed inference.
   bool HadFailure = false;
 
-  /// If this conjunction has to be solved in isolation,
-  /// this scope would be initialized once all of the
-  /// elements are successfully solved to continue solving
-  /// along the current path as-if there was no conjunction.
-  std::unique_ptr<Scope> IsolationScope = nullptr;
-
   /// If conjunction has to be solved in isolation, this
   /// variable would capture the snapshot of the constraint
   /// system step before conjunction step.
@@ -838,7 +873,6 @@ class ConjunctionStep : public BindingStep<ConjunctionElementProducer> {
 
   ~ConjunctionStep() override {
     assert(!bool(ActiveChoice));
-    assert(!bool(IsolationScope));
 
     // Return all of the type variables and constraints back.
     Snapshot.reset();