AST: More robust recursion check for opaque types with infinite underlying types

slavapestov · slavapestov · commit 103428fe804b · 2025-07-18T20:03:03.000-04:00
Tracking seen declarations and substitution maps only detects the
situation where the opaque type's underlying type contains itself
with the same substitution map. However, it is also possible to
recurse with a different substitution map.

In general termination is undecidable with a problem like this,
so instead of trying to catch cycles, just impose a termination
limit.

This converts a stack overflow into an assertion, which is still
not ideal; we should really diagnose something instead. But this
is a first step.
diff --git a/include/swift/AST/InFlightSubstitution.h b/include/swift/AST/InFlightSubstitution.h
@@ -28,22 +28,27 @@ namespace swift {
 class SubstitutionMap;
 
 class InFlightSubstitution {
-  SubstOptions Options;
   TypeSubstitutionFn BaselineSubstType;
   LookupConformanceFn BaselineLookupConformance;
+  SubstOptions Options;
   RecursiveTypeProperties Props;
+  unsigned RemainingCount : 16;
+  unsigned RemainingDepth : 15;
+  unsigned LimitReached : 1;
 
   struct ActivePackExpansion {
     bool isSubstExpansion = false;
     unsigned expansionIndex = 0;
   };
-  SmallVector<ActivePackExpansion, 4> ActivePackExpansions;
+  llvm::SmallVector<ActivePackExpansion, 4> ActivePackExpansions;
 
   Type projectLaneFromPackType(
       Type substType, unsigned level);
   ProtocolConformanceRef projectLaneFromPackConformance(
       PackConformance *substPackConf, unsigned level);
 
+  bool checkLimits();
+
 public:
   InFlightSubstitution(TypeSubstitutionFn substType,
                        LookupConformanceFn lookupConformance,
@@ -150,6 +155,10 @@ class InFlightSubstitution {
 
   /// Is the given type invariant to substitution?
   bool isInvariant(Type type) const;
+
+  bool wasLimitReached() const {
+    return LimitReached;
+  }
 };
 
 /// A helper classes that provides stable storage for the query
diff --git a/include/swift/AST/Types.h b/include/swift/AST/Types.h
@@ -7077,24 +7077,16 @@ enum class OpaqueSubstitutionKind {
 /// archetypes with underlying types visible at a given resilience expansion
 /// to their underlying types.
 class ReplaceOpaqueTypesWithUnderlyingTypes {
-public:
-  using SeenDecl = std::pair<OpaqueTypeDecl *, SubstitutionMap>;
 private:
   ResilienceExpansion contextExpansion;
   llvm::PointerIntPair<const DeclContext *, 1, bool> inContextAndIsWholeModule;
-  llvm::DenseSet<SeenDecl> *seenDecls;
 
 public:
   ReplaceOpaqueTypesWithUnderlyingTypes(const DeclContext *inContext,
                                         ResilienceExpansion contextExpansion,
                                         bool isWholeModuleContext)
       : contextExpansion(contextExpansion),
-        inContextAndIsWholeModule(inContext, isWholeModuleContext),
-        seenDecls(nullptr) {}
-
-  ReplaceOpaqueTypesWithUnderlyingTypes(
-      const DeclContext *inContext, ResilienceExpansion contextExpansion,
-      bool isWholeModuleContext, llvm::DenseSet<SeenDecl> &seen);
+        inContextAndIsWholeModule(inContext, isWholeModuleContext) {}
 
   /// TypeSubstitutionFn
   Type operator()(SubstitutableType *maybeOpaqueType) const;
diff --git a/lib/AST/SubstitutionMap.cpp b/lib/AST/SubstitutionMap.cpp
@@ -624,9 +624,23 @@ SubstitutionMap swift::substOpaqueTypesWithUnderlyingTypes(
   ReplaceOpaqueTypesWithUnderlyingTypes replacer(
       context.getContext(), context.getResilienceExpansion(),
       context.isWholeModuleContext());
-  return subs.subst(replacer, replacer,
-                    SubstFlags::SubstituteOpaqueArchetypes |
-                    SubstFlags::PreservePackExpansionLevel);
+  InFlightSubstitution IFS(replacer, replacer,
+                           SubstFlags::SubstituteOpaqueArchetypes |
+                           SubstFlags::PreservePackExpansionLevel);
+
+  auto substSubs = subs.subst(IFS);
+
+  if (IFS.wasLimitReached()) {
+    ABORT([&](auto &out) {
+      out << "Possible non-terminating type substitution detected\n\n";
+      out << "Original substitution map:\n";
+      subs.dump(out, SubstitutionMap::DumpStyle::NoConformances);
+      out << "Substituted substitution map:\n";
+      substSubs.dump(out, SubstitutionMap::DumpStyle::NoConformances);
+    });
+  }
+
+  return substSubs;
 }
 
 Type OuterSubstitutions::operator()(SubstitutableType *type) const {
diff --git a/lib/AST/TypeSubstitution.cpp b/lib/AST/TypeSubstitution.cpp
@@ -123,9 +123,16 @@ operator()(InFlightSubstitution &IFS, Type dependentType,
 InFlightSubstitution::InFlightSubstitution(TypeSubstitutionFn substType,
                                            LookupConformanceFn lookupConformance,
                                            SubstOptions options)
-  : Options(options),
-    BaselineSubstType(substType),
-    BaselineLookupConformance(lookupConformance) {
+  : BaselineSubstType(substType),
+    BaselineLookupConformance(lookupConformance),
+    Options(options) {
+
+  LimitReached = 0;
+
+  // FIXME: Make these configurable
+  RemainingCount = 1000;
+  RemainingDepth = 20;
+
   // FIXME: Don't substitute type parameters if one of the special flags is set.
   Props |= RecursiveTypeProperties::HasTypeParameter;
 
@@ -233,6 +240,14 @@ Type InFlightSubstitution::projectLaneFromPackType(Type substType,
   }
 }
 
+bool InFlightSubstitution::checkLimits() {
+  if (RemainingCount == 0 || RemainingDepth == 0) {
+    LimitReached = true;
+    return true;
+  }
+  return false;
+}
+
 Type InFlightSubstitution::substType(SubstitutableType *origType,
                                      unsigned level) {
   auto substType = BaselineSubstType(origType);
@@ -244,6 +259,20 @@ Type InFlightSubstitution::substType(SubstitutableType *origType,
   else
     substType = substType->increasePackElementLevel(level);
 
+  // Opaque type replacement must iterate until fixed point.
+  if (shouldSubstituteOpaqueArchetypes() &&
+      substType->hasOpaqueArchetype() &&
+      !substType->isEqual(origType)) {
+    if (checkLimits()) {
+      return ErrorType::get(substType);
+    }
+
+    --RemainingCount;
+    --RemainingDepth;
+    substType = substType.subst(*this);
+    ++RemainingDepth;
+  }
+
   return substType;
 }
 
@@ -271,6 +300,19 @@ InFlightSubstitution::lookupConformance(Type dependentType,
         substConfRef.getPack(), level);
   }
 
+  // Opaque type replacement must iterate until fixed point.
+  if (shouldSubstituteOpaqueArchetypes() && substConfRef &&
+      substConfRef.getType()->hasOpaqueArchetype() &&
+      !substConfRef.getType()->isEqual(dependentType)) {
+    if (checkLimits()) {
+      return ProtocolConformanceRef::forInvalid();
+    }
+    --RemainingCount;
+    --RemainingDepth;
+    substConfRef = substConfRef.subst(*this);
+    ++RemainingDepth;
+  }
+
   return substConfRef;
 }
 
@@ -906,17 +948,6 @@ ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
   return OpaqueSubstitutionKind::SubstituteNonResilientModule;
 }
 
-static Type substOpaqueTypesWithUnderlyingTypesRec(
-    Type ty, const DeclContext *inContext, ResilienceExpansion contextExpansion,
-    bool isWholeModuleContext,
-    llvm::DenseSet<ReplaceOpaqueTypesWithUnderlyingTypes::SeenDecl> &decls) {
-  ReplaceOpaqueTypesWithUnderlyingTypes replacer(inContext, contextExpansion,
-                                                 isWholeModuleContext, decls);
-  return ty.subst(replacer, replacer,
-                  SubstFlags::SubstituteOpaqueArchetypes |
-                  SubstFlags::PreservePackExpansionLevel);
-}
-
 /// Checks that \p dc has access to \p ty for the purposes of an opaque
 /// substitution described by \p kind.
 ///
@@ -973,13 +1004,6 @@ static bool canSubstituteTypeInto(Type ty, const DeclContext *dc,
   llvm_unreachable("invalid substitution kind");
 }
 
-ReplaceOpaqueTypesWithUnderlyingTypes::ReplaceOpaqueTypesWithUnderlyingTypes(
-    const DeclContext *inContext, ResilienceExpansion contextExpansion,
-    bool isWholeModuleContext, llvm::DenseSet<SeenDecl> &seen)
-    : contextExpansion(contextExpansion),
-      inContextAndIsWholeModule(inContext, isWholeModuleContext),
-      seenDecls(&seen) {}
-
 Type ReplaceOpaqueTypesWithUnderlyingTypes::
 operator()(SubstitutableType *maybeOpaqueType) const {
   auto *archetype = dyn_cast<OpaqueTypeArchetypeType>(maybeOpaqueType);
@@ -1027,36 +1051,7 @@ operator()(SubstitutableType *maybeOpaqueType) const {
   // for its type arguments. We perform this substitution after checking for
   // visibility, since we do not want the result of the visibility check to
   // depend on the substitutions previously applied.
-  auto substTy = partialSubstTy.subst(outerSubs);
-
-  // If the type changed, but still contains opaque types, recur.
-  if (!substTy->isEqual(maybeOpaqueType) && substTy->hasOpaqueArchetype()) {
-    SeenDecl seenKey(decl, outerSubs);
-    if (auto *alreadySeen = this->seenDecls) {
-      // Detect substitution loops. If we find one, just bounce the original
-      // type back to the caller. This substitution will fail at runtime
-      // instead.
-      if (!alreadySeen->insert(seenKey).second) {
-        return maybeOpaqueType;
-      }
-
-      auto res = ::substOpaqueTypesWithUnderlyingTypesRec(
-          substTy, inContext, contextExpansion, isContextWholeModule,
-          *alreadySeen);
-      alreadySeen->erase(seenKey);
-      return res;
-    } else {
-      // We're the top of the stack for the recursion check. Allocate a set of
-      // opaque result type decls we've already seen for the rest of the check.
-      llvm::DenseSet<SeenDecl> seenDecls;
-      seenDecls.insert(seenKey);
-      return ::substOpaqueTypesWithUnderlyingTypesRec(
-          substTy, inContext, contextExpansion, isContextWholeModule,
-          seenDecls);
-    }
-  }
-
-  return substTy;
+  return partialSubstTy.subst(outerSubs);
 }
 
 Type swift::substOpaqueTypesWithUnderlyingTypes(Type ty,
@@ -1068,9 +1063,25 @@ Type swift::substOpaqueTypesWithUnderlyingTypes(Type ty,
   ReplaceOpaqueTypesWithUnderlyingTypes replacer(
       context.getContext(), context.getResilienceExpansion(),
       context.isWholeModuleContext());
-  SubstOptions flags = (SubstFlags::SubstituteOpaqueArchetypes |
-                        SubstFlags::PreservePackExpansionLevel);
-  return ty.subst(replacer, replacer, flags);
+  InFlightSubstitution IFS(replacer, replacer,
+                           SubstFlags::SubstituteOpaqueArchetypes |
+                           SubstFlags::PreservePackExpansionLevel);
+
+  auto substTy = ty.subst(IFS);
+
+  // FIXME: This should be a diagnostic at the source location of the innermost
+  // request or something.
+  if (IFS.wasLimitReached()) {
+    ABORT([&](auto &out) {
+      out << "Possible non-terminating type substitution detected\n\n";
+      out << "Original type:\n";
+      ty.dump(out);
+      out << "Substituted type:\n";
+      substTy.dump(out);
+    });
+  }
+
+  return substTy;
 }
 
 CanType
@@ -1080,24 +1091,29 @@ swift::substOpaqueTypesWithUnderlyingTypes(CanType ty,
       ->getCanonicalType();
 }
 
-static ProtocolConformanceRef substOpaqueTypesWithUnderlyingTypesRec(
-    ProtocolConformanceRef ref, const DeclContext *inContext,
-    ResilienceExpansion contextExpansion, bool isWholeModuleContext,
-    llvm::DenseSet<ReplaceOpaqueTypesWithUnderlyingTypes::SeenDecl> &decls) {
-  ReplaceOpaqueTypesWithUnderlyingTypes replacer(inContext, contextExpansion,
-                                                 isWholeModuleContext, decls);
-  return ref.subst(replacer, replacer,
-                   SubstFlags::SubstituteOpaqueArchetypes |
-                   SubstFlags::PreservePackExpansionLevel);
-}
-
 ProtocolConformanceRef swift::substOpaqueTypesWithUnderlyingTypes(
     ProtocolConformanceRef ref, TypeExpansionContext context) {
   ReplaceOpaqueTypesWithUnderlyingTypes replacer(
       context.getContext(), context.getResilienceExpansion(),
       context.isWholeModuleContext());
-  return ref.subst(replacer, replacer,
-                   SubstFlags::SubstituteOpaqueArchetypes);
+  InFlightSubstitution IFS(replacer, replacer,
+                           SubstFlags::SubstituteOpaqueArchetypes);
+
+  auto substRef = ref.subst(IFS);
+
+  // FIXME: This should be a diagnostic at the source location of the innermost
+  // request or something.
+  if (IFS.wasLimitReached()) {
+    ABORT([&](auto &out) {
+      out << "Possible non-terminating conformance substitution detected\n\n";
+      out << "Original conformance:\n";
+      ref.dump(out);
+      out << "\nSubstituted conformance:\n";
+      substRef.dump(out);
+    });
+  }
+
+  return substRef;
 }
 
 ProtocolConformanceRef ReplaceOpaqueTypesWithUnderlyingTypes::
@@ -1151,36 +1167,7 @@ operator()(InFlightSubstitution &IFS, Type maybeOpaqueType,
   auto partialSubstRef =
       subs->lookupConformance(archetype->getInterfaceType()->getCanonicalType(),
                               protocol);
-  auto substRef = partialSubstRef.subst(outerSubs);
-
-  // If the type still contains opaque types, recur.
-  if (substRef.getType()->hasOpaqueArchetype()) {
-    SeenDecl seenKey(decl, outerSubs);
-    
-    if (auto *alreadySeen = this->seenDecls) {
-      // Detect substitution loops. If we find one, just bounce the original
-      // type back to the caller. This substitution will fail at runtime
-      // instead.
-      if (!alreadySeen->insert(seenKey).second) {
-        return lookupConformance(maybeOpaqueType.subst(IFS), protocol);
-      }
-
-      auto res = ::substOpaqueTypesWithUnderlyingTypesRec(
-          substRef, inContext, contextExpansion, isContextWholeModule,
-          *alreadySeen);
-      alreadySeen->erase(seenKey);
-      return res;
-    } else {
-      // We're the top of the stack for the recursion check. Allocate a set of
-      // opaque result type decls we've already seen for the rest of the check.
-      llvm::DenseSet<SeenDecl> seenDecls;
-      seenDecls.insert(seenKey);
-      return ::substOpaqueTypesWithUnderlyingTypesRec(
-          substRef, inContext, contextExpansion, isContextWholeModule,
-          seenDecls);
-    }
-  }
-  return substRef;
+  return partialSubstRef.subst(outerSubs);
 }
 
 Type ReplaceExistentialArchetypesWithConcreteTypes::getInterfaceType(
