AST: Lazier opaque type validation

Author: slavapestov

include/swift/AST/ASTDemangler.h

@@ -58,9 +58,6 @@ class ASTBuilder {
   using BuiltTypeDecl = swift::GenericTypeDecl *; // nominal or type alias
   using BuiltProtocolDecl = swift::ProtocolDecl *;
   explicit ASTBuilder(ASTContext &ctx) : Ctx(ctx) {}
-  
-  /// The resolver to use for type checking, if necessary.
-  LazyResolver *Resolver = nullptr;
 
   ASTContext &getASTContext() { return Ctx; }
   DeclContext *getNotionalDC();

include/swift/AST/FileUnit.h

@@ -60,8 +60,7 @@ class FileUnit : public DeclContext {
 
   /// Look up an opaque return type by the mangled name of the declaration
   /// that defines it.
-  virtual OpaqueTypeDecl *lookupOpaqueResultType(StringRef MangledName,
-                                                 LazyResolver *resolver) {
+  virtual OpaqueTypeDecl *lookupOpaqueResultType(StringRef MangledName) {
     return nullptr;
   }
 

include/swift/AST/Module.h

@@ -354,8 +354,7 @@ class ModuleDecl : public DeclContext, public TypeDecl {
 
   /// Look up an opaque return type by the mangled name of the declaration
   /// that defines it.
-  OpaqueTypeDecl *lookupOpaqueResultType(StringRef MangledName,
-                                         LazyResolver *resolver);
+  OpaqueTypeDecl *lookupOpaqueResultType(StringRef MangledName);
 
   /// Find ValueDecls in the module and pass them to the given consumer object.
   ///

include/swift/AST/SourceFile.h

@@ -117,6 +117,13 @@ class SourceFile final : public FileUnit {
   /// The scope map that describes this source file.
   std::unique_ptr<ASTScope> Scope;
 
+  /// The set of validated opaque return type decls in the source file.
+  llvm::SmallVector<OpaqueTypeDecl *, 4> OpaqueReturnTypes;
+  llvm::StringMap<OpaqueTypeDecl *> ValidatedOpaqueReturnTypes;
+  /// The set of parsed decls with opaque return types that have not yet
+  /// been validated.
+  llvm::SetVector<ValueDecl *> UnvalidatedDeclsWithOpaqueReturnTypes;
+
   friend ASTContext;
   friend Impl;
 public:
@@ -130,13 +137,6 @@ class SourceFile final : public FileUnit {
   /// The list of local type declarations in the source file.
   llvm::SetVector<TypeDecl *> LocalTypeDecls;
 
-  /// The set of validated opaque return type decls in the source file.
-  llvm::SmallVector<OpaqueTypeDecl *, 4> OpaqueReturnTypes;
-  llvm::StringMap<OpaqueTypeDecl *> ValidatedOpaqueReturnTypes;
-  /// The set of parsed decls with opaque return types that have not yet
-  /// been validated.
-  llvm::DenseSet<ValueDecl *> UnvalidatedDeclsWithOpaqueReturnTypes;
-
   /// A set of special declaration attributes which require the
   /// Foundation module to be imported to work. If the foundation
   /// module is still not imported by the time type checking is
@@ -430,14 +430,13 @@ class SourceFile final : public FileUnit {
   void setSyntaxRoot(syntax::SourceFileSyntax &&Root);
   bool hasSyntaxRoot() const;
 
-  OpaqueTypeDecl *lookupOpaqueResultType(StringRef MangledName,
-                                         LazyResolver *resolver) override;
+  OpaqueTypeDecl *lookupOpaqueResultType(StringRef MangledName) override;
 
   void addUnvalidatedDeclWithOpaqueResultType(ValueDecl *vd) {
     UnvalidatedDeclsWithOpaqueReturnTypes.insert(vd);
   }
 
-  void markDeclWithOpaqueResultTypeAsValidated(ValueDecl *vd);
+  ArrayRef<OpaqueTypeDecl *> getOpaqueReturnTypeDecls();
 
 private:
 

include/swift/Serialization/SerializedModuleLoader.h

@@ -278,7 +278,7 @@ class SerializedASTFile final : public LoadedFile {
   virtual TypeDecl *lookupLocalType(StringRef MangledName) const override;
 
   virtual OpaqueTypeDecl *
-  lookupOpaqueResultType(StringRef MangledName, LazyResolver *resolver) override;
+  lookupOpaqueResultType(StringRef MangledName) override;
 
   virtual TypeDecl *
   lookupNestedType(Identifier name,

lib/AST/ASTDemangler.cpp

@@ -259,8 +259,7 @@ Type ASTBuilder::resolveOpaqueType(NodePointer opaqueDescriptor,
     if (!parentModule)
       return Type();
 
-    auto opaqueDecl = parentModule->lookupOpaqueResultType(mangledName,
-                                                           Resolver);
+    auto opaqueDecl = parentModule->lookupOpaqueResultType(mangledName);
     if (!opaqueDecl)
       return Type();
     // TODO: multiple opaque types

lib/AST/Module.cpp

@@ -475,10 +475,9 @@ TypeDecl * ModuleDecl::lookupLocalType(StringRef MangledName) const {
 }
 
 OpaqueTypeDecl *
-ModuleDecl::lookupOpaqueResultType(StringRef MangledName,
-                                   LazyResolver *resolver) {
+ModuleDecl::lookupOpaqueResultType(StringRef MangledName) {
   for (auto file : getFiles()) {
-    auto OTD = file->lookupOpaqueResultType(MangledName, resolver);
+    auto OTD = file->lookupOpaqueResultType(MangledName);
     if (OTD)
       return OTD;
   }
@@ -670,9 +669,8 @@ void SourceFile::getLocalTypeDecls(SmallVectorImpl<TypeDecl*> &Results) const {
 void
 SourceFile::getOpaqueReturnTypeDecls(SmallVectorImpl<OpaqueTypeDecl*> &Results)
 const {
-  for (auto &member : ValidatedOpaqueReturnTypes) {
-    Results.push_back(member.second);
-  }
+  auto result = const_cast<SourceFile *>(this)->getOpaqueReturnTypeDecls();
+  llvm::copy(result, std::back_inserter(Results));
 }
 
 TypeDecl *SourceFile::lookupLocalType(llvm::StringRef mangledName) const {
@@ -1806,44 +1804,41 @@ void SourceFile::createReferencedNameTracker() {
   ReferencedNames.emplace(ReferencedNameTracker());
 }
 
+ArrayRef<OpaqueTypeDecl *> SourceFile::getOpaqueReturnTypeDecls() {
+  for (auto *vd : UnvalidatedDeclsWithOpaqueReturnTypes) {
+    if (auto opaqueDecl = vd->getOpaqueResultTypeDecl()) {
+      auto inserted = ValidatedOpaqueReturnTypes.insert(
+                {opaqueDecl->getOpaqueReturnTypeIdentifier().str(),
+                 opaqueDecl});
+      if (inserted.second) {
+        OpaqueReturnTypes.push_back(opaqueDecl);
+      }
+    }
+  }
+
+  UnvalidatedDeclsWithOpaqueReturnTypes.clear();
+  return OpaqueReturnTypes;
+}
+
 OpaqueTypeDecl *
-SourceFile::lookupOpaqueResultType(StringRef MangledName,
-                                   LazyResolver *resolver) {
+SourceFile::lookupOpaqueResultType(StringRef MangledName) {
   // Check already-validated decls.
   auto found = ValidatedOpaqueReturnTypes.find(MangledName);
   if (found != ValidatedOpaqueReturnTypes.end())
     return found->second;
 
   // If there are unvalidated decls with opaque types, go through and validate
   // them now.
-  if (resolver && !UnvalidatedDeclsWithOpaqueReturnTypes.empty()) {
-    while (!UnvalidatedDeclsWithOpaqueReturnTypes.empty()) {
-      ValueDecl *decl = *UnvalidatedDeclsWithOpaqueReturnTypes.begin();
-      UnvalidatedDeclsWithOpaqueReturnTypes.erase(decl);
-      // FIXME(InterfaceTypeRequest): Remove this.
-      (void)decl->getInterfaceType();
-    }
-    
-    found = ValidatedOpaqueReturnTypes.find(MangledName);
-    if (found != ValidatedOpaqueReturnTypes.end())
-      return found->second;
-  }
+  (void) getOpaqueReturnTypeDecls();
+
+  found = ValidatedOpaqueReturnTypes.find(MangledName);
+  if (found != ValidatedOpaqueReturnTypes.end())
+    return found->second;
 
   // Otherwise, we don't have a matching opaque decl.
   return nullptr;
 }
 
-void SourceFile::markDeclWithOpaqueResultTypeAsValidated(ValueDecl *vd) {
-  UnvalidatedDeclsWithOpaqueReturnTypes.erase(vd);
-  if (auto opaqueDecl = vd->getOpaqueResultTypeDecl()) {
-    auto inserted = ValidatedOpaqueReturnTypes.insert(
-              {opaqueDecl->getOpaqueReturnTypeIdentifier().str(), opaqueDecl});
-    if (inserted.second) {
-      OpaqueReturnTypes.push_back(opaqueDecl);
-    }
-  }
-}
-
 //===----------------------------------------------------------------------===//
 // Miscellaneous
 //===----------------------------------------------------------------------===//

lib/IRGen/GenDecl.cpp

@@ -440,7 +440,7 @@ void IRGenModule::emitSourceFile(SourceFile &SF) {
     emitGlobalDecl(decl);
   for (auto *localDecl : SF.LocalTypeDecls)
     emitGlobalDecl(localDecl);
-  for (auto *opaqueDecl : SF.OpaqueReturnTypes)
+  for (auto *opaqueDecl : SF.getOpaqueReturnTypeDecls())
     maybeEmitOpaqueTypeDecl(opaqueDecl);
 
   SF.collectLinkLibraries([this](LinkLibrary linkLib) {

lib/Sema/TypeCheckDecl.cpp

@@ -3966,12 +3966,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
       setBoundVarsTypeError(parentPattern, Context);
     }
 
-    if (VD->getOpaqueResultTypeDecl()) {
-      if (auto SF = VD->getInnermostDeclContext()->getParentSourceFile()) {
-        SF->markDeclWithOpaqueResultTypeAsValidated(VD);
-      }
-    }
-
     break;
   }
 
@@ -4111,13 +4105,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
         }
       }
     }
-
-    // Mark the opaque result type as validated, if there is one.
-    if (FD->getOpaqueResultTypeDecl()) {
-      if (auto sf = FD->getDeclContext()->getParentSourceFile()) {
-        sf->markDeclWithOpaqueResultTypeAsValidated(FD);
-      }
-    }
 
     break;
   }
@@ -4157,12 +4144,6 @@ void TypeChecker::validateDecl(ValueDecl *D) {
     validateResultType(SD, SD->getElementTypeLoc());
     SD->computeType();
 
-    if (SD->getOpaqueResultTypeDecl()) {
-      if (auto SF = SD->getInnermostDeclContext()->getParentSourceFile()) {
-        SF->markDeclWithOpaqueResultTypeAsValidated(SD);
-      }
-    }
-
     break;
   }
 

lib/Sema/TypeCheckPattern.cpp

@@ -1135,14 +1135,6 @@ bool TypeChecker::coercePatternToType(Pattern *&P, TypeResolution resolution,
     var->getTypeLoc() = tyLoc;
     var->getTypeLoc().setType(var->getType());
 
-    // FIXME: Copy pasted from validateDecl(). This needs to be converted to
-    // use requests.
-    if (var->getOpaqueResultTypeDecl()) {
-      if (auto sf = var->getInnermostDeclContext()->getParentSourceFile()) {
-        sf->markDeclWithOpaqueResultTypeAsValidated(var);
-      }
-    }
-
     // FIXME: Should probably just remove the forbidden prefix stuff, it no
     // longer makes a lot of sense in a request-based world.
     checkForForbiddenPrefix(var);

lib/Sema/TypeCheckType.cpp

@@ -2449,7 +2449,6 @@ Type TypeResolver::resolveOpaqueReturnType(TypeRepr *repr,
   if (definingDeclNode->getKind() == Node::Kind::Global)
     definingDeclNode = definingDeclNode->getChild(0);
   ASTBuilder builder(Context);
-  builder.Resolver = resolution.Resolver;
   auto opaqueNode =
     builder.getNodeFactory().createNode(Node::Kind::OpaqueReturnTypeOf);
   opaqueNode->addChild(definingDeclNode, builder.getNodeFactory());

lib/Serialization/Deserialization.cpp

@@ -1261,7 +1261,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
     auto name = getIdentifier(DefiningDeclNameID);
     pathTrace.addOpaqueReturnType(name);
 
-    if (auto opaque = baseModule->lookupOpaqueResultType(name.str(), nullptr)) {
+    if (auto opaque = baseModule->lookupOpaqueResultType(name.str())) {
       values.push_back(opaque);
     }
     break;
@@ -1664,8 +1664,7 @@ ModuleFile::resolveCrossReference(ModuleID MID, uint32_t pathLen) {
       pathTrace.addOpaqueReturnType(name);
 
       auto lookupModule = M ? M : baseModule;
-      if (auto opaqueTy = lookupModule->lookupOpaqueResultType(name.str(),
-                                                               nullptr)) {
+      if (auto opaqueTy = lookupModule->lookupOpaqueResultType(name.str())) {
         values.push_back(opaqueTy);
       }
       break;

lib/Serialization/SerializedModuleLoader.cpp

@@ -961,8 +961,7 @@ TypeDecl *SerializedASTFile::lookupLocalType(llvm::StringRef MangledName) const{
 }
 
 OpaqueTypeDecl *
-SerializedASTFile::lookupOpaqueResultType(StringRef MangledName,
-                                          LazyResolver *resolver) {
+SerializedASTFile::lookupOpaqueResultType(StringRef MangledName) {
   return File.lookupOpaqueResultType(MangledName);
 }