Revert "[clang][modules] Lazily load by name lookups in module maps (llvm#132853)"

This is causing a slowdown in SourceKit completion requests.

The slowdown is caused by the call to `SourceManager::translateFile`
that was added to avoid allocating separate source location space for
each module map in each `CompilerInstance`.

rdar://162196108

This reverts commit 32fb8c5.

Author: Bigcheese

clang-tools-extra/modularize/ModularizeUtilities.cpp

@@ -287,7 +287,7 @@ std::error_code ModularizeUtilities::loadModuleMap(
     Target.get(), *HeaderInfo));
 
   // Parse module.modulemap file into module map.
-  if (ModMap->parseAndLoadModuleMapFile(ModuleMapEntry, false, Dir)) {
+  if (ModMap->loadModuleMapFile(ModuleMapEntry, false, Dir)) {
     return std::error_code(1, std::generic_category());
   }
 

clang/include/clang/Basic/DiagnosticGroups.td

@@ -629,7 +629,6 @@ def ModuleImport : DiagGroup<"module-import">;
 def ModuleConflict : DiagGroup<"module-conflict">;
 def ModuleFileExtension : DiagGroup<"module-file-extension">;
 def ModuleIncludeDirectiveTranslation : DiagGroup<"module-include-translation">;
-def ModuleMap : DiagGroup<"module-map">;
 def IndexStore : DiagGroup<"index-store">;
 def RoundTripCC1Args : DiagGroup<"round-trip-cc1-args">;
 def NewlineEOF : DiagGroup<"newline-eof">;

clang/include/clang/Basic/DiagnosticLexKinds.td

@@ -848,12 +848,6 @@ def err_pp_encounter_nonreproducible: Error<
   "encountered non-reproducible token, caching failed">;
 
 // Module map parsing
-def remark_mmap_parse : Remark<
-  "parsing modulemap '%0'">, ShowInSystemHeader, InGroup<ModuleMap>;
-def remark_mmap_load : Remark<
-  "loading modulemap '%0'">, ShowInSystemHeader, InGroup<ModuleMap>;
-def remark_mmap_load_module : Remark<
-  "loading parsed module '%0'">, ShowInSystemHeader, InGroup<ModuleMap>;
 def err_mmap_unknown_token : Error<"skipping stray token">;
 def err_mmap_expected_module : Error<"expected module declaration">;
 def err_mmap_expected_module_name : Error<"expected module name">;

clang/include/clang/Lex/HeaderSearch.h

@@ -332,27 +332,13 @@ class HeaderSearch {
   /// The mapping between modules and headers.
   mutable ModuleMap ModMap;
 
-  struct ModuleMapDirectoryState {
-    OptionalFileEntryRef ModuleMapFile;
-    enum {
-      Parsed,
-      Loaded,
-      Invalid,
-    } Status;
-  };
-
   /// Describes whether a given directory has a module map in it.
-  llvm::DenseMap<const DirectoryEntry *, ModuleMapDirectoryState>
-      DirectoryModuleMap;
+  llvm::DenseMap<const DirectoryEntry *, bool> DirectoryHasModuleMap;
 
   /// Set of module map files we've already loaded, and a flag indicating
   /// whether they were valid or not.
   llvm::DenseMap<const FileEntry *, bool> LoadedModuleMaps;
 
-  /// Set of module map files we've already parsed, and a flag indicating
-  /// whether they were valid or not.
-  llvm::DenseMap<const FileEntry *, bool> ParsedModuleMaps;
-
   // A map of discovered headers with their associated include file name.
   llvm::DenseMap<const FileEntry *, llvm::SmallString<64>> IncludeNames;
 
@@ -447,6 +433,11 @@ class HeaderSearch {
   /// Retrieve the path to the module cache.
   StringRef getModuleCachePath() const { return ModuleCachePath; }
 
+  /// Consider modules when including files from this directory.
+  void setDirectoryHasModuleMap(const DirectoryEntry* Dir) {
+    DirectoryHasModuleMap[Dir] = true;
+  }
+
   /// Forget everything we know about headers so far.
   void ClearFileInfo() {
     FileInfo.clear();
@@ -722,10 +713,9 @@ class HeaderSearch {
   ///        used to resolve paths within the module (this is required when
   ///        building the module from preprocessed source).
   /// \returns true if an error occurred, false otherwise.
-  bool parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem,
-                                 FileID ID = FileID(),
-                                 unsigned *Offset = nullptr,
-                                 StringRef OriginalModuleMapFile = StringRef());
+  bool loadModuleMapFile(FileEntryRef File, bool IsSystem, FileID ID = FileID(),
+                         unsigned *Offset = nullptr,
+                         StringRef OriginalModuleMapFile = StringRef());
 
   /// Collect the set of all known, top-level modules.
   ///
@@ -925,31 +915,26 @@ class HeaderSearch {
   size_t getTotalMemory() const;
 
 private:
-  /// Describes what happened when we tried to load or parse a module map file.
-  enum ModuleMapResult {
-    /// The module map file had already been processed.
-    MMR_AlreadyProcessed,
+  /// Describes what happened when we tried to load a module map file.
+  enum LoadModuleMapResult {
+    /// The module map file had already been loaded.
+    LMM_AlreadyLoaded,
 
-    /// The module map file was processed by this invocation.
-    MMR_NewlyProcessed,
+    /// The module map file was loaded by this invocation.
+    LMM_NewlyLoaded,
 
     /// There is was directory with the given name.
-    MMR_NoDirectory,
+    LMM_NoDirectory,
 
     /// There was either no module map file or the module map file was
     /// invalid.
-    MMR_InvalidModuleMap
+    LMM_InvalidModuleMap
   };
 
-  ModuleMapResult parseAndLoadModuleMapFileImpl(FileEntryRef File,
-                                                bool IsSystem,
-                                                DirectoryEntryRef Dir,
-                                                FileID ID = FileID(),
-                                                unsigned *Offset = nullptr);
-
-  ModuleMapResult parseModuleMapFileImpl(FileEntryRef File, bool IsSystem,
-                                         DirectoryEntryRef Dir,
-                                         FileID ID = FileID());
+  LoadModuleMapResult loadModuleMapFileImpl(FileEntryRef File, bool IsSystem,
+                                            DirectoryEntryRef Dir,
+                                            FileID ID = FileID(),
+                                            unsigned *Offset = nullptr);
 
   /// Try to load the module map file in the given directory.
   ///
@@ -960,8 +945,8 @@ class HeaderSearch {
   ///
   /// \returns The result of attempting to load the module map file from the
   /// named directory.
-  ModuleMapResult parseAndLoadModuleMapFile(StringRef DirName, bool IsSystem,
-                                            bool IsFramework);
+  LoadModuleMapResult loadModuleMapFile(StringRef DirName, bool IsSystem,
+                                        bool IsFramework);
 
   /// Try to load the module map file in the given directory.
   ///
@@ -971,13 +956,8 @@ class HeaderSearch {
   ///
   /// \returns The result of attempting to load the module map file from the
   /// named directory.
-  ModuleMapResult parseAndLoadModuleMapFile(DirectoryEntryRef Dir,
-                                            bool IsSystem, bool IsFramework);
-
-  ModuleMapResult parseModuleMapFile(StringRef DirName, bool IsSystem,
-                                     bool IsFramework);
-  ModuleMapResult parseModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
-                                     bool IsFramework);
+  LoadModuleMapResult loadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
+                                        bool IsFramework);
 };
 
 /// Apply the header search options to get given HeaderSearch object.

clang/include/clang/Lex/ModuleMap.h

@@ -18,7 +18,6 @@
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/Module.h"
 #include "clang/Basic/SourceLocation.h"
-#include "clang/Lex/ModuleMapFile.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
@@ -271,18 +270,6 @@ class ModuleMap {
   /// Describes whether we haved loaded a particular file as a module
   /// map.
   llvm::DenseMap<const FileEntry *, bool> LoadedModuleMap;
-  llvm::DenseMap<const FileEntry *, const modulemap::ModuleMapFile *>
-      ParsedModuleMap;
-
-  std::vector<std::unique_ptr<modulemap::ModuleMapFile>> ParsedModuleMaps;
-
-  /// Map from top level module name to a list of ModuleDecls in the order they
-  /// were discovered. This allows handling shadowing correctly and diagnosing
-  /// redefinitions.
-  llvm::StringMap<SmallVector<std::pair<const modulemap::ModuleMapFile *,
-                                        const modulemap::ModuleDecl *>,
-                              1>>
-      ParsedModules;
 
   /// Resolve the given export declaration into an actual export
   /// declaration.
@@ -499,8 +486,6 @@ class ModuleMap {
   /// \returns The named module, if known; otherwise, returns null.
   Module *findModule(StringRef Name) const;
 
-  Module *findOrLoadModule(StringRef Name);
-
   Module *findOrInferSubmodule(Module *Parent, StringRef Name);
 
   /// Retrieve a module with the given name using lexical name lookup,
@@ -716,11 +701,6 @@ class ModuleMap {
   void addHeader(Module *Mod, Module::Header Header,
                  ModuleHeaderRole Role, bool Imported = false);
 
-  /// Parse a module map without creating `clang::Module` instances.
-  bool parseModuleMapFile(FileEntryRef File, bool IsSystem,
-                          DirectoryEntryRef Dir, FileID ID = FileID(),
-                          SourceLocation ExternModuleLoc = SourceLocation());
-
   /// Load the given module map file, and record any modules we
   /// encounter.
   ///
@@ -741,11 +721,10 @@ class ModuleMap {
   ///        that caused us to load this module map file, if any.
   ///
   /// \returns true if an error occurred, false otherwise.
-  bool
-  parseAndLoadModuleMapFile(FileEntryRef File, bool IsSystem,
-                            DirectoryEntryRef HomeDir, FileID ID = FileID(),
-                            unsigned *Offset = nullptr,
-                            SourceLocation ExternModuleLoc = SourceLocation());
+  bool loadModuleMapFile(FileEntryRef File, bool IsSystem,
+                         DirectoryEntryRef HomeDir, FileID ID = FileID(),
+                         unsigned *Offset = nullptr,
+                         SourceLocation ExternModuleLoc = SourceLocation());
 
   /// Dump the contents of the module map, for debugging purposes.
   void dump();

clang/include/clang/Lex/ModuleMapFile.h

@@ -133,17 +133,8 @@ using TopLevelDecl = std::variant<ModuleDecl, ExternModuleDecl>;
 /// This holds many reference types (StringRef, SourceLocation, etc.) whose
 /// lifetimes are bound by the SourceManager and FileManager used.
 struct ModuleMapFile {
-  /// The FileID used to parse this module map. This is always a local ID.
-  FileID ID;
-
-  /// The directory in which the module map was discovered. Declarations in
-  /// the module map are relative to this directory.
-  OptionalDirectoryEntryRef Dir;
-
   /// Beginning of the file, used for moduleMapFileRead callback.
   SourceLocation Start;
-
-  bool IsSystem;
   std::vector<TopLevelDecl> Decls;
 
   void dump(llvm::raw_ostream &out) const;

clang/lib/Frontend/CompilerInstance.cpp

@@ -599,13 +599,13 @@ struct ReadModuleNames : ASTReaderListener {
     ModuleMap &MM = PP.getHeaderSearchInfo().getModuleMap();
     for (const std::string &LoadedModule : LoadedModules)
       MM.cacheModuleLoad(*PP.getIdentifierInfo(LoadedModule),
-                         MM.findOrLoadModule(LoadedModule));
+                         MM.findModule(LoadedModule));
     LoadedModules.clear();
   }
 
   void markAllUnavailable() {
     for (const std::string &LoadedModule : LoadedModules) {
-      if (Module *M = PP.getHeaderSearchInfo().getModuleMap().findOrLoadModule(
+      if (Module *M = PP.getHeaderSearchInfo().getModuleMap().findModule(
               LoadedModule)) {
         M->HasIncompatibleModuleFile = true;
 

clang/lib/Frontend/FrontendAction.cpp

@@ -627,8 +627,8 @@ static bool loadModuleMapForModuleBuild(CompilerInstance &CI, bool IsSystem,
   }
 
   // Load the module map file.
-  if (HS.parseAndLoadModuleMapFile(*ModuleMap, IsSystem, ModuleMapID, &Offset,
-                                   PresumedModuleMapFile))
+  if (HS.loadModuleMapFile(*ModuleMap, IsSystem, ModuleMapID, &Offset,
+                           PresumedModuleMapFile))
     return true;
 
   if (SrcMgr.getBufferOrFake(ModuleMapID).getBufferSize() == Offset)
@@ -1248,8 +1248,8 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI,
   // If we were asked to load any module map files, do so now.
   for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
     if (auto File = CI.getFileManager().getOptionalFileRef(Filename))
-      CI.getPreprocessor().getHeaderSearchInfo().parseAndLoadModuleMapFile(
-          *File, /*IsSystem*/ false);
+      CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+          *File, /*IsSystem*/false);
     else
       CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
   }