Merge pull request #32067 from hamishknight/a-delayed-filing

Author: hamishknight

include/swift/AST/FileUnit.h

@@ -384,6 +384,9 @@ class LoadedFile : public FileUnit {
 
   virtual bool isSystemModule() const { return false; }
 
+  /// Checks whether an error was encountered while loading the file.
+  virtual bool hadLoadError() const { return false; }
+
   /// Retrieve the set of generic signatures stored within this module.
   ///
   /// \returns \c true if this module file supports retrieving all of the

include/swift/AST/Module.h

@@ -363,6 +363,7 @@ class ModuleDecl : public DeclContext, public TypeDecl {
   ArrayRef<ImplicitImport> getImplicitImports() const;
 
   ArrayRef<FileUnit *> getFiles() {
+    assert(!Files.empty() || failedToLoad());
     return Files;
   }
   ArrayRef<const FileUnit *> getFiles() const {
@@ -371,7 +372,6 @@ class ModuleDecl : public DeclContext, public TypeDecl {
 
   bool isClangModule() const;
   void addFile(FileUnit &newFile);
-  void removeFile(FileUnit &existingFile);
 
   /// Creates a map from \c #filePath strings to corresponding \c #file
   /// strings, diagnosing any conflicts.

include/swift/Serialization/SerializedModuleLoader.h

@@ -149,12 +149,13 @@ class SerializedModuleLoaderBase : public ModuleLoader {
   ///
   /// If the AST cannot be loaded and \p diagLoc is present, a diagnostic is
   /// printed. (Note that \p diagLoc is allowed to be invalid.)
-  FileUnit *loadAST(ModuleDecl &M, Optional<SourceLoc> diagLoc,
-                    StringRef moduleInterfacePath,
-                    std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
-                    std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
-                    std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
-                    bool isFramework, bool treatAsPartialModule);
+  FileUnit *
+  loadAST(ModuleDecl &M, Optional<SourceLoc> diagLoc,
+          StringRef moduleInterfacePath,
+          std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
+          std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
+          std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
+          bool isFramework);
 
   /// Check whether the module with a given name can be imported without
   /// importing it.
@@ -321,6 +322,8 @@ class SerializedASTFile final : public LoadedFile {
   /// file.
   const version::Version &getLanguageVersionBuiltWith() const;
 
+  virtual bool hadLoadError() const override;
+
   virtual bool isSystemModule() const override;
 
   virtual void lookupValue(DeclName name, NLKind lookupKind,

lib/AST/ASTContext.cpp

@@ -1515,11 +1515,6 @@ void ASTContext::verifyAllLoadedModules() const {
   FrontendStatsTracer tracer(Stats, "verify-all-loaded-modules");
   for (auto &loader : getImpl().ModuleLoaders)
     loader->verifyAllModules();
-
-  for (auto &topLevelModulePair : LoadedModules) {
-    ModuleDecl *M = topLevelModulePair.second;
-    assert(!M->getFiles().empty() || M->failedToLoad());
-  }
 #endif
 }
 

lib/AST/Module.cpp

@@ -498,6 +498,10 @@ bool ModuleDecl::isClangModule() const {
 }
 
 void ModuleDecl::addFile(FileUnit &newFile) {
+  // If this is a LoadedFile, make sure it loaded without error.
+  assert(!(isa<LoadedFile>(newFile) &&
+           cast<LoadedFile>(newFile).hadLoadError()));
+
   // Require Main and REPL files to be the first file added.
   assert(Files.empty() ||
          !isa<SourceFile>(newFile) ||
@@ -507,19 +511,6 @@ void ModuleDecl::addFile(FileUnit &newFile) {
   clearLookupCache();
 }
 
-void ModuleDecl::removeFile(FileUnit &existingFile) {
-  // Do a reverse search; usually the file to be deleted will be at the end.
-  std::reverse_iterator<decltype(Files)::iterator> I(Files.end()),
-  E(Files.begin());
-  I = std::find(I, E, &existingFile);
-  assert(I != E);
-
-  // Adjust for the std::reverse_iterator offset.
-  ++I;
-  Files.erase(I.base());
-  clearLookupCache();
-}
-
 #define FORWARD(name, args) \
   for (const FileUnit *file : getFiles()) \
     file->name args;

lib/Frontend/Frontend.cpp

@@ -845,11 +845,16 @@ bool CompilerInstance::loadPartialModulesAndImplicitImports() {
   // Parse all the partial modules first.
   for (auto &PM : PartialModules) {
     assert(PM.ModuleBuffer);
-    if (!SML->loadAST(*MainModule, SourceLoc(), /*moduleInterfacePath*/"",
-                      std::move(PM.ModuleBuffer), std::move(PM.ModuleDocBuffer),
-                      std::move(PM.ModuleSourceInfoBuffer), /*isFramework*/false,
-                      /*treatAsPartialModule*/true))
+    auto *file =
+        SML->loadAST(*MainModule, SourceLoc(), /*moduleInterfacePath*/ "",
+                     std::move(PM.ModuleBuffer), std::move(PM.ModuleDocBuffer),
+                     std::move(PM.ModuleSourceInfoBuffer),
+                     /*isFramework*/ false);
+    if (file) {
+      MainModule->addFile(*file);
+    } else {
       hadLoadError = true;
+    }
   }
   return hadLoadError;
 }

lib/Serialization/ModuleFile.cpp

@@ -1977,9 +1977,7 @@ ModuleFile::ModuleFile(
   }
 }
 
-Status ModuleFile::associateWithFileContext(FileUnit *file,
-                                            SourceLoc diagLoc,
-                                            bool treatAsPartialModule) {
+Status ModuleFile::associateWithFileContext(FileUnit *file, SourceLoc diagLoc) {
   PrettyStackTraceModuleFile stackEntry(*this);
 
   assert(!hasError() && "error already detected; should not call this");
@@ -2033,8 +2031,12 @@ Status ModuleFile::associateWithFileContext(FileUnit *file,
       continue;
     }
 
+    // If this module file is being installed into the main module, it's treated
+    // as a partial module.
+    auto isPartialModule = M->isMainModule();
+
     if (dependency.isImplementationOnly() &&
-        !(treatAsPartialModule || ctx.LangOpts.DebuggerSupport)) {
+        !(isPartialModule || ctx.LangOpts.DebuggerSupport)) {
       // When building normally (and not merging partial modules), we don't
       // want to bring in the implementation-only module, because that might
       // change the set of visible declarations. However, when debugging we

lib/Serialization/ModuleFile.h

@@ -705,14 +705,10 @@ class ModuleFile
   /// This does not include diagnostics about \e this file failing to load,
   /// but rather other things that might be imported as part of bringing the
   /// file into the AST.
-  /// \param treatAsPartialModule If true, processes implementation-only
-  /// information instead of assuming the client won't need it and shouldn't
-  /// see it.
   ///
   /// \returns any error that occurred during association, such as being
   /// compiled for a different OS.
-  Status associateWithFileContext(FileUnit *file, SourceLoc diagLoc,
-                                  bool treatAsPartialModule);
+  Status associateWithFileContext(FileUnit *file, SourceLoc diagLoc);
 
   /// Transfers ownership of a buffer that might contain source code where
   /// other parts of the compiler could have emitted diagnostics, to keep them

lib/Serialization/SerializedModuleLoader.cpp

@@ -651,7 +651,7 @@ FileUnit *SerializedModuleLoaderBase::loadAST(
     std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer,
     std::unique_ptr<llvm::MemoryBuffer> moduleDocInputBuffer,
     std::unique_ptr<llvm::MemoryBuffer> moduleSourceInfoInputBuffer,
-    bool isFramework, bool treatAsPartialModule) {
+    bool isFramework) {
   assert(moduleInputBuffer);
 
   StringRef moduleBufferID = moduleInputBuffer->getBufferIdentifier();
@@ -680,16 +680,14 @@ FileUnit *SerializedModuleLoaderBase::loadAST(
 
     // We've loaded the file. Now try to bring it into the AST.
     auto fileUnit = new (Ctx) SerializedASTFile(M, *loadedModuleFile);
-    M.addFile(*fileUnit);
     if (extendedInfo.isTestable())
       M.setTestingEnabled();
     if (extendedInfo.arePrivateImportsEnabled())
       M.setPrivateImportsEnabled();
 
     auto diagLocOrInvalid = diagLoc.getValueOr(SourceLoc());
     loadInfo.status =
-        loadedModuleFile->associateWithFileContext(fileUnit, diagLocOrInvalid,
-                                                   treatAsPartialModule);
+        loadedModuleFile->associateWithFileContext(fileUnit, diagLocOrInvalid);
 
     // FIXME: This seems wrong. Overlay for system Clang module doesn't
     // necessarily mean it's "system" module. User can make their own overlay
@@ -706,8 +704,6 @@ FileUnit *SerializedModuleLoaderBase::loadAST(
       findOverlayFiles(diagLoc.getValueOr(SourceLoc()), &M, fileUnit);
       return fileUnit;
     }
-
-    M.removeFile(*fileUnit);
   }
 
   // From here on is the failure path.
@@ -961,13 +957,15 @@ SerializedModuleLoaderBase::loadModule(SourceLoc importLoc,
   StringRef moduleInterfacePathStr =
     Ctx.AllocateCopy(moduleInterfacePath.str());
 
-  if (!loadAST(*M, moduleID.Loc, moduleInterfacePathStr,
-               std::move(moduleInputBuffer), std::move(moduleDocInputBuffer),
-               std::move(moduleSourceInfoInputBuffer),
-               isFramework, /*treatAsPartialModule*/false)) {
+  auto *file =
+      loadAST(*M, moduleID.Loc, moduleInterfacePathStr,
+              std::move(moduleInputBuffer), std::move(moduleDocInputBuffer),
+              std::move(moduleSourceInfoInputBuffer), isFramework);
+  if (file) {
+    M->addFile(*file);
+  } else {
     M->setFailedToLoad();
   }
-
   return M;
 }
 
@@ -990,7 +988,6 @@ MemoryBufferSerializedModuleLoader::loadModule(SourceLoc importLoc,
     return nullptr;
 
   bool isFramework = false;
-  bool treatAsPartialModule = false;
   std::unique_ptr<llvm::MemoryBuffer> moduleInputBuffer;
   moduleInputBuffer = std::move(bufIter->second);
   MemoryBuffers.erase(bufIter);
@@ -999,12 +996,12 @@ MemoryBufferSerializedModuleLoader::loadModule(SourceLoc importLoc,
   auto *M = ModuleDecl::create(moduleID.Item, Ctx);
   SWIFT_DEFER { M->setHasResolvedImports(); };
 
-  if (!loadAST(*M, moduleID.Loc, /*moduleInterfacePath*/ "",
-               std::move(moduleInputBuffer), {}, {},
-               isFramework, treatAsPartialModule)) {
+  auto *file = loadAST(*M, moduleID.Loc, /*moduleInterfacePath*/ "",
+                       std::move(moduleInputBuffer), {}, {}, isFramework);
+  if (!file)
     return nullptr;
-  }
 
+  M->addFile(*file);
   Ctx.LoadedModules[moduleID.Item] = M;
   return M;
 }
@@ -1106,6 +1103,10 @@ bool SerializedASTFile::isSIB() const {
   return File.IsSIB;
 }
 
+bool SerializedASTFile::hadLoadError() const {
+  return File.hasError();
+}
+
 bool SerializedASTFile::isSystemModule() const {
   if (auto Mod = File.getUnderlyingModule()) {
     return Mod->isSystemModule();

tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp

@@ -186,8 +186,7 @@ static void indexModule(llvm::MemoryBuffer *Input,
     // correct filename here.
     auto FUnit = Loader->loadAST(*Mod, None, /*moduleInterfacePath*/"",
                                  std::move(Buf), nullptr, nullptr,
-                                 /*isFramework*/false,
-                                 /*treatAsPartialModule*/false);
+                                 /*isFramework*/false);
 
     // FIXME: Not knowing what went wrong is pretty bad. loadModule() should be
     // more modular, rather than emitting diagnostics itself.
@@ -196,6 +195,7 @@ static void indexModule(llvm::MemoryBuffer *Input,
       return;
     }
 
+    Mod->addFile(*FUnit);
     Mod->setHasResolvedImports();
   }