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_svob...@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_svob...@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, _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits