[NameLookup] Opaque Type Collector should skip existential types

lib/AST/ASTScopeCreation.cpp

@@ -928,7 +928,7 @@ namespace {
       }
     }
 
-    return decl->getGenericParams();
+    return decl->getParsedGenericParams();
   }
 }
 

lib/AST/NameLookup.cpp

@@ -2798,7 +2798,6 @@ bool TypeRepr::isProtocol(DeclContext *dc){
   auto &ctx = dc->getASTContext();
   return findIf([&ctx, dc](TypeRepr *ty) {
     return declsAreProtocols(directReferencesForTypeRepr(ctx.evaluator, ctx, ty, dc));
-
   });
 }
 
@@ -2847,20 +2846,13 @@ CollectedOpaqueReprs swift::collectOpaqueReturnTypeReprs(TypeRepr *r, ASTContext
         return Action::Continue();
 
       if (auto existential = dyn_cast<ExistentialTypeRepr>(repr)) {
-        auto meta = dyn_cast<MetatypeTypeRepr>(existential->getConstraint());
-        auto generic = dyn_cast<GenericIdentTypeRepr>(existential->getConstraint());
-        if(generic)
-          Reprs.push_back(existential);
-        return Action::VisitChildrenIf(meta || generic);
+        return Action::SkipChildren();
       } else if (auto compositionRepr = dyn_cast<CompositionTypeRepr>(repr)) {
         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();
-        }
+        return Action::Continue();
       } 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

@@ -79,6 +79,7 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
         .diagnose(repr->getLoc(), diag::opaque_type_in_protocol_requirement)
         .fixItInsert(fixitLoc, result)
         .fixItReplace(repr->getSourceRange(), placeholder);
+    repr->setInvalid();
 
     return nullptr;
   }
@@ -136,6 +137,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) {
@@ -157,6 +163,7 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
              .diagnose(currentRepr->getLoc(), diag::opaque_of_optional_rewrite)
              .fixItReplaceChars(currentRepr->getStartLoc(),
                                 currentRepr->getEndLoc(), stream.str());
+          repr->setInvalid();
           return nullptr;
         }
       }
@@ -194,6 +201,7 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
         // Error out if the constraint type isn't a class or existential type.
         ctx.Diags.diagnose(currentRepr->getLoc(),
                            diag::opaque_type_invalid_constraint);
+        currentRepr->setInvalid();
         return nullptr;
       }
 
@@ -241,6 +249,7 @@ OpaqueResultTypeRequest::evaluate(Evaluator &evaluator,
         ctx.Diags.diagnose(repr->getLoc(),
                            diag::opaque_type_in_parameter,
                            false, interfaceType);
+        repr->setInvalid();
         return true;
       }
     }
@@ -469,8 +478,15 @@ void TypeChecker::checkReferencedGenericParams(GenericContext *dc) {
           continue;
       }
       // Produce an error that this generic parameter cannot be bound.
-      paramDecl->diagnose(diag::unreferenced_generic_parameter,
-                          paramDecl->getNameStr());
+      if (paramDecl->isImplicit()) {
+        paramDecl->getASTContext().Diags
+          .diagnose(paramDecl->getOpaqueTypeRepr()->getLoc(),
+                    diag::unreferenced_generic_parameter,
+                    paramDecl->getNameStr());
+      } else {
+        paramDecl->diagnose(diag::unreferenced_generic_parameter,
+                            paramDecl->getNameStr());
+      }
     }
   }
 }

lib/Sema/TypeCheckPattern.cpp

@@ -732,7 +732,7 @@ validateTypedPattern(TypedPattern *TP, DeclContext *dc,
   auto *Repr = TP->getTypeRepr();
   if (Repr && (Repr->hasOpaque() ||
                (Context.LangOpts.hasFeature(Feature::ImplicitSome) &&
-                Repr->isProtocol(dc)))) {
+                !collectOpaqueReturnTypeReprs(Repr, Context, dc).empty()))) {
     auto named = dyn_cast<NamedPattern>(
         TP->getSubPattern()->getSemanticsProvidingPattern());
     if (!named) {

lib/Sema/TypeCheckType.cpp

@@ -360,7 +360,7 @@ TypeChecker::getDynamicBridgedThroughObjCClass(DeclContext *dc,
 }
 
 /// Retrieve the identity form of the opaque type archetype type.
-static Type getIdentityOpaqueTypeArchetypeType(
+static Type getOpaqueArchetypeIdentity(
     OpaqueTypeDecl *opaqueDecl, unsigned ordinal) {
   auto outerGenericSignature = opaqueDecl->getNamingDecl()
                                    ->getInnermostDeclContext()
@@ -418,8 +418,7 @@ Type TypeResolution::resolveTypeInContext(TypeDecl *typeDecl,
     if (auto opaqueDecl = dyn_cast<OpaqueTypeDecl>(getDeclContext())) {
       if (genericParam->getDepth() ==
               opaqueDecl->getOpaqueGenericParams().front()->getDepth()) {
-        return getIdentityOpaqueTypeArchetypeType(
-            opaqueDecl, genericParam->getIndex());
+        return getOpaqueArchetypeIdentity(opaqueDecl, genericParam->getIndex());
       }
     }
 
@@ -1990,8 +1989,11 @@ namespace {
       return diags.diagnose(std::forward<ArgTypes>(Args)...);
     }
 
-    Type diagnoseDisallowedExistential(TypeRepr *repr, Type type);
     bool diagnoseMoveOnly(TypeRepr *repr, Type genericArgTy);
+
+    bool diagnoseDisallowedExistential(TypeRepr *repr);
+
+    bool diagnoseInvalidPlaceHolder(OpaqueReturnTypeRepr *repr);
 
     NeverNullType resolveOpenedExistentialArchetype(
         TypeAttributes &attrs, TypeRepr *repr,
@@ -2205,7 +2207,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 +2219,18 @@ 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;
+bool TypeResolver::diagnoseInvalidPlaceHolder(OpaqueReturnTypeRepr *repr) {
+  if (repr->getConstraint()->isInvalid()){
+    if (isa<PlaceholderTypeRepr>(repr->getConstraint()))
+      return true;
+  }
+  return false;
 }
 
 /// Checks the given type, assuming that it appears as an argument for a
@@ -2336,9 +2347,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 getOpaqueArchetypeIdentity(opaqueDecl, *ordinal);
+          }
         }
       }
 
@@ -2358,10 +2370,15 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
     // evaluation of an `OpaqueResultTypeRequest`.
     auto opaqueRepr = cast<OpaqueReturnTypeRepr>(repr);
     auto *DC = getDeclContext();
+
+    bool isInExistential = diagnoseDisallowedExistential(opaqueRepr);
+    bool hasInvalidPlaceholder = diagnoseInvalidPlaceHolder(opaqueRepr);
+
     if (auto opaqueDecl = dyn_cast<OpaqueTypeDecl>(DC)) {
-      if (auto ordinal = opaqueDecl->getAnonymousOpaqueParamOrdinal(opaqueRepr))
-        return diagnoseDisallowedExistential(opaqueRepr,
-            getIdentityOpaqueTypeArchetypeType(opaqueDecl, *ordinal));
+      if (auto ordinal = opaqueDecl->getAnonymousOpaqueParamOrdinal(opaqueRepr)){
+        return !isInExistential ? getOpaqueArchetypeIdentity(opaqueDecl, *ordinal)
+                                : ErrorType::get(getASTContext());
+      }
     }
 
     // Check whether any of the generic parameters in the context represents
@@ -2371,19 +2388,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 (!repr->isInvalid() && !hasInvalidPlaceholder){
+      // 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.
     options |= TypeResolutionFlags::SilenceErrors;
     auto constraintType = resolveType(opaqueRepr->getConstraint(),
@@ -2447,9 +2463,10 @@ NeverNullType TypeResolver::resolveType(TypeRepr *repr,
         return ty;
 
     // Complain if we're allowed to and bail out with an error.
-    if (!options.contains(TypeResolutionFlags::SilenceErrors))
+    if (!options.contains(TypeResolutionFlags::SilenceErrors)) {
       ctx.Diags.diagnose(repr->getLoc(),
                          diag::placeholder_type_not_allowed);
+    }
 
     return ErrorType::get(resolution.getASTContext());
   }
@@ -4074,8 +4091,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 getOpaqueArchetypeIdentity(opaqueDecl, *ordinal);
         }
       }
 
@@ -4706,6 +4723,7 @@ TypeResolver::resolveExistentialType(ExistentialTypeRepr *repr,
   if (constraintType->hasError())
     return ErrorType::get(getASTContext());
 
+  //TO-DO: generalize this and emit the same erorr for some P?
   if (!constraintType->isConstraintType()) {
     // Emit a tailored diagnostic for the incorrect optional
     // syntax 'any P?' with a fix-it to add parenthesis.