Sema: Record generated constraints in the trail

Author: slavapestov

include/swift/Sema/CSTrail.def

@@ -30,6 +30,10 @@
 #define CLOSURE_CHANGE(Name) CHANGE(Name)
 #endif
 
+#ifndef CONSTRAINT_CHANGE
+#define CONSTRAINT_CHANGE(Name) CHANGE(Name)
+#endif
+
 #ifndef LAST_CHANGE
 #define LAST_CHANGE(Name)
 #endif
@@ -54,6 +58,10 @@ EXPR_CHANGE(RecordedExprPattern)
 CLOSURE_CHANGE(RecordedClosureType)
 CLOSURE_CHANGE(RecordedPreconcurrencyClosure)
 
+CONSTRAINT_CHANGE(DisabledConstraint)
+CONSTRAINT_CHANGE(FavoredConstraint)
+CONSTRAINT_CHANGE(GeneratedConstraint)
+
 CHANGE(AddedTypeVariable)
 CHANGE(AddedConstraint)
 CHANGE(RemovedConstraint)
@@ -69,8 +77,6 @@ CHANGE(RecordedOpenedPackExpansionType)
 CHANGE(RecordedPackEnvironment)
 CHANGE(RecordedNodeType)
 CHANGE(RecordedKeyPathComponentType)
-CHANGE(DisabledConstraint)
-CHANGE(FavoredConstraint)
 CHANGE(RecordedResultBuilderTransform)
 CHANGE(RecordedContextualInfo)
 CHANGE(RecordedTarget)
@@ -86,5 +92,6 @@ LAST_CHANGE(DecreasedScore)
 #undef LOCATOR_CHANGE
 #undef EXPR_CHANGE
 #undef CLOSURE_CHANGE
+#undef CONSTRAINT_CHANGE
 #undef LAST_CHANGE
 #undef CHANGE

include/swift/Sema/CSTrail.h

@@ -142,6 +142,7 @@ class SolverTrail {
 #define LOCATOR_CHANGE(Name, _) static Change Name(ConstraintLocator *locator);
 #define EXPR_CHANGE(Name) static Change Name(Expr *expr);
 #define CLOSURE_CHANGE(Name) static Change Name(ClosureExpr *closure);
+#define CONSTRAINT_CHANGE(Name) static Change Name(Constraint *constraint);
 #include "swift/Sema/CSTrail.def"
 
     /// Create a change that added a type variable.
@@ -202,12 +203,6 @@ class SolverTrail {
                                                unsigned component,
                                                Type oldType);
 
-    /// Create a change that disabled a constraint.
-    static Change DisabledConstraint(Constraint *constraint);
-
-    /// Create a change that favored a constraint.
-    static Change FavoredConstraint(Constraint *constraint);
-
     /// Create a change that recorded a result builder transform.
     static Change RecordedResultBuilderTransform(AnyFunctionRef fn);
 

include/swift/Sema/ConstraintSystem.h

@@ -2566,8 +2566,7 @@ class ConstraintSystem {
     ///
     /// \param constraint The newly generated constraint.
     void addGeneratedConstraint(Constraint *constraint) {
-      assert(constraint && "Null generated constraint?");
-      generatedConstraints.push_back(constraint);
+      Trail.recordChange(SolverTrail::Change::GeneratedConstraint(constraint));
     }
 
     /// Register given scope to be tracked by the current solver state,
@@ -2582,8 +2581,7 @@ class ConstraintSystem {
 
       CS.incrementScopeCounter();
       auto scopeInfo =
-        std::make_tuple(scope, retiredConstraints.begin(),
-                        generatedConstraints.size());
+        std::make_tuple(scope, retiredConstraints.begin());
       scopes.push_back(scopeInfo);
     }
 
@@ -2604,10 +2602,8 @@ class ConstraintSystem {
       SolverScope *savedScope;
       // The position of last retired constraint before given scope.
       ConstraintList::iterator lastRetiredPos;
-      // The original number of generated constraints before given scope.
-      unsigned numGenerated;
 
-      std::tie(savedScope, lastRetiredPos, numGenerated) =
+      std::tie(savedScope, lastRetiredPos) =
         scopes.pop_back_val();
 
       assert(savedScope == scope && "Scope rollback not in LIFO order!");
@@ -2616,15 +2612,6 @@ class ConstraintSystem {
       CS.InactiveConstraints.splice(CS.InactiveConstraints.end(),
                                     retiredConstraints,
                                     retiredConstraints.begin(), lastRetiredPos);
-
-      // And remove all of the generated constraints.
-      auto genStart = generatedConstraints.begin() + numGenerated,
-           genEnd = generatedConstraints.end();
-      for (auto genI = genStart; genI != genEnd; ++genI) {
-        CS.InactiveConstraints.erase(ConstraintList::iterator(*genI));
-      }
-
-      generatedConstraints.erase(genStart, genEnd);
     }
 
     /// Check whether constraint system is allowed to form solutions
@@ -2660,17 +2647,13 @@ class ConstraintSystem {
     /// creating, it's used to re-activate them on destruction.
     SmallVector<Constraint *, 4> activeConstraints;
 
-    /// The current set of generated constraints.
-    SmallVector<Constraint *, 4> generatedConstraints;
-
     /// The collection which holds association between solver scope
     /// and position of the last retired constraint and number of
     /// constraints generated before registration of given scope,
     /// this helps to rollback all of the constraints retired/generated
     /// each of the registered scopes correct (LIFO) order.
     llvm::SmallVector<
-      std::tuple<SolverScope *, ConstraintList::iterator, unsigned>, 4> scopes;
-
+      std::tuple<SolverScope *, ConstraintList::iterator>, 4> scopes;
 
     /// Depth of the solution stack.
     unsigned depth = 0;

lib/Sema/CSSolver.cpp

@@ -658,9 +658,6 @@ ConstraintSystem::SolverState::~SolverState() {
   // to constraint system.
   assert(!hasRetiredConstraints());
 
-  // Make sure that all of the generated constraints have been handled.
-  assert(generatedConstraints.empty());
-
   // Re-activate constraints which were initially marked as "active"
   // to restore original state of the constraint system.
   for (auto *constraint : activeConstraints) {

lib/Sema/CSTrail.cpp

@@ -72,6 +72,14 @@ SolverTrail::~SolverTrail() {
     result.TheClosure = closure; \
     return result; \
   }
+#define CONSTRAINT_CHANGE(Name) \
+  SolverTrail::Change \
+  SolverTrail::Change::Name(Constraint *constraint) { \
+    Change result; \
+    result.Kind = ChangeKind::Name; \
+    result.TheConstraint.Constraint = constraint; \
+    return result; \
+  }
 #include "swift/Sema/CSTrail.def"
 
 SolverTrail::Change
@@ -221,22 +229,6 @@ SolverTrail::Change::RecordedKeyPathComponentType(const KeyPathExpr *expr,
   return result;
 }
 
-SolverTrail::Change
-SolverTrail::Change::DisabledConstraint(Constraint *constraint) {
-  Change result;
-  result.Kind = ChangeKind::DisabledConstraint;
-  result.TheConstraint.Constraint = constraint;
-  return result;
-}
-
-SolverTrail::Change
-SolverTrail::Change::FavoredConstraint(Constraint *constraint) {
-  Change result;
-  result.Kind = ChangeKind::FavoredConstraint;
-  result.TheConstraint.Constraint = constraint;
-  return result;
-}
-
 SolverTrail::Change
 SolverTrail::Change::RecordedResultBuilderTransform(AnyFunctionRef fn) {
   Change result;
@@ -522,6 +514,12 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
     cs.CurrentScore.Data[kind] += value;
     break;
   }
+
+  case ChangeKind::GeneratedConstraint: {
+    auto iter = ConstraintList::iterator(TheConstraint.Constraint);
+    cs.InactiveConstraints.erase(iter);
+    break;
+  }
   }
 }
 
@@ -553,6 +551,13 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out,
     simple_display(out, TheClosure); \
     out << ")\n"; \
     break;
+#define CONSTRAINT_CHANGE(Name) \
+  case ChangeKind::Name: \
+    out << "(" << #Name << " "; \
+    TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr, \
+                                    indent + 2); \
+    out << ")\n"; \
+    break;
 #include "swift/Sema/CSTrail.def"
 
   case ChangeKind::AddedTypeVariable:
@@ -689,20 +694,6 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out,
     out << " for component " << Options << ")\n";
     break;
 
-  case ChangeKind::DisabledConstraint:
-    out << "(DisabledConstraint ";
-    TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr,
-                                    indent + 2);
-    out << ")\n";
-    break;
-
-  case ChangeKind::FavoredConstraint:
-    out << "(FavoredConstraint ";
-    TheConstraint.Constraint->print(out, &cs.getASTContext().SourceMgr,
-                                    indent + 2);
-    out << ")\n";
-    break;
-
   case ChangeKind::RecordedResultBuilderTransform:
     out << "(RecordedResultBuilderTransform ";
     simple_display(out, TheRef);
@@ -823,4 +814,4 @@ void SolverTrail::dumpActiveScopeChanges(llvm::raw_ostream &out,
 
   out.indent(indent);
   out << ")\n";
-}
+}