[clang] da1a16a - [clang][modules] Preserve the module map that allowed inferring (#113389)
via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 28 11:24:32 PDT 2024
Author: Jan Svoboda
Date: 2024-10-28T11:24:27-07:00
New Revision: da1a16ae10177494c7cae929bec987e90a160403
URL: https://github.com/llvm/llvm-project/commit/da1a16ae10177494c7cae929bec987e90a160403
DIFF: https://github.com/llvm/llvm-project/commit/da1a16ae10177494c7cae929bec987e90a160403.diff
LOG: [clang][modules] Preserve the module map that allowed inferring (#113389)
With inferred modules, the dependency scanner takes care to replace the
fake "__inferred_module.map" path with the file that allowed the module
to be inferred. However, this only worked when such a module was
imported directly in the TU. Whenever such module got loaded
transitively, the scanner would fail to perform the replacement. This is
caused by the fact that PCM files are lossy and drop this information.
This patch makes sure that PCMs include this file for each submodule (in
the `SUBMODULE_DEFINITION` record), fixes one existing test with an
incorrect assertion, and does a little drive-by refactoring of
`ModuleMap`.
Added:
Modified:
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/Frontend/FrontendAction.cpp
clang/lib/Lex/ModuleMap.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/link-libraries.c
Removed:
################################################################################
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index 3ddbc5fcd26c44..b6193866fc7134 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -44,7 +44,7 @@ namespace serialization {
/// Version 4 of AST files also requires that the version control branch and
/// revision match exactly, since there is no backward compatibility of
/// AST files at this time.
-const unsigned VERSION_MAJOR = 31;
+const unsigned VERSION_MAJOR = 32;
/// AST file minor version number supported by this version of
/// Clang.
@@ -54,7 +54,7 @@ const unsigned VERSION_MAJOR = 31;
/// for the previous version could still support reading the new
/// version by ignoring new kinds of subblocks), this number
/// should be increased.
-const unsigned VERSION_MINOR = 1;
+const unsigned VERSION_MINOR = 0;
/// An ID number that refers to an identifier in an AST file.
///
diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h
index b476a40ebd2c8c..070c1c9a54f48c 100644
--- a/clang/include/clang/Serialization/ASTReader.h
+++ b/clang/include/clang/Serialization/ASTReader.h
@@ -2335,6 +2335,8 @@ class ASTReader
/// Translate a FileID from another module file's FileID space into ours.
FileID TranslateFileID(ModuleFile &F, FileID FID) const {
assert(FID.ID >= 0 && "Reading non-local FileID.");
+ if (FID.isInvalid())
+ return FID;
return FileID::get(F.SLocEntryBaseID + FID.ID - 1);
}
diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp
index 8264bd702fe43f..9a50e7453eb61a 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -534,7 +534,6 @@ static Module *prepareToBuildModule(CompilerInstance &CI,
}
if (*OriginalModuleMap != CI.getSourceManager().getFileEntryRefForID(
CI.getSourceManager().getMainFileID())) {
- M->IsInferred = true;
auto FileCharacter =
M->IsSystem ? SrcMgr::C_System_ModuleMap : SrcMgr::C_User_ModuleMap;
FileID OriginalModuleMapFID = CI.getSourceManager().getOrCreateFileID(
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index bc76a54abd95ad..201ab91cf68ca1 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -657,8 +657,7 @@ ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) {
llvm::sys::path::stem(SkippedDir.getName()), NameBuf);
Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
Explicit).first;
- InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
- Result->IsInferred = true;
+ setInferredModuleAllowedBy(Result, UmbrellaModuleMap);
// Associate the module and the directory.
UmbrellaDirs[SkippedDir] = Result;
@@ -675,8 +674,7 @@ ModuleMap::findOrCreateModuleForHeaderInUmbrellaDir(FileEntryRef File) {
llvm::sys::path::stem(File.getName()), NameBuf);
Result = findOrCreateModule(Name, Result, /*IsFramework=*/false,
Explicit).first;
- InferredModuleAllowedBy[Result] = UmbrellaModuleMap;
- Result->IsInferred = true;
+ setInferredModuleAllowedBy(Result, UmbrellaModuleMap);
Result->addTopHeader(File);
// If inferred submodules export everything they import, add a
@@ -1097,8 +1095,7 @@ Module *ModuleMap::inferFrameworkModule(DirectoryEntryRef FrameworkDir,
Module *Result = new (ModulesAlloc.Allocate())
Module(ModuleConstructorTag{}, ModuleName, SourceLocation(), Parent,
/*IsFramework=*/true, /*IsExplicit=*/false, NumCreatedModules++);
- InferredModuleAllowedBy[Result] = ModuleMapFID;
- Result->IsInferred = true;
+ setInferredModuleAllowedBy(Result, ModuleMapFID);
if (!Parent) {
if (LangOpts.CurrentModule == ModuleName)
SourceModule = Result;
@@ -1346,7 +1343,7 @@ ModuleMap::getModuleMapFileForUniquing(const Module *M) const {
}
void ModuleMap::setInferredModuleAllowedBy(Module *M, FileID ModMapFID) {
- assert(M->IsInferred && "module not inferred");
+ M->IsInferred = true;
InferredModuleAllowedBy[M] = ModMapFID;
}
diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp
index 7d9170e7f0b479..2419ed84e68acf 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5813,6 +5813,7 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
SubmoduleID Parent = getGlobalSubmoduleID(F, Record[Idx++]);
Module::ModuleKind Kind = (Module::ModuleKind)Record[Idx++];
SourceLocation DefinitionLoc = ReadSourceLocation(F, Record[Idx++]);
+ FileID InferredAllowedBy = ReadFileID(F, Record, Idx);
bool IsFramework = Record[Idx++];
bool IsExplicit = Record[Idx++];
bool IsSystem = Record[Idx++];
@@ -5834,8 +5835,6 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
ModMap.findOrCreateModule(Name, ParentModule, IsFramework, IsExplicit)
.first;
- // FIXME: Call ModMap.setInferredModuleAllowedBy()
-
SubmoduleID GlobalIndex = GlobalID - NUM_PREDEF_SUBMODULE_IDS;
if (GlobalIndex >= SubmodulesLoaded.size() ||
SubmodulesLoaded[GlobalIndex])
@@ -5866,6 +5865,8 @@ llvm::Error ASTReader::ReadSubmoduleBlock(ModuleFile &F,
CurrentModule->DefinitionLoc = DefinitionLoc;
CurrentModule->Signature = F.Signature;
CurrentModule->IsFromModuleFile = true;
+ if (InferredAllowedBy.isValid())
+ ModMap.setInferredModuleAllowedBy(CurrentModule, InferredAllowedBy);
CurrentModule->IsSystem = IsSystem || CurrentModule->IsSystem;
CurrentModule->IsExternC = IsExternC;
CurrentModule->InferSubmodules = InferSubmodules;
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index c09a41f4d1403c..569c688f793d81 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -2914,6 +2914,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 6)); // Parent
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 4)); // Kind
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 8)); // Definition location
+ Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::VBR, 4)); // Inferred allowed by
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsFramework
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsExplicit
Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // IsSystem
@@ -3018,6 +3019,12 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
SourceLocationEncoding::RawLocEncoding DefinitionLoc =
getRawSourceLocationEncoding(getAdjustedLocation(Mod->DefinitionLoc));
+ ModuleMap &ModMap = PP->getHeaderSearchInfo().getModuleMap();
+ FileID UnadjustedInferredFID;
+ if (Mod->IsInferred)
+ UnadjustedInferredFID = ModMap.getModuleMapFileIDForUniquing(Mod);
+ int InferredFID = getAdjustedFileID(UnadjustedInferredFID).getOpaqueValue();
+
// Emit the definition of the block.
{
RecordData::value_type Record[] = {SUBMODULE_DEFINITION,
@@ -3025,6 +3032,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
ParentID,
(RecordData::value_type)Mod->Kind,
DefinitionLoc,
+ (RecordData::value_type)InferredFID,
Mod->IsFramework,
Mod->IsExplicit,
Mod->IsSystem,
diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 77f9d07175c2c1..637416cd1fc621 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -587,9 +587,7 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
ModuleMap &ModMapInfo =
MDC.ScanInstance.getPreprocessor().getHeaderSearchInfo().getModuleMap();
- OptionalFileEntryRef ModuleMap = ModMapInfo.getModuleMapFileForUniquing(M);
-
- if (ModuleMap) {
+ if (auto ModuleMap = ModMapInfo.getModuleMapFileForUniquing(M)) {
SmallString<128> Path = ModuleMap->getNameAsRequested();
ModMapInfo.canonicalizeModuleMapPath(Path);
MD.ClangModuleMapFile = std::string(Path);
@@ -601,15 +599,13 @@ ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
MDC.ScanInstance.getASTReader()->visitInputFileInfos(
*MF, /*IncludeSystem=*/true,
[&](const serialization::InputFileInfo &IFI, bool IsSystem) {
- // __inferred_module.map is the result of the way in which an implicit
- // module build handles inferred modules. It adds an overlay VFS with
- // this file in the proper directory and relies on the rest of Clang to
- // handle it like normal. With explicitly built modules we don't need
- // to play VFS tricks, so replace it with the correct module map.
- if (StringRef(IFI.Filename).ends_with("__inferred_module.map")) {
- MDC.addFileDep(MD, ModuleMap->getName());
+ // The __inferred_module.map file is an insignificant implementation
+ // detail of implicitly-built modules. The PCM will also report the
+ // actual on-disk module map file that allowed inferring the module,
+ // which is what we need for building the module explicitly
+ // Let's ignore this file.
+ if (StringRef(IFI.Filename).ends_with("__inferred_module.map"))
return;
- }
MDC.addFileDep(MD, IFI.Filename);
});
diff --git a/clang/test/ClangScanDeps/link-libraries.c b/clang/test/ClangScanDeps/link-libraries.c
index c09691d2356efc..bc0b0c546ea032 100644
--- a/clang/test/ClangScanDeps/link-libraries.c
+++ b/clang/test/ClangScanDeps/link-libraries.c
@@ -39,14 +39,13 @@ module transitive {
// CHECK-NEXT: "modules": [
// CHECK-NEXT: {
// CHECK-NEXT: "clang-module-deps": [],
-// CHECK-NEXT: "clang-modulemap-file": "{{.*}}/__inferred_module.map",
+// CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/Inputs/frameworks/module.modulemap",
// CHECK-NEXT: "command-line": [
// CHECK: ],
// CHECK-NEXT: "context-hash": "{{.*}}",
// CHECK-NEXT: "file-deps": [
-// CHECK-NEXT: "{{.*}}/Framework.h"
-// CHECK-NEXT: "{{.*}}/__inferred_module.map"
-// CHECK-NEXT: "{{.*}}/module.modulemap"
+// CHECK-NEXT: "[[PREFIX]]/Inputs/frameworks/Framework.framework/Headers/Framework.h"
+// CHECK-NEXT: "[[PREFIX]]/Inputs/frameworks/module.modulemap"
// CHECK-NEXT: ],
// CHECK-NEXT: "link-libraries": [
// CHECK-NEXT: {
More information about the cfe-commits
mailing list