From 0f7d4105c60b5b4ee80fd32225658a4d8261c120 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Thu, 22 Apr 2021 13:49:25 +0200 Subject: [PATCH] [clang][deps] Only generate absolute paths when asked to Add option to `clang-scan-deps` to enable/disable generation of command-line arguments with absolute paths. This is essentially a revert of D100533, but with improved naming and added test. Reviewed By: dexonsmith Differential Revision: https://reviews.llvm.org/D101051 --- .../DependencyScanningTool.h | 16 +++-- .../DependencyScanning/ModuleDepCollector.h | 18 +++-- .../DependencyScanningTool.cpp | 10 ++- .../DependencyScanning/ModuleDepCollector.cpp | 9 ++- clang/test/ClangScanDeps/modules-full.cpp | 71 +++++++++++-------- clang/tools/clang-scan-deps/ClangScanDeps.cpp | 51 ++++++++++--- 6 files changed, 124 insertions(+), 51 deletions(-) diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h index 33e155f4b4ae..89a70fb723c4 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h @@ -41,16 +41,22 @@ struct FullDependencies { /// Get additional arguments suitable for appending to the original Clang /// command line. /// - /// \param LookupPCMPath This function is called to fill in `-fmodule-file=` - /// flags and for the `-o` flag. It needs to return a - /// path for where the PCM for the given module is to + /// \param LookupPCMPath This function is called to fill in "-fmodule-file=" + /// 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 getAdditionalCommandLine( + /// compilation and fill in "-fmodule-map-file=" + /// arguments. + std::vector getAdditionalArgs( std::function LookupPCMPath, std::function LookupModuleDeps) const; + + /// Get additional arguments suitable for appending to the original Clang + /// command line, excluding arguments containing modules-related paths: + /// "-fmodule-file=", "-fmodule-map-file=". + std::vector getAdditionalArgsWithoutModulePaths() const; }; struct FullDependenciesResult { diff --git a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h index 5e90fb2f7b09..95876bb665f1 100644 --- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h +++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h @@ -79,18 +79,24 @@ struct ModuleDeps { /// this module. std::shared_ptr Invocation; - /// Gets the full command line suitable for passing to clang. + /// Gets the canonical command line suitable for passing to clang. /// - /// \param LookupPCMPath This function is called to fill in `-fmodule-file=` - /// flags and for the `-o` flag. It needs to return a - /// path for where the PCM for the given module is to + /// \param LookupPCMPath This function is called to fill in "-fmodule-file=" + /// 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 getFullCommandLine( + /// compilation and fill in "-fmodule-map-file=" + /// arguments. + std::vector getCanonicalCommandLine( std::function LookupPCMPath, std::function LookupModuleDeps) const; + + /// Gets the canonical command line suitable for passing to clang, excluding + /// arguments containing modules-related paths: "-fmodule-file=", "-o", + /// "-fmodule-map-file=". + std::vector getCanonicalCommandLineWithoutModulePaths() const; }; namespace detail { diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp index a59ab688b10b..35817d3f7ae9 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp @@ -13,7 +13,7 @@ namespace clang{ namespace tooling{ namespace dependencies{ -std::vector FullDependencies::getAdditionalCommandLine( +std::vector FullDependencies::getAdditionalArgs( std::function LookupPCMPath, std::function LookupModuleDeps) const { std::vector Ret{ @@ -33,6 +33,14 @@ std::vector FullDependencies::getAdditionalCommandLine( return Ret; } +std::vector +FullDependencies::getAdditionalArgsWithoutModulePaths() const { + return { + "-fno-implicit-modules", + "-fno-implicit-module-maps", + }; +} + DependencyScanningTool::DependencyScanningTool( DependencyScanningService &Service) : Worker(Service) {} diff --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp index a1e54532f3e6..7c858178cdd5 100644 --- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp +++ b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp @@ -52,7 +52,7 @@ serializeCompilerInvocation(CompilerInvocation &CI) { return std::vector{Args.begin(), Args.end()}; } -std::vector ModuleDeps::getFullCommandLine( +std::vector ModuleDeps::getCanonicalCommandLine( std::function LookupPCMPath, std::function LookupModuleDeps) const { CompilerInvocation CI(makeInvocationForModuleBuildWithoutPaths(*this)); @@ -64,6 +64,13 @@ std::vector ModuleDeps::getFullCommandLine( return serializeCompilerInvocation(CI); } +std::vector +ModuleDeps::getCanonicalCommandLineWithoutModulePaths() const { + CompilerInvocation CI(makeInvocationForModuleBuildWithoutPaths(*this)); + + return serializeCompilerInvocation(CI); +} + void dependencies::detail::collectPCMAndModuleMapPaths( llvm::ArrayRef Modules, std::function LookupPCMPath, diff --git a/clang/test/ClangScanDeps/modules-full.cpp b/clang/test/ClangScanDeps/modules-full.cpp index b4252c5c4f60..ebd86e79a3fb 100644 --- a/clang/test/ClangScanDeps/modules-full.cpp +++ b/clang/test/ClangScanDeps/modules-full.cpp @@ -13,12 +13,17 @@ // RUN: echo %t.dir > %t.result // RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format experimental-full \ // RUN: -mode preprocess-minimized-sources >> %t.result -// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK %s - +// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK,CHECK-NO-ABS %s +// +// RUN: echo %t.dir > %t.result +// RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format experimental-full \ +// RUN: -generate-modules-path-args -mode preprocess-minimized-sources >> %t.result +// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK,CHECK-ABS %s +// // RUN: echo %t.dir > %t_clangcl.result // RUN: clang-scan-deps -compilation-database %t_clangcl.cdb -j 4 -format experimental-full \ // RUN: -mode preprocess-minimized-sources >> %t_clangcl.result -// RUN: cat %t_clangcl.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK %s +// RUN: cat %t_clangcl.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK,CHECK-NO-ABS %s // FIXME: Backslash issues. // XFAIL: system-windows @@ -37,13 +42,15 @@ // CHECK-NEXT: ], // CHECK-NEXT: "clang-modulemap-file": "[[PREFIX]]/Inputs/module.modulemap", // CHECK-NEXT: "command-line": [ -// CHECK-NEXT: "-cc1", -// CHECK: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap", -// CHECK: "-emit-module", -// CHECK: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm", -// CHECK-NOT: "-fimplicit-module-maps", -// CHECK: "-fmodule-name=header1", -// CHECK: "-fno-implicit-modules", +// CHECK-NEXT: "-cc1" +// CHECK-NO-ABS-NOT: "-fmodule-map-file={{.*}}" +// CHECK-ABS: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" +// CHECK: "-emit-module" +// CHECK-NO-ABS-NOT: "-fmodule-file={{.*}}" +// CHECK-ABS: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm" +// CHECK-NOT: "-fimplicit-module-maps" +// CHECK: "-fmodule-name=header1" +// CHECK: "-fno-implicit-modules" // CHECK: ], // CHECK-NEXT: "context-hash": "[[CONTEXT_HASH_H1]]", // CHECK-NEXT: "file-deps": [ @@ -97,10 +104,12 @@ // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "command-line": [ -// CHECK-NEXT: "-fno-implicit-modules", -// CHECK-NEXT: "-fno-implicit-module-maps", -// CHECK-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm", -// CHECK-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" +// CHECK-NEXT: "-fno-implicit-modules" +// CHECK-NEXT: "-fno-implicit-module-maps" +// CHECK-NO-ABS-NOT: "-fmodule-file={{.*}}" +// CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm" +// CHECK-NO-ABS-NOT: "-fmodule-map-file={{.*}}" +// CHECK-ABS-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "file-deps": [ // CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp" @@ -116,10 +125,12 @@ // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "command-line": [ -// CHECK-NEXT: "-fno-implicit-modules", -// CHECK-NEXT: "-fno-implicit-module-maps", -// CHECK-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm", -// CHECK-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" +// CHECK-NEXT: "-fno-implicit-modules" +// CHECK-NEXT: "-fno-implicit-module-maps" +// CHECK-NO-ABS-NOT: "-fmodule-file={{.*}}, +// CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm" +// CHECK-NO-ABS-NOT: "-fmodule-map-file={{.*}} +// CHECK-ABS-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "file-deps": [ // CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp" @@ -135,10 +146,12 @@ // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "command-line": [ -// CHECK-NEXT: "-fno-implicit-modules", -// CHECK-NEXT: "-fno-implicit-module-maps", -// CHECK-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm", -// CHECK-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" +// CHECK-NEXT: "-fno-implicit-modules" +// CHECK-NEXT: "-fno-implicit-module-maps" +// CHECK-NO-ABS-NOT: "-fmodule-file={{.*}}" +// CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm" +// CHECK-NO-ABS-NOT: "-fmodule-map-file={{.*}}" +// CHECK-ABS-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "file-deps": [ // CHECK-NEXT: "[[PREFIX]]/modules_cdb_input.cpp" @@ -154,12 +167,14 @@ // CHECK-NEXT: } // CHECK-NEXT: ], // CHECK-NEXT: "command-line": [ -// CHECK-NEXT: "-fno-implicit-modules", -// CHECK-NEXT: "-fno-implicit-module-maps", -// CHECK-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm", -// CHECK-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm", -// CHECK-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap", -// CHECK-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" +// CHECK-NEXT: "-fno-implicit-modules" +// CHECK-NEXT: "-fno-implicit-module-maps" +// CHECK-NO-ABS-NOT: "-fmodule-file={{.*}}" +// CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm" +// CHECK-ABS-NEXT: "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm" +// CHECK-NO-ABS-NOT: "-fmodule-map-file={{.*}}" +// CHECK-ABS-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" +// CHECK-ABS-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap" // CHECK-NEXT: ], // CHECK-NEXT: "file-deps": [ // CHECK-NEXT: "[[PREFIX]]/modules_cdb_input2.cpp" diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index 6d5ab77f4409..a3844e569ccc 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -138,6 +138,31 @@ static llvm::cl::opt Format( llvm::cl::init(ScanningOutputFormat::Make), llvm::cl::cat(DependencyScannerCategory)); +// This mode is mostly useful for development of explicitly built modules. +// Command lines will contain arguments specifying modulemap file paths and +// absolute paths to PCM files in the module cache directory. +// +// Build tools that want to put the PCM files in a different location should use +// the C++ APIs instead, of which there are two flavors: +// +// 1. APIs that generate arguments with paths to modulemap and PCM files via +// callbacks provided by the client: +// * ModuleDeps::getCanonicalCommandLine(LookupPCMPath, LookupModuleDeps) +// * FullDependencies::getAdditionalArgs(LookupPCMPath, LookupModuleDeps) +// +// 2. APIs that don't generate arguments with paths to modulemap or PCM files +// and instead expect the client to append them manually after the fact: +// * ModuleDeps::getCanonicalCommandLineWithoutModulePaths() +// * FullDependencies::getAdditionalArgsWithoutModulePaths() +// +static llvm::cl::opt GenerateModulesPathArgs( + "generate-modules-path-args", + llvm::cl::desc( + "With '-format experimental-full', include arguments specifying " + "modules-related paths in the generated command lines: " + "'-fmodule-file=', '-o', '-fmodule-map-file='."), + llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory)); + llvm::cl::opt NumThreads("j", llvm::cl::Optional, llvm::cl::desc("Number of worker threads to use (default: use " @@ -260,11 +285,14 @@ public: Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)}); } - ID.AdditonalCommandLine = FD.getAdditionalCommandLine( - [&](ModuleID MID) { return lookupPCMPath(MID); }, - [&](ModuleID MID) -> const ModuleDeps & { - return lookupModuleDeps(MID); - }); + ID.AdditonalCommandLine = + GenerateModulesPathArgs + ? FD.getAdditionalArgs( + [&](ModuleID MID) { return lookupPCMPath(MID); }, + [&](ModuleID MID) -> const ModuleDeps & { + return lookupModuleDeps(MID); + }) + : FD.getAdditionalArgsWithoutModulePaths(); Inputs.push_back(std::move(ID)); } @@ -295,11 +323,14 @@ public: {"file-deps", toJSONSorted(MD.FileDeps)}, {"clang-module-deps", toJSONSorted(MD.ClangModuleDeps)}, {"clang-modulemap-file", MD.ClangModuleMapFile}, - {"command-line", MD.getFullCommandLine( - [&](ModuleID MID) { return lookupPCMPath(MID); }, - [&](ModuleID MID) -> const ModuleDeps & { - return lookupModuleDeps(MID); - })}, + {"command-line", + GenerateModulesPathArgs + ? MD.getCanonicalCommandLine( + [&](ModuleID MID) { return lookupPCMPath(MID); }, + [&](ModuleID MID) -> const ModuleDeps & { + return lookupModuleDeps(MID); + }) + : MD.getCanonicalCommandLineWithoutModulePaths()}, }; OutModules.push_back(std::move(O)); }