Skip to content

Commit d19d7c6

Browse files
committedMar 25, 2025
[clang][modules] Lazily load by name lookups in module maps
Instead of eagerly populating the `clang::ModuleMap` when looking up a module by name, this patch changes `HeaderSearch` to only load the modules that are actually used. This introduces `ModuleMap::findOrLoadModule` which will load modules from parsed but not loaded module maps. This cannot be used anywhere that the module loading code calls into as it can create infinite recursion. This currently just reparses module maps when looking up a module by header. This is fine as redeclarations are allowed from the same file, but future patches will also make looking up a module by header lazy. This patch changes the shadow.m test to use explicitly built modules and `#import`. This test and the shadow feature are very brittle and do not work in general. The test relied on pcm files being left behind by prior failing clang invocations that were then reused by the last invocation. If you clean the cache then the last invocation will always fail. This is because the input module map and the `-fmodule-map-file=` module map are parsed in the same module scope, and `-fmodule-map-file=` is forwarded to implicit module builds. That means you are guaranteed to hit a module redeclaration error if the TU actually imports the module it is trying to shadow. This patch changes when we load A2's module map to after the `A` module has been loaded, which sets the `IsFromModuleFile` bit on `A`. This means that A2's `A` is skipped entirely instead of creating a shadow module, and we get textual inclusion. It is possible to construct a case where this would happen before this patch too. An upcoming patch in this series will rework shadowing to work in the general case, but that's only possible once header -> module lookup is lazy too.
1 parent e879213 commit d19d7c6

16 files changed

+394
-92
lines changed
 

‎clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -576,6 +576,7 @@ def ModuleImport : DiagGroup<"module-import">;
576576
def ModuleConflict : DiagGroup<"module-conflict">;
577577
def ModuleFileExtension : DiagGroup<"module-file-extension">;
578578
def ModuleIncludeDirectiveTranslation : DiagGroup<"module-include-translation">;
579+
def ModuleMap : DiagGroup<"module-map">;
579580
def RoundTripCC1Args : DiagGroup<"round-trip-cc1-args">;
580581
def NewlineEOF : DiagGroup<"newline-eof">;
581582
def Nullability : DiagGroup<"nullability">;

‎clang/include/clang/Basic/DiagnosticLexKinds.td

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,12 @@ def warn_pp_date_time : Warning<
836836
ShowInSystemHeader, DefaultIgnore, InGroup<DiagGroup<"date-time">>;
837837

838838
// Module map parsing
839+
def remark_mmap_parse : Remark<
840+
"parsing modulemap '%0'">, ShowInSystemHeader, InGroup<ModuleMap>;
841+
def remark_mmap_load : Remark<
842+
"loading modulemap '%0'">, ShowInSystemHeader, InGroup<ModuleMap>;
843+
def remark_mmap_load_module : Remark<
844+
"loading parsed module '%0'">, ShowInSystemHeader, InGroup<ModuleMap>;
839845
def err_mmap_unknown_token : Error<"skipping stray token">;
840846
def err_mmap_expected_module : Error<"expected module declaration">;
841847
def err_mmap_expected_module_name : Error<"expected module name">;

‎clang/include/clang/Lex/HeaderSearch.h

Lines changed: 40 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -332,13 +332,27 @@ class HeaderSearch {
332332
/// The mapping between modules and headers.
333333
mutable ModuleMap ModMap;
334334

335+
struct ModuleMapDirectoryState {
336+
OptionalFileEntryRef ModuleMapFile;
337+
enum {
338+
Parsed,
339+
Loaded,
340+
Invalid,
341+
} Status;
342+
};
343+
335344
/// Describes whether a given directory has a module map in it.
336-
llvm::DenseMap<const DirectoryEntry *, bool> DirectoryHasModuleMap;
345+
llvm::DenseMap<const DirectoryEntry *, ModuleMapDirectoryState>
346+
DirectoryModuleMap;
337347

338348
/// Set of module map files we've already loaded, and a flag indicating
339349
/// whether they were valid or not.
340350
llvm::DenseMap<const FileEntry *, bool> LoadedModuleMaps;
341351

352+
/// Set of module map files we've already parsed, and a flag indicating
353+
/// whether they were valid or not.
354+
llvm::DenseMap<const FileEntry *, bool> ParsedModuleMaps;
355+
342356
// A map of discovered headers with their associated include file name.
343357
llvm::DenseMap<const FileEntry *, llvm::SmallString<64>> IncludeNames;
344358

@@ -433,11 +447,6 @@ class HeaderSearch {
433447
/// Retrieve the path to the module cache.
434448
StringRef getModuleCachePath() const { return ModuleCachePath; }
435449

436-
/// Consider modules when including files from this directory.
437-
void setDirectoryHasModuleMap(const DirectoryEntry* Dir) {
438-
DirectoryHasModuleMap[Dir] = true;
439-
}
440-
441450
/// Forget everything we know about headers so far.
442451
void ClearFileInfo() {
443452
FileInfo.clear();
@@ -915,26 +924,30 @@ class HeaderSearch {
915924
size_t getTotalMemory() const;
916925

917926
private:
918-
/// Describes what happened when we tried to load a module map file.
919-
enum LoadModuleMapResult {
920-
/// The module map file had already been loaded.
921-
LMM_AlreadyLoaded,
927+
/// Describes what happened when we tried to load or parse a module map file.
928+
enum ModuleMapResult {
929+
/// The module map file had already been processed.
930+
MMR_AlreadyProcessed,
922931

923-
/// The module map file was loaded by this invocation.
924-
LMM_NewlyLoaded,
932+
/// The module map file was processed by this invocation.
933+
MMR_NewlyProcessed,
925934

926935
/// There is was directory with the given name.
927-
LMM_NoDirectory,
936+
MMR_NoDirectory,
928937

929938
/// There was either no module map file or the module map file was
930939
/// invalid.
931-
LMM_InvalidModuleMap
940+
MMR_InvalidModuleMap
932941
};
933942

934-
LoadModuleMapResult loadModuleMapFileImpl(FileEntryRef File, bool IsSystem,
935-
DirectoryEntryRef Dir,
936-
FileID ID = FileID(),
937-
unsigned *Offset = nullptr);
943+
ModuleMapResult loadModuleMapFileImpl(FileEntryRef File, bool IsSystem,
944+
DirectoryEntryRef Dir,
945+
FileID ID = FileID(),
946+
unsigned *Offset = nullptr);
947+
948+
ModuleMapResult parseModuleMapFileImpl(FileEntryRef File, bool IsSystem,
949+
DirectoryEntryRef Dir,
950+
FileID ID = FileID());
938951

939952
/// Try to load the module map file in the given directory.
940953
///
@@ -945,8 +958,8 @@ class HeaderSearch {
945958
///
946959
/// \returns The result of attempting to load the module map file from the
947960
/// named directory.
948-
LoadModuleMapResult loadModuleMapFile(StringRef DirName, bool IsSystem,
949-
bool IsFramework);
961+
ModuleMapResult loadModuleMapFile(StringRef DirName, bool IsSystem,
962+
bool IsFramework);
950963

951964
/// Try to load the module map file in the given directory.
952965
///
@@ -956,8 +969,13 @@ class HeaderSearch {
956969
///
957970
/// \returns The result of attempting to load the module map file from the
958971
/// named directory.
959-
LoadModuleMapResult loadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
960-
bool IsFramework);
972+
ModuleMapResult loadModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
973+
bool IsFramework);
974+
975+
ModuleMapResult parseModuleMapFile(StringRef DirName, bool IsSystem,
976+
bool IsFramework);
977+
ModuleMapResult parseModuleMapFile(DirectoryEntryRef Dir, bool IsSystem,
978+
bool IsFramework);
961979
};
962980

963981
/// Apply the header search options to get given HeaderSearch object.

‎clang/include/clang/Lex/ModuleMap.h

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
#include "clang/Basic/LangOptions.h"
1919
#include "clang/Basic/Module.h"
2020
#include "clang/Basic/SourceLocation.h"
21+
#include "clang/Lex/ModuleMapFile.h"
2122
#include "llvm/ADT/ArrayRef.h"
2223
#include "llvm/ADT/DenseMap.h"
2324
#include "llvm/ADT/DenseSet.h"
@@ -262,6 +263,18 @@ class ModuleMap {
262263
/// Describes whether we haved loaded a particular file as a module
263264
/// map.
264265
llvm::DenseMap<const FileEntry *, bool> LoadedModuleMap;
266+
llvm::DenseMap<const FileEntry *, const modulemap::ModuleMapFile *>
267+
ParsedModuleMap;
268+
269+
std::vector<std::unique_ptr<modulemap::ModuleMapFile>> ParsedModuleMaps;
270+
271+
/// Map from top level module name to a list of ModuleDecls in the order they
272+
/// were discovered. This allows handling shadowing correctly and diagnosing
273+
/// redefinitions.
274+
llvm::StringMap<SmallVector<std::pair<const modulemap::ModuleMapFile *,
275+
const modulemap::ModuleDecl *>,
276+
1>>
277+
ParsedModules;
265278

266279
/// Resolve the given export declaration into an actual export
267280
/// declaration.
@@ -478,6 +491,8 @@ class ModuleMap {
478491
/// \returns The named module, if known; otherwise, returns null.
479492
Module *findModule(StringRef Name) const;
480493

494+
Module *findOrLoadModule(StringRef Name);
495+
481496
Module *findOrInferSubmodule(Module *Parent, StringRef Name);
482497

483498
/// Retrieve a module with the given name using lexical name lookup,
@@ -693,6 +708,11 @@ class ModuleMap {
693708
void addHeader(Module *Mod, Module::Header Header,
694709
ModuleHeaderRole Role, bool Imported = false);
695710

711+
/// Parse a module map without creating `clang::Module` instances.
712+
bool parseModuleMapFile(FileEntryRef File, bool IsSystem,
713+
DirectoryEntryRef Dir, FileID ID = FileID(),
714+
SourceLocation ExternModuleLoc = SourceLocation());
715+
696716
/// Load the given module map file, and record any modules we
697717
/// encounter.
698718
///

‎clang/include/clang/Lex/ModuleMapFile.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,17 @@ using TopLevelDecl = std::variant<ModuleDecl, ExternModuleDecl>;
133133
/// This holds many reference types (StringRef, SourceLocation, etc.) whose
134134
/// lifetimes are bound by the SourceManager and FileManager used.
135135
struct ModuleMapFile {
136+
/// The FileID used to parse this module map. This is always a local ID.
137+
FileID ID;
138+
139+
/// The directory in which the module map was discovered. Declarations in
140+
/// the module map are relative to this directory.
141+
OptionalDirectoryEntryRef Dir;
142+
136143
/// Beginning of the file, used for moduleMapFileRead callback.
137144
SourceLocation Start;
145+
146+
bool IsSystem;
138147
std::vector<TopLevelDecl> Decls;
139148

140149
void dump(llvm::raw_ostream &out) const;

‎clang/lib/Frontend/CompilerInstance.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -579,13 +579,13 @@ struct ReadModuleNames : ASTReaderListener {
579579
ModuleMap &MM = PP.getHeaderSearchInfo().getModuleMap();
580580
for (const std::string &LoadedModule : LoadedModules)
581581
MM.cacheModuleLoad(*PP.getIdentifierInfo(LoadedModule),
582-
MM.findModule(LoadedModule));
582+
MM.findOrLoadModule(LoadedModule));
583583
LoadedModules.clear();
584584
}
585585

586586
void markAllUnavailable() {
587587
for (const std::string &LoadedModule : LoadedModules) {
588-
if (Module *M = PP.getHeaderSearchInfo().getModuleMap().findModule(
588+
if (Module *M = PP.getHeaderSearchInfo().getModuleMap().findOrLoadModule(
589589
LoadedModule)) {
590590
M->HasIncompatibleModuleFile = true;
591591

0 commit comments

Comments
 (0)