Skip to content

[clang][modules] Preserve the module map that allowed inferring #113389

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Oct 28, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions clang/include/clang/Serialization/ASTBitCodes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
///
Expand Down
2 changes: 2 additions & 0 deletions clang/include/clang/Serialization/ASTReader.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
1 change: 0 additions & 1 deletion clang/lib/Frontend/FrontendAction.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
11 changes: 4 additions & 7 deletions clang/lib/Lex/ModuleMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}

Expand Down
5 changes: 3 additions & 2 deletions clang/lib/Serialization/ASTReader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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++];
Expand All @@ -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])
Expand Down Expand Up @@ -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;
Expand Down
8 changes: 8 additions & 0 deletions clang/lib/Serialization/ASTWriter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -3018,13 +3019,20 @@ 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,
ID,
ParentID,
(RecordData::value_type)Mod->Kind,
DefinitionLoc,
(RecordData::value_type)InferredFID,
Mod->IsFramework,
Mod->IsExplicit,
Mod->IsSystem,
Expand Down
18 changes: 7 additions & 11 deletions clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
});

Expand Down
7 changes: 3 additions & 4 deletions clang/test/ClangScanDeps/link-libraries.c
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down
Loading