[clang] 0f7d410 - [clang][deps] Only generate absolute paths when asked to

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 26 01:53:47 PDT 2021


Author: Jan Svoboda
Date: 2021-04-26T10:53:41+02:00
New Revision: 0f7d4105c60b5b4ee80fd32225658a4d8261c120

URL: https://github.com/llvm/llvm-project/commit/0f7d4105c60b5b4ee80fd32225658a4d8261c120
DIFF: https://github.com/llvm/llvm-project/commit/0f7d4105c60b5b4ee80fd32225658a4d8261c120.diff

LOG: [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

Added: 
    

Modified: 
    clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
    clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
    clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
    clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
    clang/test/ClangScanDeps/modules-full.cpp
    clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 
    


################################################################################
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<std::string> getAdditionalCommandLine(
+  ///                         compilation and fill in "-fmodule-map-file="
+  ///                         arguments.
+  std::vector<std::string> getAdditionalArgs(
       std::function<StringRef(ModuleID)> LookupPCMPath,
       std::function<const ModuleDeps &(ModuleID)> 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<std::string> 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<CompilerInvocation> 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<std::string> getFullCommandLine(
+  ///                         compilation and fill in "-fmodule-map-file="
+  ///                         arguments.
+  std::vector<std::string> getCanonicalCommandLine(
       std::function<StringRef(ModuleID)> LookupPCMPath,
       std::function<const ModuleDeps &(ModuleID)> 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<std::string> 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<std::string> FullDependencies::getAdditionalCommandLine(
+std::vector<std::string> FullDependencies::getAdditionalArgs(
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
   std::vector<std::string> Ret{
@@ -33,6 +33,14 @@ std::vector<std::string> FullDependencies::getAdditionalCommandLine(
   return Ret;
 }
 
+std::vector<std::string>
+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<std::string>{Args.begin(), Args.end()};
 }
 
-std::vector<std::string> ModuleDeps::getFullCommandLine(
+std::vector<std::string> ModuleDeps::getCanonicalCommandLine(
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const {
   CompilerInvocation CI(makeInvocationForModuleBuildWithoutPaths(*this));
@@ -64,6 +64,13 @@ std::vector<std::string> ModuleDeps::getFullCommandLine(
   return serializeCompilerInvocation(CI);
 }
 
+std::vector<std::string>
+ModuleDeps::getCanonicalCommandLineWithoutModulePaths() const {
+  CompilerInvocation CI(makeInvocationForModuleBuildWithoutPaths(*this));
+
+  return serializeCompilerInvocation(CI);
+}
+
 void dependencies::detail::collectPCMAndModuleMapPaths(
     llvm::ArrayRef<ModuleID> Modules,
     std::function<StringRef(ModuleID)> 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<ScanningOutputFormat> 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 
diff erent 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<bool> 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<unsigned>
     NumThreads("j", llvm::cl::Optional,
                llvm::cl::desc("Number of worker threads to use (default: use "
@@ -260,11 +285,14 @@ class FullDeps {
       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 @@ class FullDeps {
           {"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));
     }


        


More information about the cfe-commits mailing list