From 7f6af607464e5bcf2bd8b7bc7724a5985d7a0c34 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Sat, 12 Mar 2022 11:32:47 +0100 Subject: [PATCH] [clang][deps] Generate '-fmodule-file=' only for direct dependencies The `clang-scan-deps` tool currently generates `-fmodule-file=` command-line arguments for the whole transitive closure of modular dependencies. This is not necessary, we only need to provide the direct dependencies on the command line. Information about transitive dependencies is stored within the `.pcm` files of direct dependencies. This makes the command lines shorter, but should be a NFC otherwise (unless there are bugs in the loading mechanism for explicit modules). Depends on D120465. Reviewed By: Bigcheese Differential Revision: https://reviews.llvm.org/D118915 --- .../DependencyScanning/DependencyScanningTool.h | 8 ++---- .../DependencyScanning/ModuleDepCollector.h | 15 +---------- .../DependencyScanning/DependencyScanningTool.cpp | 10 +++----- .../DependencyScanning/ModuleDepCollector.cpp | 30 +++------------------- clang/test/ClangScanDeps/modules-full.cpp | 2 -- clang/test/ClangScanDeps/modules-pch.c | 1 - clang/tools/clang-scan-deps/ClangScanDeps.cpp | 24 ++++++----------- 7 files changed, 17 insertions(+), 73 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h index 36447dd..aee4dde 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h @@ -51,12 +51,8 @@ struct FullDependencies { /// arguments and the "-o" argument. It needs to return /// a path for where the PCM for the given module is to /// be located. - /// \param LookupModuleDeps This function is called to collect the full - /// transitive set of dependencies for this - /// compilation. - std::vector getCommandLine( - std::function LookupPCMPath, - std::function LookupModuleDeps) const; + std::vector + getCommandLine(std::function LookupPCMPath) const; /// Get the full command line, excluding -fmodule-file=" arguments. std::vector getCommandLineWithoutModulePaths() const; diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index d0fa474..7936f9c 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -113,27 +113,14 @@ struct ModuleDeps { /// arguments and the "-o" argument. It needs to return /// a path for where the PCM for the given module is to /// be located. - /// \param LookupModuleDeps This function is called to collect the full - /// transitive set of dependencies for this - /// compilation. std::vector getCanonicalCommandLine( - std::function LookupPCMPath, - std::function LookupModuleDeps) const; + std::function LookupPCMPath) const; /// Gets the canonical command line suitable for passing to clang, excluding /// "-fmodule-file=" and "-o" arguments. std::vector getCanonicalCommandLineWithoutModulePaths() const; }; -namespace detail { -/// Collect the paths of PCM for the modules in \c Modules transitively. -void collectPCMPaths( - llvm::ArrayRef Modules, - std::function LookupPCMPath, - std::function LookupModuleDeps, - std::vector &PCMPaths); -} // namespace detail - class ModuleDepCollector; /// Callback that records textual includes and direct modular includes/imports diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp index 12602fc..06d39c2 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp @@ -14,15 +14,11 @@ namespace tooling { namespace dependencies { std::vector FullDependencies::getCommandLine( - std::function LookupPCMPath, - std::function LookupModuleDeps) const { + std::function LookupPCMPath) const { std::vector Ret = getCommandLineWithoutModulePaths(); - std::vector PCMPaths; - dependencies::detail::collectPCMPaths(ClangModuleDeps, LookupPCMPath, - LookupModuleDeps, PCMPaths); - for (const std::string &PCMPath : PCMPaths) - Ret.push_back("-fmodule-file=" + PCMPath); + for (ModuleID MID : ClangModuleDeps) + Ret.push_back(("-fmodule-file=" + LookupPCMPath(MID)).str()); return Ret; } diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index 0b77ba8..ed18a11 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -87,8 +87,7 @@ serializeCompilerInvocation(const CompilerInvocation &CI) { } std::vector ModuleDeps::getCanonicalCommandLine( - std::function LookupPCMPath, - std::function LookupModuleDeps) const { + std::function LookupPCMPath) const { CompilerInvocation CI(BuildInvocation); FrontendOptions &FrontendOpts = CI.getFrontendOpts(); @@ -97,9 +96,8 @@ std::vector ModuleDeps::getCanonicalCommandLine( FrontendOpts.Inputs.emplace_back(ClangModuleMapFile, ModuleMapInputKind); FrontendOpts.OutputFile = std::string(LookupPCMPath(ID)); - dependencies::detail::collectPCMPaths(ClangModuleDeps, LookupPCMPath, - LookupModuleDeps, - FrontendOpts.ModuleFiles); + for (ModuleID MID : ClangModuleDeps) + FrontendOpts.ModuleFiles.emplace_back(LookupPCMPath(MID)); return serializeCompilerInvocation(CI); } @@ -109,28 +107,6 @@ ModuleDeps::getCanonicalCommandLineWithoutModulePaths() const { return serializeCompilerInvocation(BuildInvocation); } -void dependencies::detail::collectPCMPaths( - llvm::ArrayRef Modules, - std::function LookupPCMPath, - std::function LookupModuleDeps, - std::vector &PCMPaths) { - llvm::StringSet<> AlreadyAdded; - - std::function)> AddArgs = - [&](llvm::ArrayRef Modules) { - for (const ModuleID &MID : Modules) { - if (!AlreadyAdded.insert(MID.ModuleName + MID.ContextHash).second) - continue; - const ModuleDeps &M = LookupModuleDeps(MID); - // Depth first traversal. - AddArgs(M.ClangModuleDeps); - PCMPaths.push_back(LookupPCMPath(MID).str()); - } - }; - - AddArgs(Modules); -} - void ModuleDepCollectorPP::FileChanged(SourceLocation Loc, FileChangeReason Reason, SrcMgr::CharacteristicKind FileType, diff --git a/clang/test/ClangScanDeps/modules-full.cpp b/clang/test/ClangScanDeps/modules-full.cpp index 9802f36..1881053 100644 --- a/clang/test/ClangScanDeps/modules-full.cpp +++ b/clang/test/ClangScanDeps/modules-full.cpp @@ -169,9 +169,7 @@ // CHECK: "-fno-implicit-modules" // CHECK-NEXT: "-fno-implicit-module-maps" // CHECK-NO-ABS-NOT: "-fmodule-file={{.*}}" -// CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm" // CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1_DINCLUDE]]/header1-{{[A-Z0-9]+}}.pcm" -// CHECK-CUSTOM-NEXT: "-fmodule-file=[[PREFIX]]/custom/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm" // CHECK-CUSTOM-NEXT: "-fmodule-file=[[PREFIX]]/custom/[[HASH_H1_DINCLUDE]]/header1-{{[A-Z0-9]+}}.pcm" // CHECK-NEXT: ], // CHECK-NEXT: "file-deps": [ diff --git a/clang/test/ClangScanDeps/modules-pch.c b/clang/test/ClangScanDeps/modules-pch.c index 9ac43c1..7258d36 100644 --- a/clang/test/ClangScanDeps/modules-pch.c +++ b/clang/test/ClangScanDeps/modules-pch.c @@ -97,7 +97,6 @@ // CHECK-PCH: "-fno-implicit-modules", // CHECK-PCH-NEXT: "-fno-implicit-module-maps", // CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_1]]/ModCommon1-{{.*}}.pcm", -// CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm", // CHECK-PCH-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_PCH]]/ModPCH-{{.*}}.pcm" // CHECK-PCH-NEXT: ], // CHECK-PCH-NEXT: "file-deps": [ diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index e88290d..8d70808 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -298,10 +298,7 @@ public: ID.CommandLine = GenerateModulesPathArgs ? FD.getCommandLine( - [&](ModuleID MID) { return lookupPCMPath(MID); }, - [&](ModuleID MID) -> const ModuleDeps & { - return lookupModuleDeps(MID); - }) + [&](ModuleID MID) { return lookupPCMPath(MID); }) : FD.getCommandLineWithoutModulePaths(); Inputs.push_back(std::move(ID)); @@ -336,10 +333,7 @@ public: {"command-line", GenerateModulesPathArgs ? MD.getCanonicalCommandLine( - [&](ModuleID MID) { return lookupPCMPath(MID); }, - [&](ModuleID MID) -> const ModuleDeps & { - return lookupModuleDeps(MID); - }) + [&](ModuleID MID) { return lookupPCMPath(MID); }) : MD.getCanonicalCommandLineWithoutModulePaths()}, }; OutModules.push_back(std::move(O)); @@ -369,12 +363,16 @@ private: StringRef lookupPCMPath(ModuleID MID) { auto PCMPath = PCMPaths.insert({MID, ""}); if (PCMPath.second) - PCMPath.first->second = constructPCMPath(lookupModuleDeps(MID)); + PCMPath.first->second = constructPCMPath(MID); return PCMPath.first->second; } /// Construct a path for the explicitly built PCM. - std::string constructPCMPath(const ModuleDeps &MD) const { + std::string constructPCMPath(ModuleID MID) const { + auto MDIt = Modules.find(IndexedModuleID{MID, 0}); + assert(MDIt != Modules.end()); + const ModuleDeps &MD = MDIt->second; + StringRef Filename = llvm::sys::path::filename(MD.ImplicitModulePCMPath); SmallString<256> ExplicitPCMPath( @@ -385,12 +383,6 @@ private: return std::string(ExplicitPCMPath); } - const ModuleDeps &lookupModuleDeps(ModuleID MID) { - auto I = Modules.find(IndexedModuleID{MID, 0}); - assert(I != Modules.end()); - return I->second; - }; - struct IndexedModuleID { ModuleID ID; mutable size_t InputIndex; -- 2.7.4