Sema: Remove ConstraintGraphScope

Author: slavapestov

include/swift/Sema/CSTrail.h

@@ -31,7 +31,6 @@ class TypeVariableType;
 namespace constraints {
 
 class Constraint;
-class ConstraintGraphScope;
 
 class SolverTrail {
 public:
@@ -134,6 +133,7 @@ class SolverTrail {
   void recordChange(Change change);
 
   void dumpActiveScopeChanges(llvm::raw_ostream &out,
+                              unsigned fromIndex,
                               unsigned indent = 0);
 
   unsigned size() const {
@@ -148,7 +148,7 @@ class SolverTrail {
   /// The list of changes made to this constraint system.
   std::vector<Change> Changes;
 
-  friend class ConstraintGraphScope;
+  bool UndoActive = false;
 };
 
 } // namespace constraints

include/swift/Sema/ConstraintGraph.h

@@ -42,7 +42,6 @@ namespace constraints {
 
 class Constraint;
 class ConstraintGraph;
-class ConstraintGraphScope;
 class ConstraintSystem;
 class TypeVariableBinding;
 
@@ -440,11 +439,6 @@ class ConstraintGraph {
   /// to contract constraint graph edges.
   void incrementConstraintsPerContractionCounter();
 
-  /// The currently active scope, or null if we aren't tracking changes made
-  /// to the constraint graph.
-  ConstraintGraphScope *ActiveScope = nullptr;
-
-  friend class ConstraintGraphScope;
   friend class SolverTrail;
 };
 

include/swift/Sema/ConstraintGraphScope.h

@@ -35,7 +35,6 @@
 #include "swift/Sema/CSTrail.h"
 #include "swift/Sema/Constraint.h"
 #include "swift/Sema/ConstraintGraph.h"
-#include "swift/Sema/ConstraintGraphScope.h"
 #include "swift/Sema/ConstraintLocator.h"
 #include "swift/Sema/OverloadChoice.h"
 #include "swift/Sema/SolutionResult.h"
@@ -2852,8 +2851,8 @@ class ConstraintSystem {
     /// The length of \c TypeVariables.
     unsigned numTypeVariables;
 
-    /// The length of \c SavedBindings.
-    unsigned numSavedBindings;
+    /// The length of \c Trail.
+    unsigned numTrailChanges;
 
     /// The length of \c ConstraintRestrictions.
     unsigned numConstraintRestrictions;
@@ -2958,9 +2957,6 @@ class ConstraintSystem {
     /// The scope number of this scope. Set when the scope is registered.
     unsigned scopeNumber = 0;
 
-    /// Constraint graph scope associated with this solver scope.
-    ConstraintGraphScope CGScope;
-
     SolverScope(const SolverScope &) = delete;
     SolverScope &operator=(const SolverScope &) = delete;
 

include/swift/Sema/ConstraintSystem.h

@@ -648,8 +648,9 @@ ConstraintSystem::SolverState::~SolverState() {
 }
 
 ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
-  : cs(cs), CGScope(cs.CG)
-{
+  : cs(cs) {
+  numTrailChanges = cs.solverState->Trail.size();
+
   numTypeVariables = cs.TypeVariables.size();
   numConstraintRestrictions = cs.ConstraintRestrictions.size();
   numFixes = cs.Fixes.size();
@@ -711,6 +712,9 @@ ConstraintSystem::SolverScope::~SolverScope() {
                                   cs.ActiveConstraints);
   }
 
+  // Roll back changes to the constraint system.
+  cs.solverState->Trail.undo(numTrailChanges);
+
   // Rollback all of the changes done to constraints by the current scope,
   // e.g. add retired constraints back to the circulation and remove generated
   // constraints introduced by the current scope.

lib/Sema/CSSolver.cpp

@@ -545,13 +545,16 @@ template <typename P> class BindingStep : public SolverStep {
       }
 
       {
+        auto &trail = CS.solverState->Trail;
+        unsigned size = trail.size();
+
         auto scope = std::make_unique<Scope>(CS);
         if (attempt(*choice)) {
           ActiveChoice.emplace(std::move(scope), *choice);
 
           if (CS.isDebugMode()) {
-            CS.solverState->Trail.dumpActiveScopeChanges(
-              llvm::errs(), CS.solverState->getCurrentIndent());
+            trail.dumpActiveScopeChanges(
+              llvm::errs(), size, CS.solverState->getCurrentIndent());
           }
 
           return suspend(std::make_unique<SplitterStep>(CS, Solutions));

lib/Sema/CSStep.h

@@ -18,7 +18,6 @@
 #include "swift/Basic/Defer.h"
 #include "swift/Basic/Statistic.h"
 #include "swift/Sema/ConstraintGraph.h"
-#include "swift/Sema/ConstraintGraphScope.h"
 #include "swift/Sema/ConstraintSystem.h"
 #include "swift/Sema/CSTrail.h"
 #include "swift/Basic/Assertions.h"
@@ -102,11 +101,6 @@ SolverTrail::Change::updatedTypeVariable(
 void SolverTrail::Change::undo(ConstraintSystem &cs) {
   auto &cg = cs.getConstraintGraph();
 
-  /// Temporarily change the active scope to null, so we don't record
-  /// any changes made while performing the undo operation.
-  llvm::SaveAndRestore<ConstraintGraphScope *> prevActiveScope(cg.ActiveScope,
-                                                               nullptr);
-
   switch (Kind) {
   case ChangeKind::AddedTypeVariable:
     cg.removeNode(TypeVar);
@@ -138,33 +132,40 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) {
 }
 
 void SolverTrail::recordChange(Change change) {
+  if (UndoActive)
+    return;
+
   Changes.push_back(change);
 }
 
-void SolverTrail::undo(unsigned upTo) {
+void SolverTrail::undo(unsigned toIndex) {
   // Don't attempt to rollback if constraint system ended up
   // in an invalid state.
   if (CS.inInvalidState())
     return;
 
-  ASSERT(Changes.size() >= upTo && "Trail corrupted");
+  ASSERT(Changes.size() >= toIndex && "Trail corrupted");
+  ASSERT(!UndoActive);
+  UndoActive = true;
 
-  for (unsigned i = Changes.size(); i > upTo; i--) {
+  for (unsigned i = Changes.size(); i > toIndex; i--) {
     auto change = Changes[i - 1];
     if (change.Kind == ChangeKind::UpdatedTypeVariable)
       change.undo(CS);
   }
 
-  for (unsigned i = Changes.size(); i > upTo; i--) {
+  for (unsigned i = Changes.size(); i > toIndex; i--) {
     auto change = Changes[i - 1];
     if (change.Kind != ChangeKind::UpdatedTypeVariable)
       change.undo(CS);
   }
 
-  Changes.resize(upTo);
+  Changes.resize(toIndex);
+  UndoActive = false;
 }
 
 void SolverTrail::dumpActiveScopeChanges(llvm::raw_ostream &out,
+                                         unsigned fromIndex,
                                          unsigned indent) {
   if (Changes.empty())
     return;
@@ -175,8 +176,7 @@ void SolverTrail::dumpActiveScopeChanges(llvm::raw_ostream &out,
   std::vector<TypeVariableType *> equivTypeVars;
   std::set<Constraint *> addedConstraints;
   std::set<Constraint *> removedConstraints;
-  for (unsigned int i = CS.getConstraintGraph().ActiveScope->getStartIdx();
-       i < Changes.size(); i++) {
+  for (unsigned int i = fromIndex; i < Changes.size(); i++) {
     auto change = Changes[i];
     switch (change.Kind) {
     case ChangeKind::BoundTypeVariable:

lib/Sema/CSTrail.cpp

@@ -18,7 +18,6 @@
 #include "swift/Basic/Defer.h"
 #include "swift/Basic/Statistic.h"
 #include "swift/Sema/ConstraintGraph.h"
-#include "swift/Sema/ConstraintGraphScope.h"
 #include "swift/Sema/ConstraintSystem.h"
 #include "swift/Sema/CSTrail.h"
 #include "swift/Basic/Assertions.h"
@@ -69,7 +68,7 @@ ConstraintGraph::lookupNode(TypeVariableType *typeVar) {
   TypeVariables.push_back(typeVar);
 
   // Record the change, if there are active scopes.
-  if (ActiveScope)
+  if (CS.solverState)
     CS.solverState->recordChange(SolverTrail::Change::addedTypeVariable(typeVar));
 
   // If this type variable is not the representative of its equivalence class,
@@ -405,26 +404,6 @@ void ConstraintGraphNode::resetBindingSet() {
   }
 }
 
-#pragma mark Graph scope management
-ConstraintGraphScope::ConstraintGraphScope(ConstraintGraph &CG)
-  : CG(CG), ParentScope(CG.ActiveScope),
-    NumChanges(CG.getConstraintSystem().solverState->Trail.size())
-{
-  CG.ActiveScope = this;
-}
-
-ConstraintGraphScope::~ConstraintGraphScope() {
-  // Don't attempt to rollback if constraint system ended up
-  // in an invalid state.
-  if (CG.CS.inInvalidState())
-    return;
-
-  CG.CS.solverState->Trail.undo(NumChanges);
-
-  // The active scope is now the parent scope.
-  CG.ActiveScope = ParentScope;
-}
-
 #pragma mark Graph mutation
 
 void ConstraintGraph::removeNode(TypeVariableType *typeVar) {
@@ -459,7 +438,7 @@ void ConstraintGraph::addConstraint(Constraint *constraint) {
   }
 
   // Record the change, if there are active scopes.
-  if (ActiveScope)
+  if (CS.solverState)
     CS.solverState->recordChange(SolverTrail::Change::addedConstraint(constraint));
 }
 
@@ -485,7 +464,7 @@ void ConstraintGraph::removeConstraint(Constraint *constraint) {
   }
 
   // Record the change, if there are active scopes.
-  if (ActiveScope)
+  if (CS.solverState)
     CS.solverState->recordChange(SolverTrail::Change::removedConstraint(constraint));
 }
 
@@ -504,7 +483,7 @@ void ConstraintGraph::mergeNodes(TypeVariableType *typeVar1,
   auto typeVarNonRep = typeVar1 == typeVarRep? typeVar2 : typeVar1;
 
   // Record the change, if there are active scopes.
-  if (ActiveScope) {
+  if (CS.solverState) {
     CS.solverState->recordChange(
       SolverTrail::Change::extendedEquivalenceClass(
                         typeVarRep,
@@ -521,7 +500,7 @@ void ConstraintGraph::bindTypeVariable(TypeVariableType *typeVar, Type fixed) {
          "Cannot bind to type variable; merge equivalence classes instead");
 
   // Record the change, if there are active scopes.
-  if (ActiveScope) {
+  if (CS.solverState) {
     CS.solverState->recordChange(
       SolverTrail::Change::boundTypeVariable(typeVar, fixed));
   }