Merge pull request #83736 from slavapestov/fix-rdar157329046

Sema: New opaque return type circularity check that doesn't trigger lazy type checking of bodies

Author: slavapestov

include/swift/AST/Decl.h

@@ -3659,7 +3659,8 @@ class OpaqueTypeDecl final :
 
   /// The substitutions that map the generic parameters of the opaque type to
   /// the unique underlying types, when that information is known.
-  std::optional<SubstitutionMap> getUniqueUnderlyingTypeSubstitutions() const;
+  std::optional<SubstitutionMap> getUniqueUnderlyingTypeSubstitutions(
+      bool typeCheckFunctionBodies=true) const;
 
   void setUniqueUnderlyingTypeSubstitutions(SubstitutionMap subs) {
     assert(!UniqueUnderlyingType.has_value() && "resetting underlying type?!");

include/swift/AST/Types.h

@@ -7081,15 +7081,20 @@ enum class OpaqueSubstitutionKind {
 /// to their underlying types.
 class ReplaceOpaqueTypesWithUnderlyingTypes {
 private:
-  ResilienceExpansion contextExpansion;
-  llvm::PointerIntPair<const DeclContext *, 1, bool> inContextAndIsWholeModule;
+  const DeclContext *inContext;
+  unsigned contextExpansion : 1;
+  bool isWholeModule : 1;
+  bool typeCheckFunctionBodies : 1;
 
 public:
   ReplaceOpaqueTypesWithUnderlyingTypes(const DeclContext *inContext,
                                         ResilienceExpansion contextExpansion,
-                                        bool isWholeModuleContext)
-      : contextExpansion(contextExpansion),
-        inContextAndIsWholeModule(inContext, isWholeModuleContext) {}
+                                        bool isWholeModule,
+                                        bool typeCheckFunctionBodies=true)
+      : inContext(inContext),
+        contextExpansion(unsigned(contextExpansion)),
+        isWholeModule(isWholeModule),
+        typeCheckFunctionBodies(typeCheckFunctionBodies) {}
 
   /// TypeSubstitutionFn
   Type operator()(SubstitutableType *maybeOpaqueType) const;
@@ -7105,13 +7110,6 @@ class ReplaceOpaqueTypesWithUnderlyingTypes {
   static OpaqueSubstitutionKind
   shouldPerformSubstitution(OpaqueTypeDecl *opaque, ModuleDecl *contextModule,
                             ResilienceExpansion contextExpansion);
-
-private:
-  const DeclContext *getContext() const {
-    return inContextAndIsWholeModule.getPointer();
-  }
-
-  bool isWholeModule() const { return inContextAndIsWholeModule.getInt(); }
 };
 
 /// A function object that can be used as a \c TypeSubstitutionFn and

lib/AST/Decl.cpp

@@ -10840,7 +10840,11 @@ bool OpaqueTypeDecl::exportUnderlyingType() const {
 }
 
 std::optional<SubstitutionMap>
-OpaqueTypeDecl::getUniqueUnderlyingTypeSubstitutions() const {
+OpaqueTypeDecl::getUniqueUnderlyingTypeSubstitutions(
+    bool typeCheckFunctionBodies) const {
+  if (!typeCheckFunctionBodies)
+    return UniqueUnderlyingType;
+
   return evaluateOrDefault(getASTContext().evaluator,
                            UniqueUnderlyingTypeSubstitutionsRequest{this}, {});
 }

lib/AST/TypeSubstitution.cpp

@@ -903,10 +903,10 @@ Type TypeBase::adjustSuperclassMemberDeclType(const ValueDecl *baseDecl,
 OpaqueSubstitutionKind
 ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
     OpaqueTypeDecl *opaque) const {
-  const auto *inContext = getContext();
   auto inModule = inContext ? inContext->getParentModule()
                             : opaque->getParentModule();
-  return shouldPerformSubstitution(opaque, inModule, contextExpansion);
+  return shouldPerformSubstitution(
+      opaque, inModule, ResilienceExpansion(contextExpansion));
 }
 OpaqueSubstitutionKind
 ReplaceOpaqueTypesWithUnderlyingTypes::shouldPerformSubstitution(
@@ -1025,7 +1025,7 @@ operator()(SubstitutableType *maybeOpaqueType) const {
     return maybeOpaqueType;
   }
 
-  auto subs = decl->getUniqueUnderlyingTypeSubstitutions();
+  auto subs = decl->getUniqueUnderlyingTypeSubstitutions(typeCheckFunctionBodies);
   // If the body of the opaque decl providing decl has not been type checked we
   // don't have a underlying substitution.
   if (!subs.has_value())
@@ -1038,16 +1038,12 @@ operator()(SubstitutableType *maybeOpaqueType) const {
 
   // Check that we are allowed to substitute the underlying type into the
   // context.
-  auto inContext = this->getContext();
-  auto isContextWholeModule = this->isWholeModule();
-  auto contextExpansion = this->contextExpansion;
   if (inContext &&
       partialSubstTy.findIf(
-          [inContext, substitutionKind, isContextWholeModule,
-           contextExpansion](Type t) -> bool {
+          [&](Type t) -> bool {
             if (!canSubstituteTypeInto(t, inContext, substitutionKind,
-                                       contextExpansion,
-                                       isContextWholeModule))
+                                       ResilienceExpansion(contextExpansion),
+                                       isWholeModule))
               return true;
             return false;
           }))
@@ -1154,16 +1150,12 @@ operator()(InFlightSubstitution &IFS, Type maybeOpaqueType,
 
   // Check that we are allowed to substitute the underlying type into the
   // context.
-  auto inContext = this->getContext();
-  auto isContextWholeModule = this->isWholeModule();
-  auto contextExpansion = this->contextExpansion;
   if (inContext &&
       partialSubstTy.findIf(
-          [inContext, substitutionKind, isContextWholeModule,
-          contextExpansion](Type t) -> bool {
+          [&](Type t) -> bool {
             if (!canSubstituteTypeInto(t, inContext, substitutionKind,
-                                       contextExpansion,
-                                       isContextWholeModule))
+                                       ResilienceExpansion(contextExpansion),
+                                       isWholeModule))
               return true;
             return false;
           })) {

lib/Sema/MiscDiagnostics.cpp

@@ -27,7 +27,6 @@
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/ExistentialLayout.h"
 #include "swift/AST/Expr.h"
-#include "swift/AST/InFlightSubstitution.h"
 #include "swift/AST/NameLookup.h"
 #include "swift/AST/NameLookupRequests.h"
 #include "swift/AST/Pattern.h"
@@ -3677,50 +3676,6 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
     }
   }
 
-  bool isSelfReferencing(const Candidate &candidate) {
-    auto substitutions = std::get<1>(candidate);
-
-    // The underlying type can't be defined recursively
-    // in terms of the opaque type itself.
-    for (auto genericParam : OpaqueDecl->getOpaqueGenericParams()) {
-      auto underlyingType = Type(genericParam).subst(substitutions);
-
-      // Look through underlying types of other opaque archetypes known to
-      // us. This is not something the type checker is allowed to do in
-      // general, since the intent is that the underlying type is completely
-      // hidden from view at the type system level. However, here we're
-      // trying to catch recursive underlying types before we proceed to
-      // SIL, so we specifically want to erase opaque archetypes just
-      // for the purpose of this check.
-      ReplaceOpaqueTypesWithUnderlyingTypes replacer(
-          OpaqueDecl->getDeclContext(),
-          ResilienceExpansion::Maximal,
-          /*isWholeModuleContext=*/false);
-      InFlightSubstitution IFS(replacer, replacer,
-                               SubstFlags::SubstituteOpaqueArchetypes |
-                               SubstFlags::PreservePackExpansionLevel);
-      auto simplifiedUnderlyingType = underlyingType.subst(IFS);
-
-      auto isSelfReferencing =
-          (IFS.wasLimitReached() ||
-           simplifiedUnderlyingType.findIf([&](Type t) -> bool {
-             if (auto *other = t->getAs<OpaqueTypeArchetypeType>()) {
-               return other->getDecl() == OpaqueDecl;
-             }
-             return false;
-           }));
-
-      if (isSelfReferencing) {
-        Ctx.Diags.diagnose(std::get<0>(candidate)->getLoc(),
-                           diag::opaque_type_self_referential_underlying_type,
-                           underlyingType);
-        return true;
-      }
-    }
-
-    return false;
-  }
-
   // A single unique underlying substitution.
   void finalizeUnique(const Candidate &candidate) {
     // If we have one successful candidate, then save it as the underlying
@@ -3819,11 +3774,6 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
       auto candidate =
           std::make_tuple(underlyingToOpaque->getSubExpr(), subMap, isUnique);
 
-      if (isSelfReferencing(candidate)) {
-        HasInvalidReturn = true;
-        return Action::Stop();
-      }
-
       if (subMap.getRecursiveProperties().hasDynamicSelf()) {
         Ctx.Diags.diagnose(E->getLoc(),
                            diag::opaque_type_cannot_contain_dynamic_self);

lib/Sema/TypeCheckGeneric.cpp

@@ -21,6 +21,7 @@
 #include "swift/AST/ASTWalker.h"
 #include "swift/AST/DiagnosticsSema.h"
 #include "swift/AST/ExistentialLayout.h"
+#include "swift/AST/InFlightSubstitution.h"
 #include "swift/AST/GenericEnvironment.h"
 #include "swift/AST/ParameterList.h"
 #include "swift/AST/ProtocolConformance.h"
@@ -267,6 +268,53 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
   return opaqueDecl;
 }
 
+void TypeChecker::checkCircularOpaqueReturnTypeDecl(OpaqueTypeDecl *opaqueDecl) {
+  auto optSubs = opaqueDecl->getUniqueUnderlyingTypeSubstitutions(
+      /*typeCheckFunctionBodies=*/false);
+  if (!optSubs)
+    return;
+
+  auto substitutions = *optSubs;
+
+  // The underlying type can't be defined recursively
+  // in terms of the opaque type itself.
+  for (auto genericParam : opaqueDecl->getOpaqueGenericParams()) {
+    auto underlyingType = Type(genericParam).subst(substitutions);
+
+    // Look through underlying types of other opaque archetypes known to
+    // us. This is not something the type checker is allowed to do in
+    // general, since the intent is that the underlying type is completely
+    // hidden from view at the type system level. However, here we're
+    // trying to catch recursive underlying types before we proceed to
+    // SIL, so we specifically want to erase opaque archetypes just
+    // for the purpose of this check.
+    ReplaceOpaqueTypesWithUnderlyingTypes replacer(
+        opaqueDecl->getDeclContext(),
+        ResilienceExpansion::Maximal,
+        /*isWholeModuleContext=*/false,
+        /*typeCheckFunctionBodies=*/false);
+    InFlightSubstitution IFS(replacer, replacer,
+                             SubstFlags::SubstituteOpaqueArchetypes |
+                             SubstFlags::PreservePackExpansionLevel);
+    auto simplifiedUnderlyingType = underlyingType.subst(IFS);
+
+    auto isSelfReferencing =
+        (IFS.wasLimitReached() ||
+         simplifiedUnderlyingType.findIf([&](Type t) -> bool {
+           if (auto *other = t->getAs<OpaqueTypeArchetypeType>()) {
+             return other->getDecl() == opaqueDecl;
+           }
+           return false;
+         }));
+
+    if (isSelfReferencing) {
+      opaqueDecl->getNamingDecl()->diagnose(
+          diag::opaque_type_self_referential_underlying_type,
+          underlyingType);
+    }
+  }
+}
+
 static bool checkProtocolSelfRequirementsImpl(
     ASTContext &ctx, ProtocolDecl *proto, ValueDecl *decl,
     GenericSignature originalSig,

lib/Sema/TypeChecker.cpp

@@ -326,6 +326,10 @@ TypeCheckPrimaryFileRequest::evaluate(Evaluator &eval, SourceFile *SF) const {
       }
     }
     SF->typeCheckDelayedFunctions();
+
+    for (auto *opaqueDecl : SF->getOpaqueReturnTypeDecls()) {
+      TypeChecker::checkCircularOpaqueReturnTypeDecl(opaqueDecl);
+    }
   }
 
   // If region-based isolation is enabled, we diagnose unnecessary

lib/Sema/TypeChecker.h

@@ -506,6 +506,8 @@ void typeCheckTopLevelCodeDecl(TopLevelCodeDecl *TLCD);
 
 void typeCheckDecl(Decl *D);
 
+void checkCircularOpaqueReturnTypeDecl(OpaqueTypeDecl *opaqueDecl);
+
 void addImplicitDynamicAttribute(Decl *D);
 void checkDeclAttributes(Decl *D);
 void checkDeclABIAttribute(Decl *apiDecl, ABIAttr *abiAttr);

test/Generics/infinite_opaque_result_type.swift

@@ -5,25 +5,25 @@ func concrete1() -> some Any {
   return concrete1()
 }
 
-func concrete2() -> some Any {
-  return [concrete2()]  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+func concrete2() -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+  return [concrete2()]
 }
 
 
-func concrete1a() -> some Any {
-  return concrete1b()  // expected-error {{function opaque return type was inferred as 'some Any', which defines the opaque type in terms of itself}}
+func concrete1a() -> some Any {  // expected-error {{function opaque return type was inferred as 'some Any', which defines the opaque type in terms of itself}}
+  return concrete1b()
 }
 
-func concrete1b() -> some Any {
+func concrete1b() -> some Any {  // expected-error {{function opaque return type was inferred as 'some Any', which defines the opaque type in terms of itself}}
   return concrete1a()
 }
 
 
-func concrete2a() -> some Any {
-  return [concrete2b()]  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+func concrete2a() -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+  return [concrete2b()]
 }
 
-func concrete2b() -> some Any {
+func concrete2b() -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
   return [concrete2a()]
 }
 
@@ -33,41 +33,41 @@ func generic1<T>(_ t: T) -> some Any {
   return generic1(t)
 }
 
-func generic2<T>(_ t: T) -> some Any {
-  return [generic2(t)]  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+func generic2<T>(_ t: T) -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+  return [generic2(t)]
 }
 
 
-func generic1a<T>(_ t: T) -> some Any {
-  return generic1b(t)  // expected-error {{function opaque return type was inferred as 'some Any', which defines the opaque type in terms of itself}}
+func generic1a<T>(_ t: T) -> some Any {  // expected-error {{function opaque return type was inferred as 'some Any', which defines the opaque type in terms of itself}}
+  return generic1b(t)
 }
 
-func generic1b<T>(_ t: T) -> some Any {
+func generic1b<T>(_ t: T) -> some Any {  // expected-error {{function opaque return type was inferred as 'some Any', which defines the opaque type in terms of itself}}
   return generic1a(t)
 }
 
 
-func generic2a<T>(_ t: T) -> some Any {
-  return [generic2b(t)]  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+func generic2a<T>(_ t: T) -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+  return [generic2b(t)]
 }
 
-func generic2b<T>(_ t: T) -> some Any {
+func generic2b<T>(_ t: T) -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
   return [generic2a(t)]
 }
 
 
-func generic3a<T>(_ t: T) -> some Any {
-  return [generic3b(t)]  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+func generic3a<T>(_ t: T) -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
+  return [generic3b(t)]
 }
 
-func generic3b<T>(_ t: T) -> some Any {
+func generic3b<T>(_ t: T) -> some Any {  // expected-error {{function opaque return type was inferred as '[some Any]', which defines the opaque type in terms of itself}}
   return [generic3a([t])]
 }
 
-func very_wide1() -> some Any {
-  return (very_wide2(), very_wide2())  // expected-error {{function opaque return type was inferred as '(some Any, some Any)', which defines the opaque type in terms of itself}}
+func very_wide1() -> some Any {  // expected-error {{function opaque return type was inferred as '(some Any, some Any)', which defines the opaque type in terms of itself}}
+  return (very_wide2(), very_wide2())
 }
 
-func very_wide2() -> some Any {
+func very_wide2() -> some Any {  // expected-error {{function opaque return type was inferred as '(some Any, some Any)', which defines the opaque type in terms of itself}}
   return (very_wide1(), very_wide1())
 }