[clang] [clang][modules] Preserve the module map that allowed inferring (PR #113389)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 28 11:22:36 PDT 2024
https://github.com/jansvoboda11 updated https://github.com/llvm/llvm-project/pull/113389
>From efcd62d35fe296b2bd4fe5cdbec9ab96493a885f Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Tue, 22 Oct 2024 14:01:45 -0700
Subject: [PATCH 1/2] [clang][modules] Preserve the module map that allowed
inferring
---
.../include/clang/Serialization/ASTBitCodes.h | 4 ++--
clang/include/clang/Serialization/ASTReader.h | 2 ++
clang/lib/Frontend/FrontendAction.cpp | 1 -
clang/lib/Lex/ModuleMap.cpp | 11 ++++-------
clang/lib/Serialization/ASTReader.cpp | 5 +++--
clang/lib/Serialization/ASTWriter.cpp | 7 +++++++
.../DependencyScanning/ModuleDepCollector.cpp | 18 +++++++-----------
clang/test/ClangScanDeps/link-libraries.c | 7 +++----
8 files changed, 28 insertions(+), 27 deletions(-)
diff --git a/clang/include/clang/Serialization/ASTBitCodes.h b/clang/include/clang/Serialization/ASTBitCodes.h
index e397dff097652b..9f50e28e94ff0c 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 81eea9c4c4dc58..a647e3e7ec6f64 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 fd6bc17ae9cdac..31617c7e86fddb 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;
@@ -1345,7 +1342,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 60b708067dc597..4fa41338024930 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -5826,6 +5826,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++];
@@ -5847,8 +5848,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])
@@ -5879,6 +5878,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 938d7b525cb959..5479ef5fe5a918 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,11 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
SourceLocationEncoding::RawLocEncoding DefinitionLoc =
getRawSourceLocationEncoding(getAdjustedLocation(Mod->DefinitionLoc));
+ ModuleMap &ModMap = PP->getHeaderSearchInfo().getModuleMap();
+ FileID InferredFID =
+ Mod->IsInferred ? ModMap.getModuleMapFileIDForUniquing(Mod) : FileID();
+ int Inferred = getAdjustedFileID(InferredFID).getOpaqueValue();
+
// Emit the definition of the block.
{
RecordData::value_type Record[] = {SUBMODULE_DEFINITION,
@@ -3025,6 +3031,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
ParentID,
(RecordData::value_type)Mod->Kind,
DefinitionLoc,
+ (RecordData::value_type)Inferred,
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: {
>From 22d2a219eb8985beb5ea9224a2b57b1df931af75 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 28 Oct 2024 11:22:23 -0700
Subject: [PATCH 2/2] Improve naming
---
clang/lib/Serialization/ASTWriter.cpp | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp
index 5479ef5fe5a918..24bbd7074f73b4 100644
--- a/clang/lib/Serialization/ASTWriter.cpp
+++ b/clang/lib/Serialization/ASTWriter.cpp
@@ -3020,9 +3020,10 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
getRawSourceLocationEncoding(getAdjustedLocation(Mod->DefinitionLoc));
ModuleMap &ModMap = PP->getHeaderSearchInfo().getModuleMap();
- FileID InferredFID =
- Mod->IsInferred ? ModMap.getModuleMapFileIDForUniquing(Mod) : FileID();
- int Inferred = getAdjustedFileID(InferredFID).getOpaqueValue();
+ FileID UnadjustedInferredFID;
+ if (Mod->IsInferred)
+ UnadjustedInferredFID = ModMap.getModuleMapFileIDForUniquing(Mod);
+ int InferredFID = getAdjustedFileID(UnadjustedInferredFID).getOpaqueValue();
// Emit the definition of the block.
{
@@ -3031,7 +3032,7 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) {
ParentID,
(RecordData::value_type)Mod->Kind,
DefinitionLoc,
- (RecordData::value_type)Inferred,
+ (RecordData::value_type)InferredFID,
Mod->IsFramework,
Mod->IsExplicit,
Mod->IsSystem,
More information about the cfe-commits
mailing list