Sema: Split off introduceToInference() into its own Change

Previously, retractFromInference() was the last step in
unbindTypeVariable(). This doesn't really make sense,
because bindTypeVariable() doesn't call introduceToInference();
its two callers do it later.

Start untangling this by splitting off introduceToInference()
into its own Change, but for now, record this change at the
incorrect place to maintain the same behavior as before.

Author: slavapestov

include/swift/Sema/CSTrail.h

@@ -47,6 +47,8 @@ class SolverTrail {
     ExtendedEquivalenceClass,
     /// Added a fixed binding for a type variable in the constraint graph.
     BoundTypeVariable,
+    /// Introduced a type variable's fixed type to inference.
+    IntroducedToInference,
     /// Set the fixed type or parent and flags for a type variable.
     UpdatedTypeVariable,
   };
@@ -110,6 +112,9 @@ class SolverTrail {
     /// Create a change that bound a type variable to a fixed type.
     static Change boundTypeVariable(TypeVariableType *typeVar, Type fixed);
 
+    /// Create a change that introduced a type variable to inference.
+    static Change introducedToInference(TypeVariableType *typeVar, Type fixed);
+
     /// Create a change that updated a type variable.
     static Change updatedTypeVariable(
                TypeVariableType *typeVar,

include/swift/Sema/ConstraintGraph.h

@@ -154,9 +154,7 @@ class ConstraintGraphNode {
   void introduceToInference(Type fixedType);
 
   /// Opposite of \c introduceToInference(Type)
-  void
-  retractFromInference(Type fixedType,
-                       SmallPtrSetImpl<TypeVariableType *> &referencedVars);
+  void retractFromInference(Type fixedType);
 
   /// Drop all previously collected bindings and re-infer based on the
   /// current set constraints associated with this equivalence class.
@@ -273,6 +271,9 @@ class ConstraintGraph {
   /// Bind the given type variable to the given fixed type.
   void bindTypeVariable(TypeVariableType *typeVar, Type fixedType);
 
+  /// Introduce the type variable's fixed type to inference.
+  void introduceToInference(TypeVariableType *typeVar, Type fixedType);
+
   /// Describes which constraints \c gatherConstraints should gather.
   enum class GatheringKind {
     /// Gather constraints associated with all of the variables within the
@@ -429,6 +430,12 @@ class ConstraintGraph {
   /// caution.
   void unbindTypeVariable(TypeVariableType *typeVar, Type fixedType);
 
+  /// Retract the given type variable from inference.
+  ///
+  /// Note that this change is not recorded and cannot be undone. Use with
+  /// caution.
+  void retractFromInference(TypeVariableType *typeVar, Type fixedType);
+
   /// Perform edge contraction on the constraint graph, merging equivalence
   /// classes until a fixed point is reached.
   bool contractEdges();

lib/Sema/CSBindings.cpp

@@ -2746,10 +2746,7 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
   // If all of the re-activated constraints where simplified,
   // let's notify binding inference about the fact that type
   // variable has been bound successfully.
-  {
-    auto &CG = cs.getConstraintGraph();
-    CG[TypeVar].introduceToInference(type);
-  }
+  cs.getConstraintGraph().introduceToInference(TypeVar, type);
 
   return true;
 }

lib/Sema/CSTrail.cpp

@@ -85,6 +85,16 @@ SolverTrail::Change::boundTypeVariable(TypeVariableType *typeVar,
   return result;
 }
 
+SolverTrail::Change
+SolverTrail::Change::introducedToInference(TypeVariableType *typeVar,
+                                           Type fixed) {
+  Change result;
+  result.Kind = ChangeKind::IntroducedToInference;
+  result.Binding.TypeVar = typeVar;
+  result.Binding.FixedType = fixed.getPointer();
+  return result;
+}
+
 SolverTrail::Change
 SolverTrail::Change::updatedTypeVariable(
     TypeVariableType *typeVar,
@@ -124,6 +134,10 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
     cg.unbindTypeVariable(Binding.TypeVar, Binding.FixedType);
     break;
 
+  case ChangeKind::IntroducedToInference:
+    cg.retractFromInference(Binding.TypeVar, Binding.FixedType);
+    break;
+
   case ChangeKind::UpdatedTypeVariable:
     Update.TypeVar->getImpl().setRawOptions(Update.Options);
     Update.TypeVar->getImpl().ParentOrFixed = Update.ParentOrFixed;
@@ -173,6 +187,14 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out,
     out << ")\n";
     break;
 
+  case ChangeKind::IntroducedToInference:
+    out << "(introduced type variable ";
+    Binding.TypeVar->print(out, PO);
+    out << " with fixed type ";
+    Binding.FixedType->print(out, PO);
+    out << " to inference)\n";
+    break;
+
   case ChangeKind::UpdatedTypeVariable:
     out << "(updated type variable ";
     Update.TypeVar->print(out, PO);

lib/Sema/ConstraintGraph.cpp

@@ -92,6 +92,11 @@ ConstraintGraph::lookupNode(TypeVariableType *typeVar) {
   if (typeVar != typeVarRep)
     mergeNodes(typeVar, typeVarRep);
   else if (auto fixed = CS.getFixedType(typeVarRep)) {
+    // FIXME: This is totally the wrong place to do it. We should do this in
+    // introduceToInference().
+    if (CS.isRecordingChanges())
+      CS.recordChange(SolverTrail::Change::introducedToInference(typeVar, fixed));
+
     // Bind the type variable.
     bindTypeVariable(typeVar, fixed);
   }
@@ -414,12 +419,14 @@ void ConstraintGraphNode::introduceToInference(Type fixedType) {
   }
 }
 
-void ConstraintGraphNode::retractFromInference(
-    Type fixedType, SmallPtrSetImpl<TypeVariableType *> &referencedVars) {
+void ConstraintGraphNode::retractFromInference(Type fixedType) {
   // Notify referencing variables (just like in bound case) that this
   // type variable has been modified.
   notifyReferencingVars();
 
+  SmallPtrSet<TypeVariableType *, 4> referencedVars;
+  fixedType->getTypeVariables(referencedVars);
+
   // TODO: This might be an overkill but it's (currently)
   // the simplest way to reliably ensure that all of the
   // no longer related constraints have been retracted.
@@ -559,6 +566,10 @@ void ConstraintGraph::bindTypeVariable(TypeVariableType *typeVar, Type fixed) {
     CS.recordChange(SolverTrail::Change::boundTypeVariable(typeVar, fixed));
 }
 
+void ConstraintGraph::introduceToInference(TypeVariableType *typeVar, Type fixed) {
+  (*this)[typeVar].introduceToInference(fixed);
+}
+
 void ConstraintGraph::unbindTypeVariable(TypeVariableType *typeVar, Type fixed) {
   auto &node = (*this)[typeVar];
 
@@ -571,8 +582,10 @@ void ConstraintGraph::unbindTypeVariable(TypeVariableType *typeVar, Type fixed)
     otherNode.removeReferencedBy(typeVar);
     node.removeReference(otherTypeVar);
   }
+}
 
-  node.retractFromInference(fixed, referencedVars);
+void ConstraintGraph::retractFromInference(TypeVariableType *typeVar, Type fixed) {
+  return (*this)[typeVar].retractFromInference(fixed);
 }
 
 #pragma mark Algorithms

lib/Sema/ConstraintSystem.cpp

@@ -239,12 +239,18 @@ void ConstraintSystem::assignFixedType(TypeVariableType *typeVar, Type type,
     }
   }
 
+  // FIXME: This is totally the wrong place to do it. We should do this in
+  // introduceToInference().
+  if (isRecordingChanges())
+    recordChange(SolverTrail::Change::introducedToInference(typeVar, type));
+
   // Notify the constraint graph.
   CG.bindTypeVariable(typeVar, type);
+
   addTypeVariableConstraintsToWorkList(typeVar);
 
   if (notifyBindingInference)
-    CG[typeVar].introduceToInference(type);
+    CG.introduceToInference(typeVar, type);
 }
 
 void ConstraintSystem::addTypeVariableConstraintsToWorkList(