Merge pull request #40715 from DougGregor/named-opaque-result-types

Named opaque result types

Author: DougGregor

include/swift/AST/Decl.h

@@ -106,6 +106,7 @@ namespace swift {
 
   namespace ast_scope {
   class AbstractPatternEntryScope;
+  class GenericParamScope;
   class PatternEntryDeclScope;
   class PatternEntryInitializerScope;
   } // namespace ast_scope
@@ -1565,6 +1566,7 @@ class PatternBindingEntry {
   friend class PatternBindingInitializer;
   friend class PatternBindingDecl;
   friend class ast_scope::AbstractPatternEntryScope;
+  friend class ast_scope::GenericParamScope;
   friend class ast_scope::PatternEntryDeclScope;
   friend class ast_scope::PatternEntryInitializerScope;
 
@@ -2779,6 +2781,16 @@ class OpaqueTypeDecl final :
     return OpaqueInterfaceGenericSignature.getInnermostGenericParams();
   }
 
+  /// Whether the generic parameters of this opaque type declaration were
+  /// explicit, i.e., for named opaque result types.
+  bool hasExplicitGenericParams() const;
+
+  /// When the generic parameters were explicit, returns the generic parameter
+  /// corresponding to the given ordinal.
+  ///
+  /// Otherwise, returns \c nullptr.
+  GenericTypeParamDecl *getExplicitGenericParam(unsigned ordinal) const;
+
   /// Retrieve the buffer containing the opaque return type
   /// representations that correspond to the opaque generic parameters.
   ArrayRef<OpaqueReturnTypeRepr *> getOpaqueReturnTypeReprs() const {

include/swift/AST/DiagnosticsSema.def

@@ -4134,6 +4134,9 @@ ERROR(opaque_type_no_underlying_type_candidates,none,
 ERROR(opaque_type_mismatched_underlying_type_candidates,none,
       "function declares an opaque return type %0, but the return statements "
       "in its body do not have matching underlying types", (TypeRepr *))
+ERROR(opaque_type_mismatched_underlying_type_candidates_named,none,
+      "function declares an opaque return type %0, but the return statements "
+      "in its body do not have matching underlying types", (Identifier))
 NOTE(opaque_type_underlying_type_candidate_here,none,
      "return statement has underlying type %0", (Type))
 ERROR(opaque_type_self_referential_underlying_type,none,

lib/AST/ASTMangler.cpp

@@ -1558,7 +1558,17 @@ unsigned ASTMangler::appendBoundGenericArgs(DeclContext *dc,
 
     // If we are generic at this level, emit all of the replacements at
     // this level.
-    if (genericContext->isGeneric()) {
+    bool treatAsGeneric;
+    if (auto opaque = dyn_cast<OpaqueTypeDecl>(decl)) {
+      // For opaque type declarations, the generic parameters of the opaque
+      // type declaration are not part of the mangling, so check whether the
+      // naming declaration has generic parameters.
+      auto namedGenericContext = opaque->getNamingDecl()->getAsGenericContext();
+      treatAsGeneric = namedGenericContext && namedGenericContext->isGeneric();
+    } else {
+      treatAsGeneric = genericContext->isGeneric();
+    }
+    if (treatAsGeneric) {
       auto genericParams = subs.getGenericSignature().getGenericParams();
       unsigned depth = genericParams[currentGenericParamIdx]->getDepth();
       auto replacements = subs.getReplacementTypes();

lib/AST/ASTPrinter.cpp

@@ -4338,9 +4338,10 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
       case PrintOptions::OpaqueReturnTypePrintingMode::Description:
         return true;
       case PrintOptions::OpaqueReturnTypePrintingMode::WithOpaqueKeyword:
-        return false;
+        return opaque->getDecl()->hasExplicitGenericParams();
       case PrintOptions::OpaqueReturnTypePrintingMode::WithoutOpaqueKeyword:
-        return isSimpleUnderPrintOptions(opaque->getExistentialType());
+        return opaque->getDecl()->hasExplicitGenericParams() ||
+            isSimpleUnderPrintOptions(opaque->getExistentialType());
       }
       llvm_unreachable("bad opaque-return-type printing mode");
     }
@@ -5410,11 +5411,28 @@ class TypePrinter : public TypeVisitor<TypePrinter> {
   }
 
   void visitOpaqueTypeArchetypeType(OpaqueTypeArchetypeType *T) {
+    // Try to print a named opaque type.
+    auto printNamedOpaque = [&] {
+      if (auto genericParam =
+              T->getDecl()->getExplicitGenericParam(T->getOrdinal())) {
+        visit(genericParam->getDeclaredInterfaceType());
+        return true;
+      }
+
+      return false;
+    };
+
     switch (Options.OpaqueReturnTypePrinting) {
     case PrintOptions::OpaqueReturnTypePrintingMode::WithOpaqueKeyword:
+      if (printNamedOpaque())
+        return;
+
       Printer.printKeyword("some", Options, /*Suffix=*/" ");
       LLVM_FALLTHROUGH;
     case PrintOptions::OpaqueReturnTypePrintingMode::WithoutOpaqueKeyword: {
+      if (printNamedOpaque())
+        return;
+
       visit(T->getExistentialType());
       return;
     }

lib/AST/ASTScopeCreation.cpp

@@ -708,6 +708,26 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
   // Initializers come before VarDecls, e.g. PCMacro/didSet.swift 19
   auto patternEntry = getPatternEntry();
 
+  // If the pattern type is for a named opaque result type, introduce the
+  // generic type parameters based on the first variable we find.
+  ASTScopeImpl *leaf = this;
+  auto pattern = patternEntry.getPattern();
+  if (auto typedPattern = dyn_cast<TypedPattern>(pattern)) {
+    if (auto namedOpaque =
+            dyn_cast_or_null<NamedOpaqueReturnTypeRepr>(
+              typedPattern->getTypeRepr())) {
+      bool addedOpaqueResultTypeScope = false;
+      pattern->forEachVariable([&](VarDecl *var) {
+        if (addedOpaqueResultTypeScope)
+          return;
+
+        leaf = scopeCreator.addNestedGenericParamScopesToTree(
+            var, namedOpaque->getGenericParams(), leaf);
+        addedOpaqueResultTypeScope = true;
+      });
+    }
+  }
+
   // Create a child for the initializer, if present.
   // Cannot trust the source range given in the ASTScopeImpl for the end of the
   // initializer (because of InterpolatedLiteralStrings and EditorPlaceHolders),
@@ -724,7 +744,7 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
         "Original inits are always after the '='");
     scopeCreator
         .constructExpandAndInsert<PatternEntryInitializerScope>(
-            this, decl, patternEntryIndex);
+            leaf, decl, patternEntryIndex);
   }
 
   // If this pattern binding entry was created by the debugger, it will always
@@ -741,12 +761,12 @@ PatternEntryDeclScope::expandAScopeThatCreatesANewInsertionPoint(
         "inits are always after the '='");
     scopeCreator
         .constructExpandAndInsert<PatternEntryInitializerScope>(
-            this, decl, patternEntryIndex);
+            leaf, decl, patternEntryIndex);
   }
 
   // Add accessors for the variables in this pattern.
-  patternEntry.getPattern()->forEachVariable([&](VarDecl *var) {
-    scopeCreator.addChildrenForParsedAccessors(var, this);
+  pattern->forEachVariable([&](VarDecl *var) {
+    scopeCreator.addChildrenForParsedAccessors(var, leaf);
   });
 
   // In local context, the PatternEntryDeclScope becomes the insertion point, so
@@ -849,6 +869,20 @@ TopLevelCodeScope::expandAScopeThatCreatesANewInsertionPoint(ScopeCreator &
 
 // Create child scopes for every declaration in a body.
 
+namespace {
+  /// Retrieve the opaque generic parameter list if present, otherwise the normal generic parameter list.
+  template<typename T>
+  GenericParamList *getPotentiallyOpaqueGenericParams(T *decl) {
+    if (auto opaqueRepr = decl->getOpaqueResultTypeRepr()) {
+      if (auto namedOpaque = dyn_cast<NamedOpaqueReturnTypeRepr>(opaqueRepr)) {
+        return namedOpaque->getGenericParams();
+      }
+    }
+
+    return decl->getGenericParams();
+  }
+}
+
 void AbstractFunctionDeclScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
     ScopeCreator &scopeCreator) {
   scopeCreator.addChildrenForKnownAttributes(decl, this);
@@ -860,7 +894,7 @@ void AbstractFunctionDeclScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
 
   if (!isa<AccessorDecl>(decl)) {
     leaf = scopeCreator.addNestedGenericParamScopesToTree(
-        decl, decl->getGenericParams(), leaf);
+        decl, getPotentiallyOpaqueGenericParams(decl), leaf);
 
     auto *params = decl->getParameters();
     if (params->size() > 0) {
@@ -1008,7 +1042,7 @@ void SubscriptDeclScope::expandAScopeThatDoesNotCreateANewInsertionPoint(
     ScopeCreator &scopeCreator) {
   scopeCreator.addChildrenForKnownAttributes(decl, this);
   auto *leaf = scopeCreator.addNestedGenericParamScopesToTree(
-      decl, decl->getGenericParams(), this);
+      decl, getPotentiallyOpaqueGenericParams(decl), this);
   scopeCreator.constructExpandAndInsert<ParameterListScope>(
       leaf, decl->getIndices(), decl->getAccessor(AccessorKind::Get));
   scopeCreator.addChildrenForParsedAccessors(decl, leaf);

lib/AST/ASTScopeLookup.cpp

@@ -266,6 +266,14 @@ bool GenericTypeOrExtensionScope::areMembersVisibleFromWhereClause() const {
 NullablePtr<const ASTScopeImpl>
 PatternEntryInitializerScope::getLookupParent() const {
   auto parent = getParent().get();
+
+  // Skip generic parameter scopes, which occur here due to named opaque
+  // result types.
+  // FIXME: Proper isa/dyn_cast support would be better than a string
+  // comparison here.
+  while (parent->getClassName() == "GenericParamScope")
+    parent = parent->getLookupParent().get();
+
   ASTScopeAssert(parent->getClassName() == "PatternEntryDeclScope",
                  "PatternEntryInitializerScope in unexpected place");
 

lib/AST/ASTScopeSourceRange.cpp

@@ -161,6 +161,15 @@ SourceRange GenericParamScope::getSourceRangeOfThisASTNode(
     return SourceRange(getLocAfterExtendedNominal(ext), ext->getEndLoc());
   }
 
+  // For a variable, the generic parameter is visible throughout the pattern
+  // binding entry.
+  if (auto var = dyn_cast<VarDecl>(holder)) {
+    if (auto patternBinding = var->getParentPatternBinding()) {
+      unsigned index = patternBinding->getPatternEntryIndexForVarDecl(var);
+      return patternBinding->getPatternList()[index].getSourceRange();
+    }
+  }
+
   // For all other declarations, generic parameters are visible everywhere.
   return holder->getSourceRange();
 }

lib/AST/Decl.cpp

@@ -7829,6 +7829,19 @@ bool OpaqueTypeDecl::isOpaqueReturnTypeOfFunction(
   return false;
 }
 
+bool OpaqueTypeDecl::hasExplicitGenericParams() const {
+  return getExplicitGenericParam(0) != nullptr;
+}
+
+GenericTypeParamDecl *OpaqueTypeDecl::getExplicitGenericParam(
+    unsigned ordinal) const {
+  if (ordinal >= getOpaqueGenericParams().size())
+    return nullptr;
+
+  auto genericParamType = getOpaqueGenericParams()[ordinal];
+  return genericParamType->getDecl();
+}
+
 unsigned OpaqueTypeDecl::getAnonymousOpaqueParamOrdinal(
     OpaqueReturnTypeRepr *repr) const {
   assert(NamingDeclAndHasOpaqueReturnTypeRepr.getInt() &&

lib/Sema/ConstraintSystem.cpp

@@ -846,7 +846,7 @@ Type ConstraintSystem::openOpaqueType(OpaqueTypeArchetypeType *opaque,
   if (knownReplacements != OpenedTypes.end()) {
     auto param = opaque->getInterfaceType()->castTo<GenericTypeParamType>();
     for (const auto &replacement : knownReplacements->second) {
-      if (replacement.first == param)
+      if (replacement.first->isEqual(param))
         return replacement.second;
     }
 

lib/Sema/MiscDiagnostics.cpp

@@ -2610,6 +2610,7 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
   OpaqueTypeDecl *OpaqueDecl;
   BraceStmt *Body;
   SmallVector<Candidate, 4> Candidates;
+  SmallPtrSet<const void *, 4> KnownCandidates;
 
   bool HasInvalidReturn = false;
 
@@ -2644,13 +2645,7 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
     // Check whether all of the underlying type candidates match up.
     // TODO [OPAQUE SUPPORT]: diagnose multiple opaque types
     SubstitutionMap underlyingSubs = Candidates.front().second;
-    SubstitutionMap canonicalUnderlyingSubs = underlyingSubs.getCanonical();
-    bool mismatch =
-        std::any_of(Candidates.begin() + 1, Candidates.end(),
-                    [&](Candidate &otherCandidate) {
-                      return canonicalUnderlyingSubs != otherCandidate.second.getCanonical();
-                    });
-    if (mismatch) {
+    if (Candidates.size() > 1) {
       unsigned mismatchIndex = OpaqueDecl->getOpaqueGenericParams().size();
       for (auto genericParam : OpaqueDecl->getOpaqueGenericParams()) {
         unsigned index = genericParam->getIndex();
@@ -2669,12 +2664,21 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
           break;
       }
       assert(mismatchIndex < OpaqueDecl->getOpaqueGenericParams().size());
-      TypeRepr *opaqueRepr =
-          OpaqueDecl->getOpaqueReturnTypeReprs()[mismatchIndex];
-      Implementation->diagnose(
-          diag::opaque_type_mismatched_underlying_type_candidates,
-          opaqueRepr)
-        .highlight(opaqueRepr->getSourceRange());
+
+      if (auto genericParam =
+              OpaqueDecl->getExplicitGenericParam(mismatchIndex)) {
+        Implementation->diagnose(
+            diag::opaque_type_mismatched_underlying_type_candidates_named,
+            genericParam->getName())
+          .highlight(genericParam->getLoc());
+      } else {
+        TypeRepr *opaqueRepr =
+            OpaqueDecl->getOpaqueReturnTypeReprs()[mismatchIndex];
+        Implementation->diagnose(
+            diag::opaque_type_mismatched_underlying_type_candidates,
+            opaqueRepr)
+          .highlight(opaqueRepr->getSourceRange());
+      }
 
       for (auto candidate : Candidates) {
         Ctx.Diags.diagnose(
@@ -2712,8 +2716,12 @@ class OpaqueUnderlyingTypeChecker : public ASTWalker {
 
   std::pair<bool, Expr *> walkToExprPre(Expr *E) override {
     if (auto underlyingToOpaque = dyn_cast<UnderlyingToOpaqueExpr>(E)) {
-      Candidates.push_back(std::make_pair(underlyingToOpaque->getSubExpr(),
-                                          underlyingToOpaque->substitutions));
+      auto key =
+          underlyingToOpaque->substitutions.getCanonical().getOpaqueValue();
+      if (KnownCandidates.insert(key).second) {
+        Candidates.push_back(std::make_pair(underlyingToOpaque->getSubExpr(),
+                                            underlyingToOpaque->substitutions));
+      }
       return {false, E};
     }
     return {true, E};