Sema: Record implicit value conversions in the trail

Author: slavapestov

include/swift/Sema/CSTrail.def

@@ -19,7 +19,7 @@
 #endif
 
 #ifndef LOCATOR_CHANGE
-#define LOCATOR_CHANGE(Name) CHANGE(Name)
+#define LOCATOR_CHANGE(Name, Map) CHANGE(Name)
 #endif
 
 #ifndef EXPR_CHANGE
@@ -34,13 +34,15 @@
 #define LAST_CHANGE(Name)
 #endif
 
-LOCATOR_CHANGE(RecordedAppliedDisjunction)
-LOCATOR_CHANGE(RecordedMatchCallArgumentResult)
-LOCATOR_CHANGE(RecordedOpenedTypes)
-LOCATOR_CHANGE(RecordedOpenedExistentialType)
-LOCATOR_CHANGE(RecordedPackExpansionEnvironment)
-LOCATOR_CHANGE(RecordedDefaultedConstraint)
-LOCATOR_CHANGE(ResolvedOverload)
+LOCATOR_CHANGE(RecordedDisjunctionChoice, DisjunctionChoices)
+LOCATOR_CHANGE(RecordedAppliedDisjunction, AppliedDisjunctions)
+LOCATOR_CHANGE(RecordedMatchCallArgumentResult, argumentMatchingChoices)
+LOCATOR_CHANGE(RecordedOpenedTypes, OpenedTypes)
+LOCATOR_CHANGE(RecordedOpenedExistentialType, OpenedExistentialTypes)
+LOCATOR_CHANGE(RecordedPackExpansionEnvironment, PackExpansionEnvironments)
+LOCATOR_CHANGE(RecordedDefaultedConstraint, DefaultedConstraints)
+LOCATOR_CHANGE(ResolvedOverload, ResolvedOverloads)
+LOCATOR_CHANGE(RecordedImplicitValueConversion, ImplicitValueConversions)
 
 EXPR_CHANGE(AppliedPropertyWrapper)
 EXPR_CHANGE(RecordedImpliedResult)
@@ -60,7 +62,6 @@ CHANGE(UpdatedTypeVariable)
 CHANGE(AddedConversionRestriction)
 CHANGE(AddedFix)
 CHANGE(AddedFixedRequirement)
-CHANGE(RecordedDisjunctionChoice)
 CHANGE(RecordedOpenedPackExpansionType)
 CHANGE(RecordedPackEnvironment)
 CHANGE(RecordedNodeType)

include/swift/Sema/CSTrail.h

@@ -139,7 +139,7 @@ class SolverTrail {
 
     Change() : Kind(ChangeKind::AddedTypeVariable), TypeVar(nullptr) { }
 
-#define LOCATOR_CHANGE(Name) static Change Name(ConstraintLocator *locator);
+#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);
 #include "swift/Sema/CSTrail.def"
@@ -187,10 +187,6 @@ class SolverTrail {
                                         unsigned reqKind,
                                         Type requirementTy);
 
-    /// Create a change that recorded a disjunction choice.
-    static Change RecordedDisjunctionChoice(ConstraintLocator *locator,
-                                            unsigned index);
-
     /// Create a change that recorded the opening of a pack expansion type.
     static Change RecordedOpenedPackExpansionType(PackExpansionType *expansion);
 

include/swift/Sema/ConstraintSystem.h

@@ -2331,7 +2331,7 @@ class ConstraintSystem {
 
   /// The set of implicit value conversions performed by the solver on
   /// a current path to reach a solution.
-  llvm::SmallMapVector<ConstraintLocator *, ConversionRestrictionKind, 2>
+  llvm::SmallDenseMap<ConstraintLocator *, ConversionRestrictionKind, 2>
       ImplicitValueConversions;
 
   /// The worklist of "active" constraints that should be revisited
@@ -2415,11 +2415,6 @@ class ConstraintSystem {
     }
   }
 
-  void removeDefaultedConstraint(ConstraintLocator *locator) {
-    bool erased = DefaultedConstraints.erase(locator);
-    ASSERT(erased);
-  }
-
   /// A cache that stores the @dynamicCallable required methods implemented by
   /// types.
   llvm::DenseMap<NominalTypeDecl *, DynamicCallableMethods>
@@ -2855,9 +2850,6 @@ class ConstraintSystem {
     /// FIXME: Remove this.
     unsigned numFixes;
 
-    /// The length of \c ImplicitValueConversions.
-    unsigned numImplicitValueConversions;
-
     /// The length of \c KeyPaths.
     unsigned numKeyPaths;
 
@@ -3453,12 +3445,6 @@ class ConstraintSystem {
   void recordOpenedExistentialType(ConstraintLocator *locator,
                                    OpenedArchetypeType *opened);
 
-  /// Undo the above change.
-  void removeOpenedExistentialType(ConstraintLocator *locator) {
-    bool erased = OpenedExistentialTypes.erase(locator);
-    ASSERT(erased);
-  }
-
   /// Get the opened element generic environment for the given locator.
   GenericEnvironment *getPackElementEnvironment(ConstraintLocator *locator,
                                                 CanType shapeClass);
@@ -3467,12 +3453,6 @@ class ConstraintSystem {
   void recordPackExpansionEnvironment(ConstraintLocator *locator,
                                       std::pair<UUID, Type> uuidAndShape);
 
-  /// Undo the above change.
-  void removePackExpansionEnvironment(ConstraintLocator *locator) {
-    bool erased = PackExpansionEnvironments.erase(locator);
-    ASSERT(erased);
-  }
-
   /// Get the opened element generic environment for the given pack element.
   PackExpansionExpr *getPackEnvironment(PackElementExpr *packElement) const;
 
@@ -3655,12 +3635,6 @@ class ConstraintSystem {
   void recordMatchCallArgumentResult(ConstraintLocator *locator,
                                      MatchCallArgumentResult result);
 
-  /// Undo the above change.
-  void removeMatchCallArgumentResult(ConstraintLocator *locator) {
-    bool erased = argumentMatchingChoices.erase(locator);
-    ASSERT(erased);
-  }
-
   /// Record implicitly generated `callAsFunction` with root at the
   /// given expression, located at \c locator.
   void recordCallAsFunction(UnresolvedDotExpr *root, ArgumentList *arguments,
@@ -4438,9 +4412,6 @@ class ConstraintSystem {
   void recordOpenedType(
       ConstraintLocator *locator, ArrayRef<OpenedType> openedTypes);
 
-  /// Undo the above change.
-  void removeOpenedType(ConstraintLocator *locator);
-
   /// Record the set of opened types for the given locator.
   void recordOpenedTypes(
          ConstraintLocatorBuilder locator,
@@ -4962,8 +4933,6 @@ class ConstraintSystem {
   void recordResolvedOverload(ConstraintLocator *locator,
                               SelectedOverload choice);
 
-  void removeResolvedOverload(ConstraintLocator *locator);
-
   /// Resolve the given overload set to the given choice.
   void resolveOverload(ConstraintLocator *locator, Type boundType,
                        OverloadChoice choice, DeclContext *useDC);
@@ -5253,6 +5222,10 @@ class ConstraintSystem {
                  TypeMatchOptions flags,
                  ConstraintLocatorBuilder locator);
 
+  /// Update ImplicitValueConversions and record a change in the trail.
+  void recordImplicitValueConversion(ConstraintLocator *locator,
+                                     ConversionRestrictionKind restriction);
+
   /// Simplify a conversion constraint by applying the given
   /// reduction rule, which is known to apply at the outermost level.
   SolutionKind simplifyRestrictedConstraint(
@@ -5393,22 +5366,10 @@ class ConstraintSystem {
   /// Record a particular disjunction choice and add a change to the trail.
   void recordDisjunctionChoice(ConstraintLocator *locator, unsigned index);
 
-  /// Undo the above change.
-  void removeDisjunctionChoice(ConstraintLocator *locator) {
-    bool erased = DisjunctionChoices.erase(locator);
-    ASSERT(erased);
-  }
-
   /// Record applied disjunction and add a change to the trail.
   void recordAppliedDisjunction(ConstraintLocator *locator,
                                 FunctionType *type);
 
-  /// Undo the above change.
-  void removeAppliedDisjunction(ConstraintLocator *locator) {
-    bool erased = AppliedDisjunctions.erase(locator);
-    ASSERT(erased);
-  }
-
   /// Filter the set of disjunction terms, keeping only those where the
   /// predicate returns \c true.
   ///

lib/Sema/CSSimplify.cpp

@@ -14082,6 +14082,17 @@ void ConstraintSystem::addRestrictedConstraint(
                                      TMF_GenerateConstraints, locator);
 }
 
+void ConstraintSystem::recordImplicitValueConversion(
+                             ConstraintLocator *locator,
+                             ConversionRestrictionKind restriction) {
+  bool inserted = ImplicitValueConversions.insert(
+        {getConstraintLocator(locator), restriction}).second;
+  ASSERT(inserted);
+
+  if (solverState)
+    recordChange(SolverTrail::Change::RecordedImplicitValueConversion(locator));
+}
+
 /// Given that we have a conversion constraint between two types, and
 /// that the given constraint-reduction rule applies between them at
 /// the top level, apply it and generate any necessary recursive

lib/Sema/CSSolver.cpp

@@ -437,7 +437,10 @@ void ConstraintSystem::applySolution(const Solution &solution) {
   }
 
   for (auto &valueConversion : solution.ImplicitValueConversions) {
-    ImplicitValueConversions.insert(valueConversion);
+    if (ImplicitValueConversions.count(valueConversion.first) == 0) {
+      recordImplicitValueConversion(valueConversion.first,
+                                    valueConversion.second);
+    }
   }
 
   // Register the argument lists.
@@ -699,7 +702,6 @@ ConstraintSystem::SolverScope::SolverScope(ConstraintSystem &cs)
   numTypeVariables = cs.TypeVariables.size();
   numFixes = cs.Fixes.size();
   numKeyPaths = cs.KeyPaths.size();
-  numImplicitValueConversions = cs.ImplicitValueConversions.size();
   numArgumentLists = cs.ArgumentLists.size();
   numImplicitCallAsFunctionRoots = cs.ImplicitCallAsFunctionRoots.size();
   numSynthesizedConformances = cs.SynthesizedConformances.size();
@@ -738,9 +740,6 @@ ConstraintSystem::SolverScope::~SolverScope() {
   /// Remove any key path expressions.
   truncate(cs.KeyPaths, numKeyPaths);
 
-  // Remove any implicit value conversions.
-  truncate(cs.ImplicitValueConversions, numImplicitValueConversions);
-
   // Remove any argument lists no longer in scope.
   truncate(cs.ArgumentLists, numArgumentLists);
 

lib/Sema/CSTrail.cpp

@@ -48,7 +48,7 @@ SolverTrail::~SolverTrail() {
     ASSERT(Changes.empty() && "Trail corrupted");
 }
 
-#define LOCATOR_CHANGE(Name) \
+#define LOCATOR_CHANGE(Name, _) \
   SolverTrail::Change \
   SolverTrail::Change::Name(ConstraintLocator *locator) { \
     Change result; \
@@ -184,16 +184,6 @@ SolverTrail::Change::AddedFixedRequirement(GenericTypeParamType *GP,
   return result;
 }
 
-SolverTrail::Change
-SolverTrail::Change::RecordedDisjunctionChoice(ConstraintLocator *locator,
-                                               unsigned index) {
-  Change result;
-  result.Kind = ChangeKind::RecordedDisjunctionChoice;
-  result.TheLocator = locator;
-  result.Options = index;
-  return result;
-}
-
 SolverTrail::Change
 SolverTrail::Change::RecordedOpenedPackExpansionType(PackExpansionType *expansionTy) {
   Change result;
@@ -360,6 +350,14 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
   auto &cg = cs.getConstraintGraph();
 
   switch (Kind) {
+#define LOCATOR_CHANGE(Name, Map) \
+  case ChangeKind::Name: { \
+    bool erased = cs.Map.erase(TheLocator); \
+    ASSERT(erased); \
+    break; \
+  }
+#include "swift/Sema/CSTrail.def"
+
   case ChangeKind::AddedTypeVariable:
     cg.removeNode(TypeVar);
     break;
@@ -409,42 +407,14 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
                               FixedRequirement.ReqTy);
     break;
 
-  case ChangeKind::RecordedDisjunctionChoice:
-    cs.removeDisjunctionChoice(TheLocator);
-    break;
-
-  case ChangeKind::RecordedAppliedDisjunction:
-    cs.removeAppliedDisjunction(TheLocator);
-    break;
-
-  case ChangeKind::RecordedMatchCallArgumentResult:
-    cs.removeMatchCallArgumentResult(TheLocator);
-    break;
-
-  case ChangeKind::RecordedOpenedTypes:
-    cs.removeOpenedType(TheLocator);
-    break;
-
-  case ChangeKind::RecordedOpenedExistentialType:
-    cs.removeOpenedExistentialType(TheLocator);
-    break;
-
   case ChangeKind::RecordedOpenedPackExpansionType:
     cs.removeOpenedPackExpansionType(TheExpansion);
     break;
 
-  case ChangeKind::RecordedPackExpansionEnvironment:
-    cs.removePackExpansionEnvironment(TheLocator);
-    break;
-
   case ChangeKind::RecordedPackEnvironment:
     cs.removePackEnvironment(TheElement);
     break;
 
-  case ChangeKind::RecordedDefaultedConstraint:
-    cs.removeDefaultedConstraint(TheLocator);
-    break;
-
   case ChangeKind::RecordedNodeType:
     cs.restoreType(Node.Node, Node.OldType);
     break;
@@ -470,10 +440,6 @@ void SolverTrail::Change::undo(ConstraintSystem &cs) const {
     cs.removePropertyWrapper(TheExpr);
     break;
 
-  case ChangeKind::ResolvedOverload:
-    cs.removeResolvedOverload(TheLocator);
-    break;
-
   case ChangeKind::RecordedClosureType:
     cs.removeClosureType(TheClosure);
     break;
@@ -522,7 +488,7 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out,
 
   switch (Kind) {
 
-#define LOCATOR_CHANGE(Name) \
+#define LOCATOR_CHANGE(Name, _) \
   case ChangeKind::Name: \
     out << "(" << #Name << " at "; \
     TheLocator->dump(&cs.getASTContext().SourceMgr, out); \
@@ -641,13 +607,6 @@ void SolverTrail::Change::dump(llvm::raw_ostream &out,
     out << ")\n";
     break;
 
-  case ChangeKind::RecordedDisjunctionChoice:
-    out << "(RecordedDisjunctionChoice at ";
-    TheLocator->dump(&cs.getASTContext().SourceMgr, out);
-    out << " index ";
-    out << Options << ")\n";
-    break;
-
   case ChangeKind::RecordedOpenedPackExpansionType:
     out << "(RecordedOpenedPackExpansionType for ";
     TheExpansion->print(out, PO);

lib/Sema/ConstraintSystem.cpp

@@ -298,7 +298,7 @@ void ConstraintSystem::recordDisjunctionChoice(
   ASSERT(inserted);
 
   if (solverState)
-    recordChange(SolverTrail::Change::RecordedDisjunctionChoice(locator, index));
+    recordChange(SolverTrail::Change::RecordedDisjunctionChoice(locator));
 }
 
 void ConstraintSystem::recordAppliedDisjunction(
@@ -1691,11 +1691,6 @@ void ConstraintSystem::recordOpenedType(
     recordChange(SolverTrail::Change::RecordedOpenedTypes(locator));
 }
 
-void ConstraintSystem::removeOpenedType(ConstraintLocator *locator) {
-  bool erased = OpenedTypes.erase(locator);
-  ASSERT(erased);
-}
-
 void ConstraintSystem::recordOpenedTypes(
        ConstraintLocatorBuilder locator,
        const OpenedTypeMap &replacements,
@@ -3754,11 +3749,6 @@ void ConstraintSystem::recordResolvedOverload(ConstraintLocator *locator,
     recordChange(SolverTrail::Change::ResolvedOverload(locator));
 }
 
-void ConstraintSystem::removeResolvedOverload(ConstraintLocator *locator) {
-  bool erased = ResolvedOverloads.erase(locator);
-  ASSERT(erased);
-}
-
 void ConstraintSystem::resolveOverload(ConstraintLocator *locator,
                                        Type boundType,
                                        OverloadChoice choice,