[clang][modulemap] Lazily load module maps by header name by Bigcheese · Pull Request #181916 · llvm/llvm-project

Bigcheese · 2026-02-17T21:43:38Z

After header search has found a header it looks for module maps that cover that header. This patch uses the parsed representation of module maps to do this search instead of relying on FileEntryRef lookups after stating headers in module maps.

This behavior is currently gated behind the -fmodules-lazy-load-module-maps -cc1 flag.

llvmbot · 2026-02-17T21:44:11Z

@llvm/pr-subscribers-clang-modules

@llvm/pr-subscribers-clang

Author: Michael Spencer (Bigcheese)

Changes

After header search has found a header it looks for module maps that cover that header. This patch uses the parsed representation of module maps to do this search instead of relying on FileEntryRef lookups after stating headers in module maps.

This behavior is currently gated behind the -fmodules-lazy-load-module-maps -cc1 flag.

Patch is 35.86 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/181916.diff

15 Files Affected:

(modified) clang/include/clang/Lex/HeaderSearch.h (+12)
(modified) clang/include/clang/Lex/HeaderSearchOptions.h (+6-1)
(modified) clang/include/clang/Lex/ModuleMap.h (+23)
(modified) clang/include/clang/Options/Options.td (+3)
(modified) clang/lib/Frontend/CompilerInstance.cpp (+4)
(modified) clang/lib/Lex/HeaderSearch.cpp (+219-38)
(modified) clang/lib/Lex/ModuleMap.cpp (+36)
(modified) clang/lib/Serialization/ASTReader.cpp (+14-8)
(added) clang/test/Modules/lazy-by-header-extern.c (+69)
(added) clang/test/Modules/lazy-by-header-lookup.c (+38)
(added) clang/test/Modules/lazy-by-header-nested.c (+46)
(added) clang/test/Modules/lazy-by-header-private.c (+41)
(added) clang/test/Modules/lazy-by-header-umbrella-dir.c (+53)
(added) clang/test/Modules/lazy-by-header-umbrella-header.c (+46)
(modified) clang/test/Modules/lazy-by-name-lookup.c (+2-2)

diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index 252e421e796f4..6d854a66ed3fa 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -334,11 +334,19 @@ class HeaderSearch {
 
   struct ModuleMapDirectoryState {
     OptionalFileEntryRef ModuleMapFile;
+    OptionalFileEntryRef PrivateModuleMapFile;
     enum {
       Parsed,
       Loaded,
       Invalid,
     } Status;
+
+    /// Relative header path -> list of module names
+    llvm::StringMap<llvm::SmallVector<StringRef, 1>> HeaderToModules{};
+    /// Relative dir path -> module name
+    llvm::SmallVector<std::pair<std::string, StringRef>, 2> UmbrellaDirModules{};
+    /// List of module names with umbrella header decls
+    llvm::SmallVector<StringRef, 2> UmbrellaHeaderModules{};
   };
 
   /// Describes whether a given directory has a module map in it.
@@ -372,6 +380,10 @@ class HeaderSearch {
   /// map their keys to the SearchDir index of their header map.
   void indexInitialHeaderMaps();
 
+  /// Build the header cache for a directory's module map.
+  void buildHeaderCache(DirectoryEntryRef Dir,
+                        ModuleMapDirectoryState &MMState);
+
 public:
   HeaderSearch(const HeaderSearchOptions &HSOpts, SourceManager &SourceMgr,
                DiagnosticsEngine &Diags, const LangOptions &LangOpts,
diff --git a/clang/include/clang/Lex/HeaderSearchOptions.h b/clang/include/clang/Lex/HeaderSearchOptions.h
index 2f33c0749f02a..d8d638b635ec9 100644
--- a/clang/include/clang/Lex/HeaderSearchOptions.h
+++ b/clang/include/clang/Lex/HeaderSearchOptions.h
@@ -283,6 +283,11 @@ class HeaderSearchOptions {
   LLVM_PREFERRED_TYPE(bool)
   unsigned AllowModuleMapSubdirectorySearch : 1;
 
+  /// Whether modules from module maps should only be loaded when used, not just
+  /// when parsed.
+  LLVM_PREFERRED_TYPE(bool)
+  unsigned LazyLoadModMaps : 1;
+
   HeaderSearchOptions(StringRef _Sysroot = "/")
       : Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(false),
         ImplicitModuleMaps(false), ModuleMapFileHomeIsCwd(false),
@@ -301,7 +306,7 @@ class HeaderSearchOptions {
         ModulesPruneNonAffectingModuleMaps(true), ModulesHashContent(false),
         ModulesSerializeOnlyPreprocessor(false),
         ModulesStrictContextHash(false), ModulesIncludeVFSUsage(false),
-        AllowModuleMapSubdirectorySearch(true) {}
+        AllowModuleMapSubdirectorySearch(true), LazyLoadModMaps(false) {}
 
   /// AddPath - Add the \p Path path to the specified \p Group list.
   void AddPath(StringRef Path, frontend::IncludeDirGroup Group,
diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h
index 6eba7cad0890b..a6e13aee3a328 100644
--- a/clang/include/clang/Lex/ModuleMap.h
+++ b/clang/include/clang/Lex/ModuleMap.h
@@ -435,6 +435,18 @@ class ModuleMap {
   KnownHeader findModuleForHeader(FileEntryRef File, bool AllowTextual = false,
                                   bool AllowExcluded = false);
 
+  /// Find the FileEntry for a header in a module as if it was written in the
+  /// module map as a header decl.
+  ///
+  /// \param M The module in which we're resolving the header directive.
+  /// \param NameAsWritten The name of the header as written in the module map.
+  /// \param RelativePathName Filled in with the relative path name from the
+  ///        module to the resolved header.
+  /// \return The resolved file, if any.
+  OptionalFileEntryRef
+  findUmbrellaHeaderForModule(Module *M, std::string NameAsWritten,
+                              SmallVectorImpl<char> &RelativePathName);
+
   /// Retrieve all the modules that contain the given header file. Note that
   /// this does not implicitly load module maps, except for builtin headers,
   /// and does not consult the external source. (Those checks are the
@@ -717,6 +729,8 @@ class ModuleMap {
                           DirectoryEntryRef Dir, FileID ID = FileID(),
                           SourceLocation ExternModuleLoc = SourceLocation());
 
+  void loadAllParsedModules();
+
   /// Load the given module map file, and record any modules we
   /// encounter.
   ///
@@ -743,6 +757,15 @@ class ModuleMap {
                             unsigned *Offset = nullptr,
                             SourceLocation ExternModuleLoc = SourceLocation());
 
+  /// Get the ModuleMapFile for a FileEntry previously parsed with
+  /// parseModuleMapFile.
+  const modulemap::ModuleMapFile *getParsedModuleMap(FileEntryRef File) const {
+    auto It = ParsedModuleMap.find(File);
+    if (It == ParsedModuleMap.end())
+      return nullptr;
+    return It->second;
+  }
+
   /// Dump the contents of the module map, for debugging purposes.
   void dump();
 
diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td
index 24b31fb3fefcc..0eb130e8d6cc2 100644
--- a/clang/include/clang/Options/Options.td
+++ b/clang/include/clang/Options/Options.td
@@ -8525,6 +8525,9 @@ def ftest_module_file_extension_EQ :
   Joined<["-"], "ftest-module-file-extension=">,
   HelpText<"introduce a module file extension for testing purposes. "
            "The argument is parsed as blockname:major:minor:hashed:user info">;
+def fmodules_lazy_load_module_maps : Flag<["-"], "fmodules-lazy-load-module-maps">,
+  HelpText<"Load modules from module maps only when needed">,
+  MarshallingInfoFlag<HeaderSearchOpts<"LazyLoadModMaps">>;
 
 defm recovery_ast : BoolOption<"f", "recovery-ast",
   LangOpts<"RecoveryAST">, DefaultTrue,
diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp
index 9f1a3c56feec1..9b48450b52a74 100644
--- a/clang/lib/Frontend/CompilerInstance.cpp
+++ b/clang/lib/Frontend/CompilerInstance.cpp
@@ -2261,6 +2261,10 @@ GlobalModuleIndex *CompilerInstance::loadGlobalModuleIndex(
   // we need to make the global index cover all modules, so we do that here.
   if (!HaveFullGlobalModuleIndex && GlobalIndex && !buildingModule()) {
     ModuleMap &MMap = getPreprocessor().getHeaderSearchInfo().getModuleMap();
+
+    // Load parsed but unloaded modules from the module maps.
+    MMap.loadAllParsedModules();
+
     bool RecreateIndex = false;
     for (ModuleMap::module_iterator I = MMap.module_begin(),
         E = MMap.module_end(); I != E; ++I) {
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 5f52d62bd36ed..7ded0624fec22 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1619,15 +1619,120 @@ StringRef HeaderSearch::getIncludeNameForHeader(const FileEntry *File) const {
   return It->second;
 }
 
+void HeaderSearch::buildHeaderCache(DirectoryEntryRef Dir,
+                                    ModuleMapDirectoryState &MMState) {
+  if (!MMState.ModuleMapFile)
+    return;
+  const modulemap::ModuleMapFile *ParsedMM =
+      ModMap.getParsedModuleMap(*MMState.ModuleMapFile);
+  if (!ParsedMM)
+    return;
+  const modulemap::ModuleMapFile *ParsedPrivateMM = nullptr;
+  if (MMState.PrivateModuleMapFile)
+    ParsedPrivateMM = ModMap.getParsedModuleMap(*MMState.PrivateModuleMapFile);
+
+  std::function<void(const modulemap::ModuleMapFile &, DirectoryEntryRef,
+                     StringRef)>
+      ProcessModuleMapFile = [&](const modulemap::ModuleMapFile &MMF,
+                                 DirectoryEntryRef MMDir, StringRef PathPrefix) {
+        auto AddToCache = [&](StringRef RelPath, StringRef ModuleName) {
+          if (PathPrefix.empty()) {
+            MMState.HeaderToModules[RelPath].push_back(ModuleName);
+          } else {
+            SmallString<128> FullPath(PathPrefix);
+            llvm::sys::path::append(FullPath, RelPath);
+            MMState.HeaderToModules[FullPath].push_back(ModuleName);
+          }
+        };
+
+        auto ProcessExternModuleDecl =
+            [&](const modulemap::ExternModuleDecl &EMD) {
+              StringRef FileNameRef = EMD.Path;
+              SmallString<128> ModuleMapFileName;
+              if (llvm::sys::path::is_relative(FileNameRef)) {
+                ModuleMapFileName = MMDir.getName();
+                llvm::sys::path::append(ModuleMapFileName, EMD.Path);
+                FileNameRef = ModuleMapFileName;
+              }
+              if (auto EFile = FileMgr.getOptionalFileRef(FileNameRef)) {
+                if (auto *ExtMMF = ModMap.getParsedModuleMap(*EFile)) {
+                  // Compute the path prefix for the extern module relative to
+                  // the original directory.
+                  SmallString<128> NewPrefix(PathPrefix);
+                  StringRef ExtDir = EFile->getDir().getName();
+                  StringRef OrigDir = Dir.getName();
+                  if (ExtDir.starts_with(OrigDir)) {
+                    StringRef RelDir = ExtDir.substr(OrigDir.size());
+                    while (RelDir.starts_with("/") || RelDir.starts_with("\\"))
+                      RelDir = RelDir.substr(1);
+                    if (!RelDir.empty())
+                      llvm::sys::path::append(NewPrefix, RelDir);
+                  }
+                  ProcessModuleMapFile(*ExtMMF, EFile->getDir(), NewPrefix);
+                }
+              }
+            };
+
+        std::function<void(const modulemap::ModuleDecl &, StringRef)>
+            ProcessModule = [&](const modulemap::ModuleDecl &MD,
+                                StringRef ModuleName) {
+              // Skip inferred submodules (module *)
+              if (MD.Id.front().first == "*")
+                return;
+              for (const auto &Decl : MD.Decls) {
+                std::visit(
+                    llvm::makeVisitor(
+                        [&](const modulemap::HeaderDecl &HD) {
+                          if (HD.Umbrella) {
+                            MMState.UmbrellaHeaderModules.push_back(ModuleName);
+                          } else {
+                            AddToCache(HD.Path, ModuleName);
+                          }
+                        },
+                        [&](const modulemap::UmbrellaDirDecl &UDD) {
+                          SmallString<128> FullPath(PathPrefix);
+                          llvm::sys::path::append(FullPath, UDD.Path);
+                          MMState.UmbrellaDirModules.push_back(
+                              std::make_pair(std::string(FullPath), ModuleName));
+                        },
+                        [&](const modulemap::ModuleDecl &SubMD) {
+                          ProcessModule(SubMD, ModuleName);
+                        },
+                        [&](const modulemap::ExternModuleDecl &EMD) {
+                          ProcessExternModuleDecl(EMD);
+                        },
+                        [](const auto &) {
+                          // Ignore other decls.
+                        }),
+                    Decl);
+              }
+            };
+
+        for (const auto &Decl : MMF.Decls) {
+          std::visit(llvm::makeVisitor(
+                         [&](const modulemap::ModuleDecl &MD) {
+                           ProcessModule(MD, MD.Id.front().first);
+                         },
+                         [&](const modulemap::ExternModuleDecl &EMD) {
+                           ProcessExternModuleDecl(EMD);
+                         }),
+                     Decl);
+        }
+      };
+
+  ProcessModuleMapFile(*ParsedMM, Dir, "");
+  if (ParsedPrivateMM)
+    ProcessModuleMapFile(*ParsedPrivateMM, Dir, "");
+}
+
 bool HeaderSearch::hasModuleMap(StringRef FileName,
                                 const DirectoryEntry *Root,
                                 bool IsSystem) {
   if (!HSOpts.ImplicitModuleMaps)
     return false;
 
-  SmallVector<DirectoryEntryRef, 2> FixUpDirectories;
-
   StringRef DirName = FileName;
+  const DirectoryEntry *CurDir = nullptr;
   do {
     // Get the parent directory name.
     DirName = llvm::sys::path::parent_path(DirName);
@@ -1638,33 +1743,76 @@ bool HeaderSearch::hasModuleMap(StringRef FileName,
     auto Dir = FileMgr.getOptionalDirectoryRef(DirName);
     if (!Dir)
       return false;
+    CurDir = *Dir;
+
+    bool IsFramework =
+        llvm::sys::path::extension(Dir->getName()) == ".framework";
+
+    // Check if it's possible that the module map for this directory can resolve
+    // this header.
+    parseModuleMapFile(*Dir, IsSystem, IsFramework);
+    auto DirState = DirectoryModuleMap.find(*Dir);
+    if (DirState == DirectoryModuleMap.end() || !DirState->second.ModuleMapFile)
+      continue;
 
-    // Try to load the module map file in this directory.
-    switch (parseAndLoadModuleMapFile(
-        *Dir, IsSystem,
-        llvm::sys::path::extension(Dir->getName()) == ".framework")) {
-    case MMR_NewlyProcessed:
-    case MMR_AlreadyProcessed: {
-      // Success. All of the directories we stepped through inherit this module
-      // map file.
-      const ModuleMapDirectoryState &MMDS = DirectoryModuleMap[*Dir];
-      for (unsigned I = 0, N = FixUpDirectories.size(); I != N; ++I)
-        DirectoryModuleMap[FixUpDirectories[I]] = MMDS;
+    if (!HSOpts.LazyLoadModMaps)
       return true;
+
+    auto &MMState = DirState->second;
+
+    // Build cache if not already built
+    if (MMState.HeaderToModules.empty() && MMState.UmbrellaDirModules.empty() &&
+        MMState.UmbrellaHeaderModules.empty()) {
+      buildHeaderCache(*Dir, MMState);
     }
-    case MMR_NoDirectory:
-    case MMR_InvalidModuleMap:
-      break;
+
+    // Compute relative path from directory to the file
+    StringRef DirPath = Dir->getName();
+    StringRef RelativePath;
+    if (FileName.starts_with(DirPath)) {
+      RelativePath = FileName.substr(DirPath.size());
+      // Strip leading slash
+      while (RelativePath.starts_with("/") || RelativePath.starts_with("\\"))
+        RelativePath = RelativePath.substr(1);
+    } else {
+      // Can't compute relative path, skip this directory
+      continue;
     }
 
-    // If we hit the top of our search, we're done.
-    if (*Dir == Root)
-      return false;
+    // Check for exact matches in cache
+    llvm::SmallVector<StringRef, 4> ModulesToLoad;
+    auto CachedMods = MMState.HeaderToModules.find(RelativePath);
+    if (CachedMods != MMState.HeaderToModules.end()) {
+      ModulesToLoad.append(CachedMods->second.begin(),
+                           CachedMods->second.end());
+    }
 
-    // Keep track of all of the directories we checked, so we can mark them as
-    // having module maps if we eventually do find a module map.
-    FixUpDirectories.push_back(*Dir);
-  } while (true);
+    // Check umbrella directories
+    for (const auto &UmbrellaDir : MMState.UmbrellaDirModules) {
+      if (RelativePath.starts_with(UmbrellaDir.first) ||
+          UmbrellaDir.first == ".") {
+        ModulesToLoad.push_back(UmbrellaDir.second);
+      }
+    }
+
+    // Add umbrella header modules (conservative)
+    ModulesToLoad.append(MMState.UmbrellaHeaderModules.begin(),
+                         MMState.UmbrellaHeaderModules.end());
+
+    // Load all matching modules
+    bool LoadedAny = false;
+    for (StringRef ModName : ModulesToLoad) {
+      if (ModMap.findOrLoadModule(ModName)) {
+        LoadedAny = true;
+      }
+    }
+
+    if (LoadedAny)
+      return true;
+
+    // If we hit the top of our search, we're done.
+  } while (CurDir != Root);
+  return false;
 }
 
 ModuleMap::KnownHeader
@@ -1698,12 +1846,9 @@ HeaderSearch::findResolvedModulesForHeader(FileEntryRef File) const {
   return ModMap.findResolvedModulesForHeader(File);
 }
 
-static bool suggestModule(HeaderSearch &HS, FileEntryRef File,
-                          Module *RequestingModule,
+static bool suggestModule(HeaderSearch &HS, ModuleMap::KnownHeader Module,
+                          FileEntryRef File, clang::Module *RequestingModule,
                           ModuleMap::KnownHeader *SuggestedModule) {
-  ModuleMap::KnownHeader Module =
-      HS.findModuleForHeader(File, /*AllowTextual*/true);
-
   // If this module specifies [no_undeclared_includes], we cannot find any
   // file that's in a non-dependency module.
   if (RequestingModule && Module && RequestingModule->NoUndeclaredIncludes) {
@@ -1737,9 +1882,31 @@ bool HeaderSearch::findUsableModuleForHeader(
     FileEntryRef File, const DirectoryEntry *Root, Module *RequestingModule,
     ModuleMap::KnownHeader *SuggestedModule, bool IsSystemHeaderDir) {
   if (needModuleLookup(RequestingModule, SuggestedModule)) {
-    // If there is a module that corresponds to this header, suggest it.
-    hasModuleMap(File.getNameAsRequested(), Root, IsSystemHeaderDir);
-    return suggestModule(*this, File, RequestingModule, SuggestedModule);
+    if (!HSOpts.LazyLoadModMaps) {
+      // NOTE: This is required for `shadowed-submodule.m` to pass as it relies
+      //       on A1/module.modulemap being loaded even though we already know
+      //       which module the header belongs to. We will remove this behavior
+      //       as part of lazy module map loading.
+      hasModuleMap(File.getNameAsRequested(), Root, IsSystemHeaderDir);
+      ModuleMap::KnownHeader Module =
+          findModuleForHeader(File, /*AllowTextual=*/true);
+      return suggestModule(*this, Module, File, RequestingModule,
+                           SuggestedModule);
+    }
+
+    // First check if we already know about this header
+    ModuleMap::KnownHeader Module =
+        findModuleForHeader(File, /*AllowTextual=*/true);
+
+    // If we don't have a module yet, try to find/load module maps
+    if (!Module) {
+      hasModuleMap(File.getNameAsRequested(), Root, IsSystemHeaderDir);
+      // Try again after loading module maps
+      Module = ModMap.findModuleForHeader(File, /*AllowTextual=*/true);
+    }
+
+    return suggestModule(*this, Module, File, RequestingModule,
+                         SuggestedModule);
   }
   return true;
 }
@@ -1766,7 +1933,10 @@ bool HeaderSearch::findUsableModuleForFrameworkHeader(
     // important so that we're consistent about whether this header
     // corresponds to a module. Possibly we should lock down framework modules
     // so that this is not possible.
-    return suggestModule(*this, File, RequestingModule, SuggestedModule);
+    ModuleMap::KnownHeader Module =
+        findModuleForHeader(File, /*AllowTextual=*/true);
+    return suggestModule(*this, Module, File, RequestingModule,
+                         SuggestedModule);
   }
   return true;
 }
@@ -1858,7 +2028,7 @@ HeaderSearch::parseAndLoadModuleMapFileImpl(FileEntryRef File, bool IsSystem,
 
   // Try to load a corresponding private module map.
   if (OptionalFileEntryRef PMMFile =
-          getPrivateModuleMap(File, FileMgr, Diags, !ParsedModuleMaps[File])) {
+          getPrivateModuleMap(File, FileMgr, Diags, false)) {
     if (ModMap.parseAndLoadModuleMapFile(*PMMFile, IsSystem, Dir)) {
       LoadedModuleMaps[File] = false;
       return MMR_InvalidModuleMap;
@@ -1886,7 +2056,7 @@ HeaderSearch::parseModuleMapFileImpl(FileEntryRef File, bool IsSystem,
 
   // Try to parse a corresponding private module map.
   if (OptionalFileEntryRef PMMFile =
-          getPrivateModuleMap(File, FileMgr, Diags)) {
+          getPrivateModuleMap(File, FileMgr, Diags, false)) {
     if (ModMap.parseModuleMapFile(*PMMFile, IsSystem, Dir)) {
       ParsedModuleMaps[File] = false;
       return MMR_InvalidModuleMap;
@@ -1965,7 +2135,7 @@ HeaderSearch::ModuleMapResult
 HeaderSearch::parseAndLoadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
                                         bool IsFramework) {
   auto InsertRes = DirectoryModuleMap.insert(std::pair{
-      Dir, ModuleMapDirectoryState{{}, ModuleMapDirectoryState::Invalid}});
+      Dir, ModuleMapDirectoryState{{}, {}, ModuleMapDirectoryState::Invalid}});
   ModuleMapDirectoryState &MMState = InsertRes.first->second;
   if (!InsertRes.second) {
     switch (MMState.Status) {
@@ -1978,8 +2148,12 @@ HeaderSearch::parseAndLoadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
     };
   }
 
-  if (!MMState.ModuleMapFile)
+  if (!MMState.ModuleMapFile) {
     MMState.ModuleMapFile = lookupModuleMapFile(Dir, IsFramework);
+    if (MMState.ModuleMapFile)
+      MMState.PrivateModuleMapFile =
+          getPrivateModuleMap(*MMState.ModuleMapFile, FileMgr, Diags);
+  }
 
   if (MMState.ModuleMapFile) {
     ModuleMapResult Result =
@@ -2008,8 +2182,11 @@ HeaderSearch::parseModul...
[truncated]

github-actions · 2026-02-17T21:45:39Z

✅ With the latest revision this PR passed the C/C++ code formatter.

github-actions · 2026-02-17T22:34:50Z

🪟 Windows x64 Test Results

54412 tests passed
2315 tests skipped

✅ The build succeeded and all tests passed.

jansvoboda11

This is a large patch, so a have a bunch of questions to make sure I'm understanding this right. Some of those explanations might make sense to put directly into code comments. I love the remarks-based tests!

jansvoboda11 · 2026-02-18T21:47:12Z

+                          MMState.UmbrellaDirModules.push_back(std::make_pair(
+                              std::string(FullPath), ModuleName));


Documentation for MMState.UmbrellaDirModules says the first element in the pair is a relative path. I guess FullPath here is not an absolute path, but a (relative) path from the top-level module map that (transitively) includes the current module map? Might be worth finding more descriptive name.

Yeah, it's the full relative path from the top level module map. I'll try to find a better name.

I still see FullPath in the latest revision. Maybe PathRelativeToTopLevel or something in that vein?

jansvoboda11 · 2026-02-18T21:57:55Z

@@ -1360,6 +1370,26 @@ bool ModuleMap::parseModuleMapFile(FileEntryRef File, bool IsSystem,
      std::make_unique<modulemap::ModuleMapFile>(std::move(*MaybeMMF)));
  const modulemap::ModuleMapFile &MMF = *ParsedModuleMaps.back();
  std::vector<const modulemap::ExternModuleDecl *> PendingExternalModuleMaps;
+  std::function<void(const modulemap::ModuleDecl &)> CollectExternDecls =


Did we not recurse into extern module maps prior to this patch?

Nope, that was an existing issue that now has a test.

Ah, ok. I would suggest creating a separate PR if other people find this PR too large to effectively review.

jansvoboda11 · 2026-02-18T22:00:29Z

+      SmallString<128> RelativePathName;
+      if (auto Umbrella = ModMap.findUmbrellaHeaderForModule(
+              CurrentModule, Blob.str(), RelativePathName)) {


Why run a search here instead of serializing RelativePathName into the PCM?

findUmbrellaHeaderForModule really just does the path modification logic for framework headers, it doesn't actually do a search. Since we have to get a FileEntryRef anyway, I thought it made sense to use the same logic as we use when loading a module from a module map.

I see. I would slightly prefer just serializing this and avoiding extra logic in the readers, but I don't think that's a blocker for this PR.

After header search has found a header it looks for module maps that cover that header. This patch uses the parsed representation of module maps to do this search instead of relying on FileEntryRef lookups after stating headers in module maps. This behavior is currently gated behind the -fmodules-lazy-load-module-maps -cc1 flag.

jansvoboda11

I left a couple more suggestions behind, but this looks good overall. Thanks!

vsapsai · 2026-02-25T03:09:07Z

I'm still looking at the change, it takes some time. If you want to merge it, I don't have anything against it and I trust Jan's and your judgment. Otherwise I'll keep looking at it. In any case it is useful for me to understand this direction.

vsapsai

Did a first pass looking only at the headers and ignoring the implementation on purpose. So some of my comments can be off.

One big part that's missing is mentioning a new option in the release notes.

vsapsai · 2026-02-25T23:37:57Z

+  /// Find the FileEntry for an umbrella header in a module as if it was written
+  /// in the module map as a header decl.


It's not clear from the comment why the umbrella-ness of the method matters here. I.e., why it cannot be something like findUmbrellaHeaderForModule.

This uses an UnresolvedHeaderDirective internally which cares if it's an umbrella header.

I see. Looks like ModuleMap::findHeader doesn't have a separate behavior for umbrella headers. One option is to call ModuleMap::findHeader (after making it public) directly from ASTReader.

To be honest, I don't have strong feelings about it and fine with whatever direction you choose.

vsapsai · 2026-02-25T23:45:27Z

  HelpText<"introduce a module file extension for testing purposes. "
           "The argument is parsed as blockname:major:minor:hashed:user info">;
+def fmodules_lazy_load_module_maps : Flag<["-"], "fmodules-lazy-load-module-maps">,
+  HelpText<"Load modules from module maps only when needed">,


"when needed" is pretty ambiguous. Though we might prefer to be ambiguous here to have an option to change the logic later.

vsapsai

Have a few questions but otherwise looks good.

Don't remember any other places where we should handle LazyLoadModuleMaps. I think at this point you know it much better than I do.

vsapsai · 2026-02-26T06:17:04Z

 }

+OptionalFileEntryRef ModuleMap::findUmbrellaHeaderForModule(
+    Module *M, std::string NameAsWritten,


Not 100% sure it is better but it seems easier to follow when you pass as an argument StringRef and then convert to std::string at the very end. The approach with copy & move is a little bit trickier for me, don't know how common it is in the codebase.

When constructing a std::string is required it's generally better to push that construction to the parameter. It allows skipping an allocation if the input is already a std::string and can just be moved.

vsapsai · 2026-02-26T06:35:01Z

+  /// Find the FileEntry for an umbrella header in a module as if it was written
+  /// in the module map as a header decl.


I see. Looks like ModuleMap::findHeader doesn't have a separate behavior for umbrella headers. One option is to call ModuleMap::findHeader (after making it public) directly from ASTReader.

To be honest, I don't have strong feelings about it and fine with whatever direction you choose.

vsapsai · 2026-02-27T20:53:26Z

Didn't click "Approve" button in UI, providing verbal "Approve".

After header search has found a header it looks for module maps that cover that header. This patch uses the parsed representation of module maps to do this search instead of relying on FileEntryRef lookups after stating headers in module maps. This behavior is currently gated behind the `-fmodules-lazy-load-module-maps` `-cc1` flag.

This cherry-picks the sequence of patches needed for the -Wmodule-map-path-outside-directory and -Wmmap-deprecated-symlink-to-modular-header diagnostics from upstream. These have been thoroughly tested as a group. Upstream commits: 1. 32fb8c5 [clang][modules] Lazily load by name lookups in module maps (llvm#132853) 2. 502d2d4 [clang][modulemap] Don't call translateFile (llvm#176288) 3. d0afaea [clang][modulemap] Lazily load module maps by header name (llvm#181916) 4. 90fdad2 [clang][modules] Add warning for module maps with ".." paths (llvm#184279) 5. a17b132 [llvm][support] Refactor symlink handling and add readlink (llvm#184256) 6. 67ff769 [clang][modules] Add warning for symlinks to modular headers (llvm#188059)

Previously in the case that the directory was missing, loading a pcm would crash. Now it attempts to load absolute paths first, and gracefully ignores missing directories. Follow up to #181916 Fixes: #215931 Assisted By: codex

Previously in the case that the directory was missing, loading a pcm would crash. Now it attempts to load absolute paths first, and gracefully ignores missing directories. Follow up to llvm/llvm-project#181916 Fixes: llvm/llvm-project#215931 Assisted By: codex

Previously in the case that the directory was missing, loading a pcm would crash. Now it attempts to load absolute paths first, and gracefully ignores missing directories. Follow up to llvm/llvm-project#181916 Fixes: llvm/llvm-project#215931 Assisted By: codex Signed-off-by: Hafidz Muzakky <ais.muzakky@gmail.com>

Previously in the case that the directory was missing, loading a pcm would crash. Now it attempts to load absolute paths first, and gracefully ignores missing directories. Follow up to llvm#181916 Fixes: llvm#215931 Assisted By: codex

Previously in the case that the directory was missing, loading a pcm would crash. Now it attempts to load absolute paths first, and gracefully ignores missing directories. Follow up to llvm#181916 Fixes: llvm#215931 Assisted By: codex (cherry picked from commit cd1abef)

Bigcheese requested a review from jansvoboda11 February 17, 2026 21:43

llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" clang:modules C++20 modules and Clang Header Modules labels Feb 17, 2026

Bigcheese force-pushed the dev/lazy-headers branch from 98195f2 to 7e2ea25 Compare February 18, 2026 00:39

jansvoboda11 reviewed Feb 18, 2026

View reviewed changes

Bigcheese force-pushed the dev/lazy-headers branch from 7e2ea25 to ce9695a Compare February 20, 2026 00:22

jansvoboda11 reviewed Feb 23, 2026

View reviewed changes

Comment thread clang/lib/Lex/HeaderSearch.cpp Outdated

Bigcheese added 4 commits February 23, 2026 17:42

Fix windows tests, this normalizes paths because they include slashes.

28d2fa8

Split up lambdas into separate private member functions.

32ccb86

format

6608927

Clarify umbrella header comment

31e20dd

Bigcheese requested a review from vsapsai February 24, 2026 19:24

jansvoboda11 approved these changes Feb 24, 2026

View reviewed changes

Comment thread clang/include/clang/Lex/HeaderSearch.h Outdated

Comment thread clang/lib/Lex/HeaderSearch.cpp Outdated

Use index instead of cache and split out visitor.

6f07c05

vsapsai reviewed Feb 25, 2026

View reviewed changes

Bigcheese added 2 commits February 25, 2026 16:08

LazyLoadModuleMaps rename and [out].

c1aa07a

Format

1d71955

vsapsai reviewed Feb 26, 2026

View reviewed changes

Adjust comments

bc28a0f

Bigcheese merged commit d0afaea into llvm:main Feb 27, 2026
10 checks passed

This was referenced Apr 15, 2026

[X86] regenerate fcopysign test checks (#183710) ampandey-1995/llvm-project#4

Closed

[X86] regenerate fcopysign test checks (#183710) ampandey-1995/llvm-project#5

Closed

Bigcheese mentioned this pull request Apr 28, 2026

[clang] Merge lazy parsing driven module map diagnostics swiftlang/llvm-project#12848

Merged

congt mentioned this pull request Aug 13, 2026

[clang] ASTReader crashes in ModuleMap::findHeader loading a PCM with an umbrella header (23.1.0-rc regression) #215931

Closed

keith mentioned this pull request Aug 21, 2026

[clang][modulemap] Fix crashes loading pcms #218011

Merged

keith mentioned this pull request Aug 28, 2026

[6.4.x][clang][modulemap] Fix crashes loading pcms (#218011) swiftlang/llvm-project#14045

Merged

		MMState.UmbrellaDirModules.push_back(std::make_pair(
		std::string(FullPath), ModuleName));

		/// Find the FileEntry for an umbrella header in a module as if it was written
		/// in the module map as a header decl.

Conversation

Bigcheese commented Feb 17, 2026

Uh oh!

llvmbot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Feb 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🪟 Windows x64 Test Results

Uh oh!

jansvoboda11 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jansvoboda11 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

vsapsai commented Feb 25, 2026

Uh oh!

vsapsai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsapsai left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vsapsai commented Feb 27, 2026

Uh oh!

Reviewers

Assignees

llvmbot commented Feb 17, 2026 •

edited

Loading

github-actions Bot commented Feb 17, 2026 •

edited

Loading

github-actions Bot commented Feb 17, 2026 •

edited

Loading