[clang][deps] Only generate absolute paths when asked to

[jansvoboda11]

Summary

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.

[jansvoboda11]

Improve naming

[dexonsmith]

LGTM, with one nit to pick.

clang/tools/clang-scan-deps/ClangScanDeps.cpp

381

Please land this spelling correction separately.

[jansvoboda11]

Undo typo fix

clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h

struct FullDependencies {
   /// 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
   ///                      be located.
   /// \param LookupModuleDeps This function is called to collect the full
   ///                         transitive set of dependencies for this
-  ///                         compilation.
+  ///                         compilation and fill in `-fmodule-map-file=`
+  ///                         arguments.
   std::vector<std::string> getAdditionalCommandLine(
       std::function<StringRef(ModuleID)> LookupPCMPath,
       std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
+
+  /// Get additional arguments suitable for appending to the original Clang
+  /// command line except flags containing absolute paths: `-fmodule-file=`,
+  /// `-fmodule-map-file`.
+  std::vector<std::string> getAdditionalCommandLine() const;
 };
 
 struct FullDependenciesResult {
   FullDependencies FullDeps;
   std::vector<ModuleDeps> DiscoveredModules;
 };
 
 /// The high-level implementation of the dependency discovery tool that runs on

clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h

struct ModuleDeps {
   /// Gets the full 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
   ///                      be located.
   /// \param LookupModuleDeps This function is called to collect the full
   ///                         transitive set of dependencies for this
-  ///                         compilation.
+  ///                         compilation and fill in `-fmodule-map-file=`
+  ///                         arguments.
   std::vector<std::string> getFullCommandLine(
       std::function<StringRef(ModuleID)> LookupPCMPath,
       std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps) const;
+
+  /// Gets the full command line suitable for passing to clang except flags
+  /// containing absolute paths: `-fmodule-file=`, `-o`, `-fmodule-map-file`.
+  std::vector<std::string> getFullCommandLine() const;
 };
 
 namespace detail {
 /// Collect the paths of PCM and module map files for the modules in \c Modules
 /// transitively.
 void collectPCMAndModuleMapPaths(
     llvm::ArrayRef<ModuleID> Modules,
     std::function<StringRef(ModuleID)> LookupPCMPath,

clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp

std::vector<std::string> FullDependencies::getAdditionalCommandLine(
   for (const std::string &PCMPath : PCMPaths)
     Ret.push_back("-fmodule-file=" + PCMPath);
   for (const std::string &ModMapPath : ModMapPaths)
     Ret.push_back("-fmodule-map-file=" + ModMapPath);
 
   return Ret;
 }
 
+std::vector<std::string> FullDependencies::getAdditionalCommandLine() const {
+  return {
+      "-fno-implicit-modules",
+      "-fno-implicit-module-maps",
+  };
+}
+
 DependencyScanningTool::DependencyScanningTool(
     DependencyScanningService &Service)
     : Worker(Service) {}
 
 llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
     const tooling::CompilationDatabase &Compilations, StringRef CWD) {
   /// Prints out all of the gathered dependencies into a string.
   class MakeDependencyPrinterConsumer : public DependencyConsumer {

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

std::vector<std::string> ModuleDeps::getFullCommandLine(
 
   dependencies::detail::collectPCMAndModuleMapPaths(
       ClangModuleDeps, LookupPCMPath, LookupModuleDeps,
       CI.getFrontendOpts().ModuleFiles, CI.getFrontendOpts().ModuleMapFiles);
 
   return serializeCompilerInvocation(CI);
 }
 
+std::vector<std::string> ModuleDeps::getFullCommandLine() const {
+  CompilerInvocation CI(makeInvocationForModuleBuildWithoutPaths(*this));
+
+  return serializeCompilerInvocation(CI);
+}
+
 void dependencies::detail::collectPCMAndModuleMapPaths(
     llvm::ArrayRef<ModuleID> Modules,
     std::function<StringRef(ModuleID)> LookupPCMPath,
     std::function<const ModuleDeps &(ModuleID)> LookupModuleDeps,
     std::vector<std::string> &PCMPaths, std::vector<std::string> &ModMapPaths) {
   llvm::StringSet<> AlreadyAdded;
 
   std::function<void(llvm::ArrayRef<ModuleID>)> AddArgs =

clang/test/ClangScanDeps/modules-full.cpp

 // RUN: rm -rf %t.dir
 // RUN: rm -rf %t.cdb
 // RUN: mkdir -p %t.dir
 // RUN: cp %s %t.dir/modules_cdb_input.cpp
 // RUN: cp %s %t.dir/modules_cdb_input2.cpp
 // RUN: mkdir %t.dir/Inputs
 // RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
 // RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
 // RUN: cp %S/Inputs/module.modulemap %t.dir/Inputs/module.modulemap
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/modules_cdb.json > %t.cdb
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/modules_cdb_clangcl.json > %t_clangcl.cdb
 //
 // 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-absolute-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
 
 #include "header.h"
 
 // CHECK: [[PREFIX:.*]]
 // CHECK-NEXT: {
 // CHECK-NEXT:   "modules": [
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-module-deps": [
 // CHECK-NEXT:         {
 // CHECK-NEXT:           "context-hash": "[[CONTEXT_HASH_H1:[A-Z0-9]+]]",
 // CHECK-NEXT:           "module-name": "header2"
 // CHECK-NEXT:         }
 // 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": [
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/header.h",
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "header1"
 // CHECK-NEXT:     },

 // CHECK-NEXT:       "clang-context-hash": "[[CONTEXT_HASH_H2]]",
 // CHECK-NEXT:       "clang-module-deps": [
 // CHECK-NEXT:         {
 // CHECK-NEXT:           "context-hash": "[[CONTEXT_HASH_H2]]",
 // CHECK-NEXT:           "module-name": "header1"
 // 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"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
 // CHECK-NEXT:     },
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-context-hash": "[[CONTEXT_HASH_H2]]",
 // CHECK-NEXT:       "clang-module-deps": [
 // CHECK-NEXT:         {
 // CHECK-NEXT:           "context-hash": "[[CONTEXT_HASH_H2]]",
 // CHECK-NEXT:           "module-name": "header1"
 // 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"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
 // CHECK-NEXT:     },
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-context-hash": "[[CONTEXT_HASH_H2]]",
 // CHECK-NEXT:       "clang-module-deps": [
 // CHECK-NEXT:         {
 // CHECK-NEXT:           "context-hash": "[[CONTEXT_HASH_H2]]",
 // CHECK-NEXT:           "module-name": "header1"
 // 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"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
 // CHECK-NEXT:     },
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-context-hash": "[[CONTEXT_HASH_H1]]",
 // CHECK-NEXT:       "clang-module-deps": [
 // CHECK-NEXT:         {
 // CHECK-NEXT:           "context-hash": "[[CONTEXT_HASH_H1]]",
 // CHECK-NEXT:           "module-name": "header1"
 // 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"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "input-file": "[[PREFIX]]/modules_cdb_input2.cpp"
 // CHECK-NEXT:     }
 // CHECK-NEXT:   ]
 // CHECK-NEXT: }

clang/tools/clang-scan-deps/ClangScanDeps.cpp

llvm::cl::values(clEnumValN(ScanningOutputFormat::Make, "make",
                                 "Makefile compatible dep file"),
                      clEnumValN(ScanningOutputFormat::Full, "experimental-full",
                                 "Full dependency graph suitable"
                                 " for explicitly building modules. This format "
                                 "is experimental and will change.")),
     llvm::cl::init(ScanningOutputFormat::Make),
     llvm::cl::cat(DependencyScannerCategory));
 
+// Note: This is only useful when experimenting with explicit modules during
+//       development. Build tools are not expected to use the absolute paths
+//       generated in this mode, since they point into module cache. Instead,
+//       build tools are expected to customize the paths via the `LookupPCMPath`
+//       and `LookupModuleDeps` callbacks when using the C++ API, or append them
+//       to the command-line generated without absolute paths.
+static llvm::cl::opt<bool> GenerateAbsolutePathArgs(
+    "generate-absolute-path-args",
+    llvm::cl::desc(
+        "With '-format experimental-full', include arguments containing "
+        "absolute 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 "
                               "all concurrent threads)"),
                llvm::cl::init(0), llvm::cl::cat(DependencyScannerCategory));
 
 llvm::cl::opt<std::string>
     CompilationDB("compilation-database",
for (const ModuleDeps &MD : FDR.DiscoveredModules) {
       auto I = Modules.find({MD.ID, 0});
       if (I != Modules.end()) {
         I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
         continue;
       }
       Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
     }
 
-    ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
+    ID.AdditionalCommandLine =
+        GenerateAbsolutePathArgs
+            ? FD.getAdditionalCommandLine(
                   [&](ModuleID MID) { return lookupPCMPath(MID); },
                   [&](ModuleID MID) -> const ModuleDeps & {
                     return lookupModuleDeps(MID);
-        });
+                  })
+            : FD.getAdditionalCommandLine();
 
     Inputs.push_back(std::move(ID));
   }
 
   void printFullOutput(raw_ostream &OS) {
     // Sort the modules by name to get a deterministic order.
     std::vector<IndexedModuleID> ModuleIDs;
     for (auto &&M : Modules)
void printFullOutput(raw_ostream &OS) {
     for (auto &&ModID : ModuleIDs) {
       auto &MD = Modules[ModID];
       Object O{
           {"name", MD.ID.ModuleName},
           {"context-hash", MD.ID.ContextHash},
           {"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",
+           GenerateAbsolutePathArgs
+               ? MD.getFullCommandLine(
+                     })
+               : MD.getFullCommandLine()},
       };
       OutModules.push_back(std::move(O));
     }
 
     Array TUs;
     for (auto &&I : Inputs) {
       Object O{
           {"input-file", I.FileName},
           {"clang-context-hash", I.ContextHash},
           {"file-deps", I.FileDeps},
           {"clang-module-deps", toJSONSorted(I.ModuleDeps)},
-          {"command-line", I.AdditonalCommandLine},
+          {"command-line", I.AdditionalCommandLine},
       };
       TUs.push_back(std::move(O));
     }
 
     Object Output{
         {"modules", std::move(OutModules)},
         {"translation-units", std::move(TUs)},
     };
struct IndexedModuleIDHasher {
     }
   };
 
   struct InputDeps {
     std::string FileName;
     std::string ContextHash;
     std::vector<std::string> FileDeps;
     std::vector<ModuleID> ModuleDeps;
-    std::vector<std::string> AdditonalCommandLine;
+    std::vector<std::string> AdditionalCommandLine;
   };
 
   std::mutex Lock;
   std::unordered_map<IndexedModuleID, ModuleDeps, IndexedModuleIDHasher>
       Modules;
   std::vector<InputDeps> Inputs;
 };