AST: Fix some nested generics issues exposed by generic initializer inheritance

BoundGenericType::getSubsitutions() would only look at the bound
generic arguments of the innermost type, ignoring parent types.
However, it would then proceed to walk the AllArchetypes list
of all outer generic parameter lists when forming the final
result.

The gatherAllSubstitutions() would also walk through parent types.
As a result, outer generic parameters would appear multiple
times.

Simplify gatherAllSubstitutions() to just skip to the innermost
BoundGenericType, and delegate to getSubsitutions() for the rest.

Most calls to gatherAllSubstitutions() are from SILGen it seems,
and fix only fixes one compiler_crasher.

However an upcoming patch adds a new call to gatherAllSubstitutions()
which caused some crashers to regress, so I'm going to fix it
properly here.

Author: slavapestov

include/swift/AST/Types.h

@@ -571,17 +571,6 @@ class alignas(1 << TypeAlignInBits) TypeBase {
   /// stitched together from multiple lists of generic arguments.
   ArrayRef<Type> getAllGenericArgs(SmallVectorImpl<Type> &scratch);
 
-  /// Gather all of the substitutions used to produce the given specialized type
-  /// from its unspecialized type.
-  ///
-  /// \param scratchSpace The substitutions will be written into this scratch
-  /// space if a single substitutions array cannot be returned.
-  ArrayRef<Substitution> gatherAllSubstitutions(
-                           ModuleDecl *module,
-                           SmallVectorImpl<Substitution> &scratchSpace,
-                           LazyResolver *resolver,
-                           DeclContext *gpContext = nullptr);
-
   /// Gather all of the substitutions used to produce the given specialized type
   /// from its unspecialized type.
   ///

lib/AST/Module.cpp

@@ -696,15 +696,40 @@ ArrayRef<Substitution> BoundGenericType::getSubstitutions(
 
   // Compute the set of substitutions.
   TypeSubstitutionMap substitutions;
-  auto genericParams = gpContext->getGenericParamsOfContext();
-  unsigned index = 0;
-  for (Type arg : canon->getGenericArgs()) {
-    auto gp = genericParams->getParams()[index++];
-    auto archetype = gp->getArchetype();
-    substitutions[archetype] = arg;
+
+  CanType parent(canon);
+  auto *parentDC = gpContext;
+  while (parent) {
+    if (auto boundGeneric = dyn_cast<BoundGenericType>(parent)) {
+      auto genericParams = parentDC->getGenericParamsOfContext();
+      unsigned index = 0;
+
+      assert(boundGeneric->getGenericArgs().size() ==
+             genericParams->getParams().size());
+
+      for (Type arg : boundGeneric->getGenericArgs()) {
+        auto gp = genericParams->getParams()[index++];
+        auto archetype = gp->getArchetype();
+        substitutions[archetype] = arg;
+      }
+
+      parent = CanType(boundGeneric->getParent());
+      parentDC = parentDC->getParent();
+      continue;
+    }
+
+    if (auto nominal = dyn_cast<NominalType>(parent)) {
+      parent = CanType(nominal->getParent());
+      parentDC = parentDC->getParent();
+      continue;
+    }
+
+    llvm_unreachable("Not a nominal or bound generic type");
   }
 
   // Collect all of the archetypes.
+  auto *genericParams = gpContext->getGenericParamsOfContext();
+
   SmallVector<ArchetypeType *, 2> allArchetypesList;
   ArrayRef<ArchetypeType *> allArchetypes = genericParams->getAllArchetypes();
   if (genericParams->getOuterParameters()) {
@@ -727,7 +752,7 @@ ArrayRef<Substitution> BoundGenericType::getSubstitutions(
   bool hasTypeVariables = canon->hasTypeVariable();
   SmallVector<Substitution, 4> resultSubstitutions;
   resultSubstitutions.resize(allArchetypes.size());
-  index = 0;
+  unsigned index = 0;
   for (auto archetype : allArchetypes) {
     // Substitute into the type.
     SubstOptions options;
@@ -913,9 +938,7 @@ Module::lookupConformance(Type type, ProtocolDecl *protocol,
     if (!explicitConformanceType->isEqual(type)) {
       // Gather the substitutions we need to map the generic conformance to
       // the specialized conformance.
-      SmallVector<Substitution, 4> substitutionsVec;
-      auto substitutions = type->gatherAllSubstitutions(this, substitutionsVec,
-                                                        resolver,
+      auto substitutions = type->gatherAllSubstitutions(this, resolver,
                                                         explicitConformanceDC);
 
       for (auto sub : substitutions) {

lib/AST/Type.cpp

@@ -331,25 +331,10 @@ ArrayRef<Type> TypeBase::getAllGenericArgs(SmallVectorImpl<Type> &scratch) {
 
 ArrayRef<Substitution>
 TypeBase::gatherAllSubstitutions(Module *module,
-                                 SmallVectorImpl<Substitution> &scratchSpace,
                                  LazyResolver *resolver,
                                  DeclContext *gpContext) {
   Type type(this);
-  SmallVector<ArrayRef<Substitution>, 2> allSubstitutions;
-  scratchSpace.clear();
-
   while (type) {
-    // Record the substitutions in a bound generic type.
-    if (auto boundGeneric = type->getAs<BoundGenericType>()) {
-      allSubstitutions.push_back(boundGeneric->getSubstitutions(module,
-                                                                resolver,
-                                                                gpContext));
-      type = boundGeneric->getParent();
-      if (gpContext)
-        gpContext = gpContext->getParent();
-      continue;
-    }
-
     // Skip to the parent of a nominal type.
     if (auto nominal = type->getAs<NominalType>()) {
       type = nominal->getParent();
@@ -358,36 +343,18 @@ TypeBase::gatherAllSubstitutions(Module *module,
       continue;
     }
 
+    // Return the substitutions in a bound generic type.
+    // This walks any further parent types.
+    if (auto boundGeneric = type->getAs<BoundGenericType>())
+      return boundGeneric->getSubstitutions(module, resolver, gpContext);
+
     llvm_unreachable("Not a nominal or bound generic type");
   }
 
-  // If there are no substitutions, return an empty array.
-  if (allSubstitutions.empty())
-    return { };
-
-  // If there is only one list of substitutions, return it. There's no
-  // need to copy it.
-  if (allSubstitutions.size() == 1)
-    return allSubstitutions.front();
-
-  for (auto substitutions : allSubstitutions)
-    scratchSpace.append(substitutions.begin(), substitutions.end());
-  return scratchSpace;
+  // Not a generic type.
+  return { };
 }
 
-ArrayRef<Substitution> TypeBase::gatherAllSubstitutions(Module *module,
-                                                        LazyResolver *resolver,
-                                                        DeclContext *gpContext){
-  SmallVector<Substitution, 4> scratchSpace;
-  auto subs = gatherAllSubstitutions(module, scratchSpace, resolver,
-                                     gpContext);
-  if (scratchSpace.empty())
-    return subs;
-
-  return getASTContext().AllocateCopy(subs);
-}
-
-
 bool TypeBase::isUnspecializedGeneric() {
   CanType CT = getCanonicalType();
   if (CT.getPointer() != this)

lib/SILGen/SILGenFunction.cpp

@@ -834,12 +834,11 @@ MetatypeInst *SILGenBuilder::createMetatype(SILLocation loc, SILType metatype) {
   case MetatypeRepresentation::Thick:
   case MetatypeRepresentation::ObjC: {
     // Walk the type recursively to look for substitutions we may need.
-    SmallVector<Substitution, 2> subsBuf;
     theMetatype.getInstanceType().findIf([&](Type t) -> bool {
       if (!t->getAnyNominal())
         return false;
 
-      auto subs = t->gatherAllSubstitutions(SGM.SwiftModule, subsBuf, nullptr);
+      auto subs = t->gatherAllSubstitutions(SGM.SwiftModule, nullptr);
       SGM.useConformancesFromSubstitutions(subs);
       return false;
     });

validation-test/compiler_crashers/28258-swift-specializedprotocolconformance-gettypewitnesssubstanddecl.swift renamed to validation-test/compiler_crashers_fixed/28258-swift-specializedprotocolconformance-gettypewitnesssubstanddecl.swift

@@ -5,6 +5,6 @@
 // See http://swift.org/LICENSE.txt for license information
 // See http://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
 
-// RUN: not --crash %target-swift-frontend %s -parse
+// RUN: not %target-swift-frontend %s -parse
 // REQUIRES: asserts
 func<{struct c<T{enum S<T{struct A:a protocol a{associatedtype e}struct S<T:A.e