[Sema] Diagnose DisallowedExistential in type resolution

Author: angela-laar

lib/AST/NameLookup.cpp

@@ -2852,11 +2852,6 @@ CollectedOpaqueReprs swift::collectOpaqueReturnTypeReprs(TypeRepr *r, ASTContext
         if (!compositionRepr->isTypeReprAny())
           Reprs.push_back(compositionRepr);
         return Action::SkipChildren();
-      } else if (auto generic = dyn_cast<GenericIdentTypeRepr>(repr)) {
-        // prevent any P<some P>
-        if (!Reprs.empty() && isa<ExistentialTypeRepr>(Reprs.front())){
-          Reprs.clear();
-        }
       } else if (auto declRefTR = dyn_cast<DeclRefTypeRepr>(repr)) {
         if (declRefTR->isProtocol(dc))
           Reprs.push_back(declRefTR);

lib/Sema/TypeCheckDecl.cpp

@@ -2137,11 +2137,8 @@ ResultTypeRequest::evaluate(Evaluator &evaluator, ValueDecl *decl) const {
     return TupleType::getEmpty(ctx);
 
   // Handle opaque types.
-  if (decl->getOpaqueResultTypeRepr()) {
-    auto *opaqueDecl = decl->getOpaqueResultTypeDecl();
-    return (opaqueDecl
-            ? opaqueDecl->getDeclaredInterfaceType()
-            : ErrorType::get(ctx));
+  if (auto *opaqueDecl = decl->getOpaqueResultTypeDecl()) {
+      return opaqueDecl->getDeclaredInterfaceType();
   }
 
   auto options =

lib/Sema/TypeCheckGeneric.cpp

@@ -136,6 +136,11 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
     }
   } else {
     opaqueReprs = collectOpaqueReturnTypeReprs(repr, ctx, dc);
+    
+    if (opaqueReprs.empty()) {
+      return nullptr;
+    }
+
     SmallVector<GenericTypeParamType *, 2> genericParamTypes;
     SmallVector<Requirement, 2> requirements;
     for (unsigned i = 0; i < opaqueReprs.size(); ++i) {

lib/Sema/TypeCheckType.cpp

@@ -1990,8 +1990,9 @@ namespace {
       return diags.diagnose(std::forward<ArgTypes>(Args)...);
     }
 
-    Type diagnoseDisallowedExistential(TypeRepr *repr, Type type);
     bool diagnoseMoveOnly(TypeRepr *repr, Type genericArgTy);
+    
+    bool diagnoseDisallowedExistential(TypeRepr *repr);
 
     NeverNullType resolveOpenedExistentialArchetype(
         TypeAttributes &attrs, TypeRepr *repr,
@@ -2205,7 +2206,7 @@ static Type evaluateTypeResolution(const TypeResolution *resolution,
   return result;
 }
 
-Type TypeResolver::diagnoseDisallowedExistential(TypeRepr *repr, Type type) {
+bool TypeResolver::diagnoseDisallowedExistential(TypeRepr *repr) {
   auto options = resolution.getOptions();
   if (!(options & TypeResolutionFlags::SilenceErrors) &&
       options.contains(TypeResolutionFlags::DisallowOpaqueTypes)) {
@@ -2217,9 +2218,10 @@ Type TypeResolver::diagnoseDisallowedExistential(TypeRepr *repr, Type type) {
     // FIXME: We shouldn't have to invalid the type repr here, but not
     // doing so causes a double-diagnostic.
     repr->setInvalid();
+    return true;
+  } else {
+    return false;
   }
-
-  return type;
 }
 
 /// Checks the given type, assuming that it appears as an argument for a
@@ -2336,9 +2338,10 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
       auto *DC = getDeclContext();
       if (getASTContext().LangOpts.hasFeature(Feature::ImplicitSome)) {
         if (auto opaqueDecl = dyn_cast<OpaqueTypeDecl>(DC)) {
-          if (auto ordinal = opaqueDecl->getAnonymousOpaqueParamOrdinal(repr))
-            return diagnoseDisallowedExistential(repr,
-                getIdentityOpaqueTypeArchetypeType(opaqueDecl, *ordinal));
+          if (auto ordinal = opaqueDecl->getAnonymousOpaqueParamOrdinal(repr)){
+            diagnoseDisallowedExistential(repr);
+            return getIdentityOpaqueTypeArchetypeType(opaqueDecl, *ordinal);
+          }
         }
       }
 
@@ -2358,10 +2361,12 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
     // evaluation of an `OpaqueResultTypeRequest`.
     auto opaqueRepr = cast<OpaqueReturnTypeRepr>(repr);
     auto *DC = getDeclContext();
+    
+    bool isInExistential = diagnoseDisallowedExistential(opaqueRepr);
+    
     if (auto opaqueDecl = dyn_cast<OpaqueTypeDecl>(DC)) {
       if (auto ordinal = opaqueDecl->getAnonymousOpaqueParamOrdinal(opaqueRepr))
-        return diagnoseDisallowedExistential(opaqueRepr,
-            getIdentityOpaqueTypeArchetypeType(opaqueDecl, *ordinal));
+        return getIdentityOpaqueTypeArchetypeType(opaqueDecl, *ordinal);
     }
 
     // Check whether any of the generic parameters in the context represents
@@ -2371,17 +2376,18 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
         if (auto genericParams = genericContext->getGenericParams()) {
           for (auto genericParam : *genericParams) {
             if (genericParam->getOpaqueTypeRepr() == opaqueRepr)
-              return diagnoseDisallowedExistential(opaqueRepr,
-                  genericParam->getDeclaredInterfaceType());
+              return genericParam->getDeclaredInterfaceType();
           }
         }
       }
     }
-
-    // We are not inside an `OpaqueTypeDecl`, so diagnose an error.
-    if (!(options & TypeResolutionFlags::SilenceErrors)) {
-      diagnose(opaqueRepr->getOpaqueLoc(),
-               diag::unsupported_opaque_type);
+    
+    if (!isInExistential){
+      // We are not inside an `OpaqueTypeDecl`, so diagnose an error.
+      if (!(options & TypeResolutionFlags::SilenceErrors)) {
+        diagnose(opaqueRepr->getOpaqueLoc(),
+                 diag::unsupported_opaque_type);
+      }
     }
 
     // Try to resolve the constraint upper bound type as a placeholder.
@@ -4074,8 +4080,8 @@ TypeResolver::resolveDeclRefTypeRepr(DeclRefTypeRepr *repr,
       // Check whether this type is an implicit opaque result type.
       if (auto *opaqueDecl = dyn_cast<OpaqueTypeDecl>(getDeclContext())) {
         if (auto ordinal = opaqueDecl->getAnonymousOpaqueParamOrdinal(repr)) {
-          return diagnoseDisallowedExistential(
-              repr, getIdentityOpaqueTypeArchetypeType(opaqueDecl, *ordinal));
+          diagnoseDisallowedExistential(repr);
+          return getIdentityOpaqueTypeArchetypeType(opaqueDecl, *ordinal);
         }
       }
 

test/type/opaque_parameterized_existential.swift

@@ -9,39 +9,24 @@ protocol P<T> {
   associatedtype T
 }
 
-extension Never: P { typealias T = Never }
+struct S: P {
+  typealias T = Self
+}
 
 // I do not like them written clear
-func test() -> any P<some P> { fatalError() } // expected-error {{'some' types cannot be used in constraints on existential types}}
-// expected-error@-1{{cannot infer underlying type for opaque result 'any P<some P>' from return expression}}
-
-// I do not like them nested here
-func test() -> any P<[some P]> { fatalError() } // expected-error {{'some' types cannot be used in constraints on existential types}}
-// expected-error@-1{{cannot infer underlying type for opaque result 'any P<[some P]>' from return expression}}
-
+func test() -> any P<some P> { S() } // expected-error {{'some' types cannot be used in constraints on existential types}}
 // I do not like them under questions
-func test() -> any P<(some P)??> { fatalError() } // expected-error {{'some' types cannot be used in constraints on existential types}}
-// expected-error@-1{{cannot infer underlying type for opaque result 'any P<(some P)??>' from return expression}}
-
+func test() -> any P<(some P)??> { S() } // expected-error {{'some' types cannot be used in constraints on existential types}}
 // I do not like meta-type intentions
-func test() -> (any P<some P>).Type { fatalError() } // expected-error {{'some' types cannot be used in constraints on existential types}}
-// expected-error@-1{{cannot infer underlying type for opaque result '(any P<some P>).Type' from return expression}}
-
+func test() -> (any P<some P>).Type { S() } // expected-error {{'some' types cannot be used in constraints on existential types}}
 // I do not like them (meta)static-ly
-func test() -> any P<some P>.Type { fatalError() } // expected-error {{'some' types cannot be used in constraints on existential types}}
-// expected-error@-1{{cannot infer underlying type for opaque result 'any P<some P>.Type' from return expression}}
-
+func test() -> any P<some P>.Type { S() } // expected-error {{'some' types cannot be used in constraints on existential types}}
 // I do not like them tupled-three
-func test() -> (Int, any P<some P>, Int) { fatalError() } // expected-error {{'some' types cannot be used in constraints on existential types}}
-// expected-error@-1{{cannot infer underlying type for opaque result '(Int, any P<some P>, Int)' from return expression}}
-
+func test() -> (Int, any P<some P>, Int) { S() } // expected-error {{'some' types cannot be used in constraints on existential types}}
 // I do not like them in generics
 struct Wrapper<T> {}
-func test() -> any P<Wrapper<some P>> { fatalError() } // expected-error {{'some' types cannot be used in constraints on existential types}}
-// expected-error@-1{{cannot infer underlying type for opaque result 'any P<Wrapper<some P>>' from return expression}}
-
+func test() -> any P<Wrapper<some P>> { S() } // expected-error {{'some' types cannot be used in constraints on existential types}}
 // Your attempts to nest them put me in hysterics.
-func test(_ x: any P<some P>) {} // expected-error {{'some' types cannot be used in constraints on existential types}}
-
+func test(_ x: any P<some P>) { } // expected-error {{'some' types cannot be used in constraints on existential types}}
 // No, I do not like nested some type params,
 // I do not like them Σam-i-am