[Serialization] Delay adding ModuleFiles until they're loaded

Rather than adding a ModuleFile to a parent module
and then removing it afterwards if it fails to
load, let's wait until we've loaded the file before
deciding to add it to the parent module. This then
allows us to get rid of `ModuleDecl::removeFile`.

In addition, push down the calls to `addFile` into
the callers of `loadAST` in preparation for
`addFile` being replaced with a one-time-only call
to a `setFiles` method.

Author: hamishknight

include/swift/AST/Module.h

@@ -371,7 +371,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.

lib/AST/Module.cpp

@@ -507,19 +507,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

@@ -837,10 +837,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))
+    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/SerializedModuleLoader.cpp

@@ -680,7 +680,6 @@ 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())
@@ -705,8 +704,6 @@ FileUnit *SerializedModuleLoaderBase::loadAST(
       findOverlayFiles(diagLoc.getValueOr(SourceLoc()), &M, fileUnit);
       return fileUnit;
     }
-
-    M.removeFile(*fileUnit);
   }
 
   // From here on is the failure path.
@@ -960,12 +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)) {
+  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;
 }
 
@@ -996,11 +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)) {
+  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;
 }

tools/SourceKit/lib/SwiftLang/SwiftIndexing.cpp

@@ -195,6 +195,7 @@ static void indexModule(llvm::MemoryBuffer *Input,
       return;
     }
 
+    Mod->addFile(*FUnit);
     Mod->setHasResolvedImports();
   }