AST: Simplify SubstitutionMap representation

slavapestov · slavapestov · commit 26fffae30a02 · 2024-07-10T13:28:26.000-04:00
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.
diff --git a/include/swift/AST/SubstitutionMap.h b/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,
diff --git a/lib/AST/ASTContext.cpp b/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());
 }
diff --git a/lib/AST/ASTDumper.cpp b/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();
         });
       }
diff --git a/lib/AST/SubstitutionMap.cpp b/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) {
diff --git a/lib/AST/SubstitutionMapStorage.h b/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;
 
diff --git a/test/IDE/complete_rdar129024996.swift b/test/IDE/complete_rdar129024996.swift
@@ -46,4 +46,4 @@ extension P where Self == B<A> {
 func bar() {}
 // COMPLETE:     Begin completions, 2 items
 // COMPLETE-DAG: Decl[StaticMethod]/Super/TypeRelation[Convertible]: foo()[#S#]; name=foo()
-// COMPLETE-DAG: Decl[StaticMethod]/Super/TypeRelation[Convertible]: qux()[#B<S>#]; name=qux()
+// COMPLETE-DAG: Decl[StaticMethod]/Super/Flair[ExprSpecific]/TypeRelation[Convertible]: qux()[#any Q#]; name=qux()
diff --git a/test/IDE/complete_rdar94369218.swift b/test/IDE/complete_rdar94369218.swift
@@ -34,7 +34,7 @@ func test3<MyGeneric: MyProto>() {
       #^GENERICEXPR^#
 // GENERICEXPR: Decl[GenericTypeParam]/Local:       MyGeneric[#MyGeneric#]; name=MyGeneric
 // GENERICEXPR: Decl[Constructor]/Local:            MyGeneric({#value: String#})[#MyProto#]; name=MyGeneric(value:)
-// GENERICEXPR: Decl[Constructor]/Local:            MyGeneric({#arg: String#})[#MyStruct#]; name=MyGeneric(arg:)
+// GENERICEXPR: Decl[Constructor]/Local:            MyGeneric({#arg: String#})[#MyProto#]; name=MyGeneric(arg:)
     }
   }
 }
diff --git a/test/SourceKit/DocSupport/doc_system_module_underscored.swift.response b/test/SourceKit/DocSupport/doc_system_module_underscored.swift.response
@@ -1145,6 +1145,16 @@ protocol Other1 {
   },
   {
     key.kind: source.lang.swift.decl.extension.struct,
+    key.generic_params: [
+      {
+        key.name: "T"
+      }
+    ],
+    key.generic_requirements: [
+      {
+        key.description: "T == String"
+      }
+    ],
     key.offset: 396,
     key.length: 98,
     key.fully_annotated_decl: "<syntaxtype.keyword>extension</syntaxtype.keyword> <ref.struct usr=\"s:16UnderscoredProto1AV\">A</ref.struct>",

Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ func test3<MyGeneric: MyProto>() {`
`34`	`34`	`#^GENERICEXPR^#`
`35`	`35`	`// GENERICEXPR: Decl[GenericTypeParam]/Local: MyGeneric[#MyGeneric#]; name=MyGeneric`
`36`	`36`	`// GENERICEXPR: Decl[Constructor]/Local: MyGeneric({#value: String#})[#MyProto#]; name=MyGeneric(value:)`
`37`		`-// GENERICEXPR: Decl[Constructor]/Local: MyGeneric({#arg: String#})[#MyStruct#]; name=MyGeneric(arg:)`
	`37`	`+// GENERICEXPR: Decl[Constructor]/Local: MyGeneric({#arg: String#})[#MyProto#]; name=MyGeneric(arg:)`
`38`	`38`	`}`
`39`	`39`	`}`
`40`	`40`	`}`