Merge pull request swiftlang#39859 from xedin/dedupe-constraint-system-storage

[ConstraintSystem] De-duplicate storage of constraint system elements

Author: xedin

include/swift/Sema/ConstraintSystem.h

@@ -2184,10 +2184,6 @@ class SolutionApplicationTarget {
 using RewriteTargetFn = std::function<
     Optional<SolutionApplicationTarget> (SolutionApplicationTarget)>;
 
-/// Represents a conversion restriction between two types.
-using ConversionRestriction =
-    std::tuple<TypeBase *, TypeBase *, ConversionRestrictionKind>;
-
 enum class ConstraintSystemPhase {
   ConstraintGeneration,
   Solving,
@@ -2377,15 +2373,15 @@ class ConstraintSystem {
   /// there are multiple ways in which one type could convert to another, e.g.,
   /// given class types A and B, the solver might choose either a superclass
   /// conversion or a user-defined conversion.
-  std::vector<ConversionRestriction> ConstraintRestrictions;
+  llvm::MapVector<std::pair<TypeBase *, TypeBase *>, ConversionRestrictionKind>
+      ConstraintRestrictions;
 
   /// The set of fixes applied to make the solution work.
-  llvm::SmallVector<ConstraintFix *, 4> Fixes;
+  llvm::SmallSetVector<ConstraintFix *, 4> Fixes;
 
   /// The set of remembered disjunction choices used to reach
   /// the current constraint system.
-  std::vector<std::pair<ConstraintLocator*, unsigned>>
-      DisjunctionChoices;
+  llvm::MapVector<ConstraintLocator *, unsigned> DisjunctionChoices;
 
   /// A map from applied disjunction constraints to the corresponding
   /// argument function type.
@@ -2394,12 +2390,12 @@ class ConstraintSystem {
 
   /// For locators associated with call expressions, the trailing closure
   /// matching rule and parameter bindings that were applied.
-  std::vector<std::pair<ConstraintLocator *, MatchCallArgumentResult>>
+  llvm::MapVector<ConstraintLocator *, MatchCallArgumentResult>
       argumentMatchingChoices;
 
   /// The set of implicit value conversions performed by the solver on
   /// a current path to reach a solution.
-  SmallVector<std::pair<ConstraintLocator *, ConversionRestrictionKind>, 2>
+  llvm::SmallMapVector<ConstraintLocator *, ConversionRestrictionKind, 2>
       ImplicitValueConversions;
 
   /// The worklist of "active" constraints that should be revisited
@@ -2415,8 +2411,8 @@ class ConstraintSystem {
 
   /// A mapping from constraint locators to the set of opened types associated
   /// with that locator.
-  SmallVector<std::pair<ConstraintLocator *, ArrayRef<OpenedType>>, 4>
-    OpenedTypes;
+  llvm::SmallMapVector<ConstraintLocator *, ArrayRef<OpenedType>, 4>
+      OpenedTypes;
 
   /// The list of all generic requirements fixed along the current
   /// solver path.
@@ -2430,11 +2426,11 @@ class ConstraintSystem {
 
   /// A mapping from constraint locators to the opened existential archetype
   /// used for the 'self' of an existential type.
-  SmallVector<std::pair<ConstraintLocator *, OpenedArchetypeType *>, 4>
-    OpenedExistentialTypes;
+  llvm::SmallMapVector<ConstraintLocator *, OpenedArchetypeType *, 4>
+      OpenedExistentialTypes;
 
   /// The set of functions that have been transformed by a result builder.
-  std::vector<std::pair<AnyFunctionRef, AppliedBuilderTransform>>
+  llvm::MapVector<AnyFunctionRef, AppliedBuilderTransform>
       resultBuilderTransformed;
 
   /// Cache of the effects any closures visited.
@@ -2450,7 +2446,7 @@ class ConstraintSystem {
   llvm::SmallMapVector<ASTNode, SmallVector<AppliedPropertyWrapper, 2>, 4> appliedPropertyWrappers;
 
   /// The locators of \c Defaultable constraints whose defaults were used.
-  std::vector<ConstraintLocator *> DefaultedConstraints;
+  llvm::SetVector<ConstraintLocator *> DefaultedConstraints;
 
   /// A cache that stores the @dynamicCallable required methods implemented by
   /// types.
@@ -3424,7 +3420,7 @@ class ConstraintSystem {
     return !solverState || solverState->recordFixes;
   }
 
-  ArrayRef<ConstraintFix *> getFixes() const { return Fixes; }
+  ArrayRef<ConstraintFix *> getFixes() const { return Fixes.getArrayRef(); }
 
   bool shouldSuppressDiagnostics() const {
     return Options.contains(ConstraintSystemFlags::SuppressDiagnostics);
@@ -3473,6 +3469,16 @@ class ConstraintSystem {
         });
   }
 
+  bool
+  hasConversionRestriction(Type type1, Type type2,
+                           ConversionRestrictionKind restrictionKind) const {
+    auto restriction =
+        ConstraintRestrictions.find({type1.getPointer(), type2.getPointer()});
+    return restriction == ConstraintRestrictions.end()
+               ? false
+               : restriction->second == restrictionKind;
+  }
+
   /// If an UnresolvedDotExpr, SubscriptMember, etc has been resolved by the
   /// constraint system, return the decl that it references.
   ValueDecl *findResolvedMemberRef(ConstraintLocator *locator);
@@ -4172,10 +4178,12 @@ class ConstraintSystem {
                           OpenedTypeMap *replacements = nullptr);
 
   /// Retrieve a list of generic parameter types solver has "opened" (replaced
-  /// with a type variable) along the current path.
-  ArrayRef<std::pair<ConstraintLocator *, ArrayRef<OpenedType>>>
-  getOpenedTypes() const {
-    return OpenedTypes;
+  /// with a type variable) at the given location.
+  ArrayRef<OpenedType> getOpenedTypes(ConstraintLocator *locator) const {
+    auto substitutions = OpenedTypes.find(locator);
+    if (substitutions == OpenedTypes.end())
+      return {};
+    return substitutions->second;
   }
 
 private:
@@ -4901,7 +4909,10 @@ class ConstraintSystem {
   /// Record a particular disjunction choice of
   void recordDisjunctionChoice(ConstraintLocator *disjunctionLocator,
                                unsigned index) {
-    DisjunctionChoices.push_back({disjunctionLocator, index});
+    // We shouldn't ever register disjunction choices multiple times.
+    assert(!DisjunctionChoices.count(disjunctionLocator) ||
+           DisjunctionChoices[disjunctionLocator] == index);
+    DisjunctionChoices.insert({disjunctionLocator, index});
   }
 
   /// Filter the set of disjunction terms, keeping only those where the

lib/Sema/BuilderTransform.cpp

@@ -1859,8 +1859,7 @@ ConstraintSystem::matchResultBuilder(AnyFunctionRef fn, Type builderType,
         return elt.first == fn;
       }) == resultBuilderTransformed.end() &&
          "already transformed this body along this path!?!");
-  resultBuilderTransformed.push_back(
-      std::make_pair(fn, std::move(*applied)));
+  resultBuilderTransformed.insert(std::make_pair(fn, std::move(*applied)));
 
   // If builder is applied to the closure expression then
   // `closure body` to `closure result` matching should

lib/Sema/CSBindings.cpp

@@ -1983,7 +1983,7 @@ bool TypeVariableBinding::attempt(ConstraintSystem &cs) const {
 
   // If this was from a defaultable binding note that.
   if (Binding.isDefaultableBinding()) {
-    cs.DefaultedConstraints.push_back(srcLocator);
+    cs.DefaultedConstraints.insert(srcLocator);
 
     if (type->isPlaceholder() && reportHole())
       return true;

lib/Sema/CSSimplify.cpp

@@ -3973,10 +3973,10 @@ bool ConstraintSystem::repairFailures(
       // we can instead suggest the conditional downcast as it is safer in
       // situations like conditional binding.
       auto useConditionalCast =
-          llvm::any_of(ConstraintRestrictions, [&](auto &restriction) {
-            ConversionRestrictionKind restrictionKind;
+          llvm::any_of(ConstraintRestrictions, [&](const auto &restriction) {
             Type type1, type2;
-            std::tie(type1, type2, restrictionKind) = restriction;
+            std::tie(type1, type2) = restriction.first;
+            auto restrictionKind = restriction.second;
 
             if (restrictionKind != ConversionRestrictionKind::ValueToOptional)
               return false;
@@ -6770,7 +6770,6 @@ static ConstraintFix *maybeWarnAboutExtraneousCast(
     ConstraintSystem &cs, Type origFromType, Type origToType, Type fromType,
     Type toType, SmallVector<Type, 4> fromOptionals,
     SmallVector<Type, 4> toOptionals,
-    const std::vector<ConversionRestriction> &constraintRestrictions,
     ConstraintSystem::TypeMatchOptions flags,
     ConstraintLocatorBuilder locator) {
 
@@ -6797,10 +6796,8 @@ static ConstraintFix *maybeWarnAboutExtraneousCast(
   // "from" expression could be a type variable with value-to-optional
   // restrictions that we have to account for optionality mismatch.
   const auto subExprType = cs.getType(castExpr->getSubExpr());
-  if (llvm::is_contained(
-          constraintRestrictions,
-          std::make_tuple(fromType.getPointer(), subExprType.getPointer(),
-                          ConversionRestrictionKind::ValueToOptional))) {
+  if (cs.hasConversionRestriction(fromType, subExprType,
+                                  ConversionRestrictionKind::ValueToOptional)) {
     extraOptionals++;
     origFromType = OptionalType::get(origFromType);
   }
@@ -6960,7 +6957,7 @@ ConstraintSystem::simplifyCheckedCastConstraint(
 
     if (auto *fix = maybeWarnAboutExtraneousCast(
             *this, origFromType, origToType, fromType, toType, fromOptionals,
-            toOptionals, ConstraintRestrictions, flags, locator)) {
+            toOptionals, flags, locator)) {
       (void)recordFix(fix);
     }
   };
@@ -7028,7 +7025,7 @@ ConstraintSystem::simplifyCheckedCastConstraint(
     // succeed or fail.
     if (auto *fix = maybeWarnAboutExtraneousCast(
             *this, origFromType, origToType, fromType, toType, fromOptionals,
-            toOptionals, ConstraintRestrictions, flags, locator)) {
+            toOptionals, flags, locator)) {
       (void)recordFix(fix);
     }
 
@@ -8893,13 +8890,10 @@ static Type getOpenedResultBuilderTypeFor(ConstraintSystem &cs,
   if (builderType->hasTypeParameter()) {
     // Find the opened type for this callee and substitute in the type
     // parametes.
-    // FIXME: We should consider changing OpenedTypes to a MapVector.
-    for (const auto &opened : cs.getOpenedTypes()) {
-      if (opened.first == calleeLocator) {
-        OpenedTypeMap replacements(opened.second.begin(), opened.second.end());
-        builderType = cs.openType(builderType, replacements);
-        break;
-      }
+    auto substitutions = cs.getOpenedTypes(calleeLocator);
+    if (!substitutions.empty()) {
+      OpenedTypeMap replacements(substitutions.begin(), substitutions.end());
+      builderType = cs.openType(builderType, replacements);
     }
     assert(!builderType->hasTypeParameter());
   }
@@ -11325,7 +11319,7 @@ ConstraintSystem::simplifyRestrictedConstraintImpl(
                   FunctionType::get({FunctionType::Param(type1)}, type2),
                   memberTy, applicationLoc);
 
-    ImplicitValueConversions.push_back(
+    ImplicitValueConversions.insert(
         {getConstraintLocator(locator), restriction});
     return SolutionKind::Solved;
   }
@@ -11361,8 +11355,8 @@ ConstraintSystem::simplifyRestrictedConstraint(
       addFixConstraint(fix, matchKind, type1, type2, locator);
     }
 
-    ConstraintRestrictions.push_back(
-        std::make_tuple(type1.getPointer(), type2.getPointer(), restriction));
+    ConstraintRestrictions.insert({
+        std::make_pair(type1.getPointer(), type2.getPointer()), restriction});
     return SolutionKind::Solved;
   }
   case SolutionKind::Unsolved:
@@ -11522,7 +11516,7 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) {
     return true;
 
   if (isAugmentingFix(fix)) {
-    Fixes.push_back(fix);
+    Fixes.insert(fix);
     return false;
   }
 
@@ -11547,7 +11541,7 @@ bool ConstraintSystem::recordFix(ConstraintFix *fix, unsigned impact) {
   }
 
   if (!found)
-    Fixes.push_back(fix);
+    Fixes.insert(fix);
 
   return false;
 }
@@ -11569,7 +11563,7 @@ void ConstraintSystem::recordAnyTypeVarAsPotentialHole(Type type) {
 void ConstraintSystem::recordMatchCallArgumentResult(
     ConstraintLocator *locator, MatchCallArgumentResult result) {
   assert(locator->isLastElement<LocatorPathElt::ApplyArgument>());
-  argumentMatchingChoices.push_back({locator, result});
+  argumentMatchingChoices.insert({locator, result});
 }
 
 ConstraintSystem::SolutionKind ConstraintSystem::simplifyFixConstraint(