AST: Simplify SubstitutionMap representation

There was a bunch of logic to lazily populate replacement types
corresponding to reducible generic parameters. This didn't seem
to have a clear purpose so let's remove it.

Author: slavapestov

include/swift/AST/SubstitutionMap.h

@@ -73,22 +73,6 @@ class SubstitutionMap {
   /// signature nor any replacement types/conformances.
   Storage *storage = nullptr;
 
-public:
-  /// Retrieve the array of replacement types, which line up with the
-  /// generic parameters.
-  ///
-  /// Note that the types may be null, for cases where the generic parameter
-  /// is concrete but hasn't been queried yet.
-  ///
-  /// Prefer \c getReplacementTypes, this is public for printing purposes.
-  ArrayRef<Type> getReplacementTypesBuffer() const;
-
-private:
-  MutableArrayRef<Type> getReplacementTypesBuffer();
-
-  /// Retrieve a mutable reference to the buffer of conformances.
-  MutableArrayRef<ProtocolConformanceRef> getConformancesBuffer();
-
   /// Form a substitution map for the given generic signature with the
   /// specified replacement types and conformances.
   SubstitutionMap(GenericSignature genericSig,

lib/AST/ASTContext.cpp

@@ -5296,21 +5296,11 @@ void SubstitutionMap::Storage::Profile(
   id.AddPointer(genericSig.getPointer());
   if (!genericSig) return;
 
-  // Profile those replacement types that corresponding to canonical generic
-  // parameters within the generic signature.
-  id.AddInteger(replacementTypes.size());
-
-  unsigned i = 0;
-  genericSig->forEachParam([&](GenericTypeParamType *gp, bool canonical) {
-    if (canonical)
-      id.AddPointer(replacementTypes[i].getPointer());
-    else
-      id.AddPointer(nullptr);
-    ++i;
-  });
+  // Replacement types.
+  for (auto replacementType : replacementTypes)
+    id.AddPointer(replacementType.getPointer());
 
   // Conformances.
-  id.AddInteger(conformances.size());
   for (auto conformance : conformances)
     id.AddPointer(conformance.getOpaqueValue());
 }

lib/AST/ASTDumper.cpp

@@ -3816,16 +3816,13 @@ class PrintConformance : public PrintBase {
 
     auto genericParams = genericSig.getGenericParams();
     auto replacementTypes =
-    static_cast<const SubstitutionMap &>(map).getReplacementTypesBuffer();
+    static_cast<const SubstitutionMap &>(map).getReplacementTypes();
     for (unsigned i : indices(genericParams)) {
       if (style == SubstitutionMap::DumpStyle::Minimal) {
         printFieldRaw([&](raw_ostream &out) {
           genericParams[i]->print(out);
           out << " -> ";
-          if (replacementTypes[i])
-            out << replacementTypes[i];
-          else
-            out << "<unresolved concrete type>";
+          out << replacementTypes[i];
         }, "");
       } else {
         printRecArbitrary([&](StringRef label) {
@@ -3834,8 +3831,7 @@ class PrintConformance : public PrintBase {
             genericParams[i]->print(out);
             out << " -> ";
           }, "");
-          if (replacementTypes[i])
-            printRec(replacementTypes[i]);
+          printRec(replacementTypes[i]);
           printFoot();
         });
       }

lib/AST/SubstitutionMap.cpp

@@ -55,7 +55,6 @@ SubstitutionMap::Storage::Storage(
             getReplacementTypes().data());
   std::copy(conformances.begin(), conformances.end(),
             getConformances().data());
-  populatedAllReplacements = false;
 }
 
 SubstitutionMap::SubstitutionMap(
@@ -69,20 +68,6 @@ SubstitutionMap::SubstitutionMap(
 #endif
 }
 
-ArrayRef<Type> SubstitutionMap::getReplacementTypesBuffer() const {
-  return storage ? storage->getReplacementTypes() : ArrayRef<Type>();
-}
-
-MutableArrayRef<Type> SubstitutionMap::getReplacementTypesBuffer() {
-  return storage ? storage->getReplacementTypes() : MutableArrayRef<Type>();
-}
-
-MutableArrayRef<ProtocolConformanceRef>
-SubstitutionMap::getConformancesBuffer() {
-  return storage ? storage->getConformances()
-                 : MutableArrayRef<ProtocolConformanceRef>();
-}
-
 ArrayRef<ProtocolConformanceRef> SubstitutionMap::getConformances() const {
   return storage ? storage->getConformances()
                  : ArrayRef<ProtocolConformanceRef>();
@@ -91,16 +76,7 @@ ArrayRef<ProtocolConformanceRef> SubstitutionMap::getConformances() const {
 ArrayRef<Type> SubstitutionMap::getReplacementTypes() const {
   if (empty()) return { };
 
-  // Make sure we've filled in all of the replacement types.
-  if (!storage->populatedAllReplacements) {
-    for (auto gp : getGenericSignature().getGenericParams()) {
-      (void)lookupSubstitution(cast<SubstitutableType>(gp->getCanonicalType()));
-    }
-
-    storage->populatedAllReplacements = true;
-  }
-
-  return getReplacementTypesBuffer();
+  return storage->getReplacementTypes();
 }
 
 ArrayRef<Type> SubstitutionMap::getInnermostReplacementTypes() const {
@@ -126,32 +102,32 @@ bool SubstitutionMap::hasAnySubstitutableParams() const {
 }
 
 bool SubstitutionMap::hasArchetypes() const {
-  for (Type replacementTy : getReplacementTypesBuffer()) {
-    if (replacementTy && replacementTy->hasArchetype())
+  for (Type replacementTy : getReplacementTypes()) {
+    if (replacementTy->hasArchetype())
       return true;
   }
   return false;
 }
 
 bool SubstitutionMap::hasLocalArchetypes() const {
-  for (Type replacementTy : getReplacementTypesBuffer()) {
-    if (replacementTy && replacementTy->hasLocalArchetype())
+  for (Type replacementTy : getReplacementTypes()) {
+    if (replacementTy->hasLocalArchetype())
       return true;
   }
   return false;
 }
 
 bool SubstitutionMap::hasOpaqueArchetypes() const {
-  for (Type replacementTy : getReplacementTypesBuffer()) {
-    if (replacementTy && replacementTy->hasOpaqueArchetype())
+  for (Type replacementTy : getReplacementTypes()) {
+    if (replacementTy->hasOpaqueArchetype())
       return true;
   }
   return false;
 }
 
 bool SubstitutionMap::hasDynamicSelf() const {
-  for (Type replacementTy : getReplacementTypesBuffer()) {
-    if (replacementTy && replacementTy->hasDynamicSelfType())
+  for (Type replacementTy : getReplacementTypes()) {
+    if (replacementTy->hasDynamicSelfType())
       return true;
   }
   return false;
@@ -162,8 +138,8 @@ bool SubstitutionMap::isCanonical() const {
 
   if (!getGenericSignature()->isCanonical()) return false;
 
-  for (Type replacementTy : getReplacementTypesBuffer()) {
-    if (replacementTy && !replacementTy->isCanonical())
+  for (Type replacementTy : getReplacementTypes()) {
+    if (!replacementTy->isCanonical())
       return false;
   }
 
@@ -182,11 +158,8 @@ SubstitutionMap SubstitutionMap::getCanonical(bool canonicalizeSignature) const
   if (canonicalizeSignature) sig = sig.getCanonicalSignature();
 
   SmallVector<Type, 4> replacementTypes;
-  for (Type replacementType : getReplacementTypesBuffer()) {
-    if (replacementType)
-      replacementTypes.push_back(replacementType->getCanonicalType());
-    else
-      replacementTypes.push_back(nullptr);
+  for (Type replacementType : getReplacementTypes()) {
+    replacementTypes.push_back(replacementType->getCanonicalType());
   }
 
   SmallVector<ProtocolConformanceRef, 4> conformances;
@@ -237,13 +210,7 @@ SubstitutionMap SubstitutionMap::get(GenericSignature genericSig,
   SmallVector<Type, 4> replacementTypes;
   replacementTypes.reserve(genericSig.getGenericParams().size());
 
-  genericSig->forEachParam([&](GenericTypeParamType *gp, bool canonical) {
-    // Don't eagerly form replacements for non-canonical generic parameters.
-    if (!canonical) {
-      replacementTypes.push_back(Type());
-      return;
-    }
-
+  for (auto *gp : genericSig.getGenericParams()) {
     // Record the replacement.
     Type replacement = Type(gp).subst(IFS);
 
@@ -252,7 +219,7 @@ SubstitutionMap SubstitutionMap::get(GenericSignature genericSig,
            "replacement for pack parameter must be a pack type");
 
     replacementTypes.push_back(replacement);
-  });
+  }
 
   // Form the stored conformances.
   SmallVector<ProtocolConformanceRef, 4> conformances;
@@ -292,11 +259,7 @@ Type SubstitutionMap::lookupSubstitution(CanSubstitutableType type) const {
   // Find the index of the replacement type based on the generic parameter we
   // have.
   auto genericParam = cast<GenericTypeParamType>(type);
-  auto mutableThis = const_cast<SubstitutionMap *>(this);
-  auto replacementTypes = mutableThis->getReplacementTypesBuffer();
-  auto genericSig = getGenericSignature();
-  assert(genericSig);
-  auto genericParams = genericSig.getGenericParams();
+  auto genericParams = getGenericSignature().getGenericParams();
   auto replacementIndex =
     GenericParamKey(genericParam).findIndexIn(genericParams);
 
@@ -305,45 +268,7 @@ Type SubstitutionMap::lookupSubstitution(CanSubstitutableType type) const {
   if (replacementIndex == genericParams.size())
     return Type();
 
-  // If we already have a replacement type, return it.
-  Type &replacementType = replacementTypes[replacementIndex];
-  if (replacementType)
-    return replacementType;
-
-  // The generic parameter may have been made concrete by the generic signature,
-  // substitute into the concrete type.
-  if (auto concreteType = genericSig->getConcreteType(genericParam)) {
-    // Set the replacement type to an error, to block infinite recursion.
-    replacementType = ErrorType::get(concreteType);
-
-    // Substitute into the replacement type.
-    replacementType = concreteType.subst(*this);
-
-    // If the generic signature is canonical, canonicalize the replacement type.
-    if (getGenericSignature()->isCanonical())
-      replacementType = replacementType->getCanonicalType();
-
-    return replacementType;
-  }
-
-  // The generic parameter may not be reduced. Retrieve the reduced
-  // type, which will be dependent.
-  CanType canonicalType = genericSig.getReducedType(genericParam);
-
-  // If nothing changed, we don't have a replacement.
-  if (canonicalType == type) return Type();
-
-  // If we're left with a substitutable type, substitute into that.
-  // First, set the replacement type to an error, to block infinite recursion.
-  replacementType = ErrorType::get(type);
-
-  replacementType = lookupSubstitution(cast<SubstitutableType>(canonicalType));
-
-  // If the generic signature is canonical, canonicalize the replacement type.
-  if (getGenericSignature()->isCanonical())
-    replacementType = replacementType->getCanonicalType();
-
-  return replacementType;
+  return getReplacementTypes()[replacementIndex];
 }
 
 ProtocolConformanceRef
@@ -500,12 +425,7 @@ SubstitutionMap SubstitutionMap::subst(InFlightSubstitution &IFS) const {
   if (empty()) return SubstitutionMap();
 
   SmallVector<Type, 4> newSubs;
-  for (Type type : getReplacementTypesBuffer()) {
-    if (!type) {
-      // Non-canonical parameter.
-      newSubs.push_back(Type());
-      continue;
-    }
+  for (Type type : getReplacementTypes()) {
     newSubs.push_back(type.subst(IFS));
     assert(type->is<PackType>() == newSubs.back()->is<PackType>() &&
            "substitution changed the pack-ness of a replacement type");
@@ -843,7 +763,7 @@ bool SubstitutionMap::isIdentity() const {
 
   GenericSignature sig = getGenericSignature();
   bool hasNonIdentityReplacement = false;
-  auto replacements = getReplacementTypesBuffer();
+  auto replacements = getReplacementTypes();
 
   sig->forEachParam([&](GenericTypeParamType *paramTy, bool isCanonical) {
     if (isCanonical) {

lib/AST/SubstitutionMapStorage.h

@@ -40,10 +40,7 @@ class SubstitutionMap::Storage final
 
   /// The number of conformance requirements, cached to avoid constantly
   /// recomputing it on conformance-buffer access.
-  const unsigned numConformanceRequirements : 31;
-
-  /// Whether we've populated all replacement types already.
-  unsigned populatedAllReplacements : 1;
+  const unsigned numConformanceRequirements;
 
   Storage() = delete;