benlangmuir updated this revision to Diff 443379.
benlangmuir added a comment.

Updates:

- Made lookup of module outputs fallible. Not currently used by 
clang-scan-deps, but since the expectation is for a build system to provide 
these settings account for possibility of errors.
- Attempt to fix windows path issue in test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D129389/new/

https://reviews.llvm.org/D129389

Files:
  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/Inputs/removed-args/cdb.json.template
  clang/test/ClangScanDeps/generate-modules-path-args.c
  clang/test/ClangScanDeps/preserved-args.c
  clang/test/ClangScanDeps/removed-args.c
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===================================================================
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -288,11 +288,16 @@
       Modules.insert(I, {{MD.ID, InputIndex}, std::move(MD)});
     }
 
-    ID.CommandLine = GenerateModulesPathArgs
-                         ? FD.getCommandLine(
-                               [&](ModuleID MID) { return lookupPCMPath(MID); })
-                         : FD.getCommandLineWithoutModulePaths();
-
+    if (Inputs.size() == 0)
+      inferOutputOptions(FD.OriginalCommandLine);
+
+    ID.CommandLine =
+        GenerateModulesPathArgs
+            ? llvm::cantFail(FD.getCommandLine(
+                  [&](ModuleID MID) -> const ModuleOutputOptions & {
+                    return lookupModuleOutputs(MID);
+                  }))
+            : FD.getCommandLineWithoutModulePaths();
     Inputs.push_back(std::move(ID));
   }
 
@@ -324,8 +329,10 @@
           {"clang-modulemap-file", MD.ClangModuleMapFile},
           {"command-line",
            GenerateModulesPathArgs
-               ? MD.getCanonicalCommandLine(
-                     [&](ModuleID MID) { return lookupPCMPath(MID); })
+               ? llvm::cantFail(MD.getCanonicalCommandLine(
+                     [&](ModuleID MID) -> const ModuleOutputOptions & {
+                       return lookupModuleOutputs(MID);
+                     }))
                : MD.getCanonicalCommandLineWithoutModulePaths()},
       };
       OutModules.push_back(std::move(O));
@@ -352,11 +359,17 @@
   }
 
 private:
-  StringRef lookupPCMPath(ModuleID MID) {
-    auto PCMPath = PCMPaths.insert({MID, ""});
-    if (PCMPath.second)
-      PCMPath.first->second = constructPCMPath(MID);
-    return PCMPath.first->second;
+  const ModuleOutputOptions &lookupModuleOutputs(const ModuleID &MID) {
+    auto MO = ModuleOutputs.insert({MID, {}});
+    if (MO.second) {
+      ModuleOutputOptions &Opts = MO.first->second;
+      Opts.ModuleFile = constructPCMPath(MID);
+      if (DependencyOutputFile)
+        Opts.DependencyFile = Opts.ModuleFile + ".d";
+      if (SerializeDiags)
+        Opts.DiagnosticSerializationFile = Opts.ModuleFile + ".diag";
+    }
+    return MO.first->second;
   }
 
   /// Construct a path for the explicitly built PCM.
@@ -375,6 +388,19 @@
     return std::string(ExplicitPCMPath);
   }
 
+  /// Infer whether modules should write serialized diagnostic, .d, etc.
+  ///
+  /// A build system should model this directly, but here we infer it from an
+  /// original TU command.
+  void inferOutputOptions(ArrayRef<std::string> Args) {
+    for (StringRef Arg : Args) {
+      if (Arg == "-serialize-diagnostics")
+        SerializeDiags = true;
+      else if (Arg == "-M" || Arg == "-MM" || Arg == "-MMD" || Arg == "-MD")
+        DependencyOutputFile = true;
+    }
+  }
+
   struct IndexedModuleID {
     ModuleID ID;
     mutable size_t InputIndex;
@@ -404,8 +430,11 @@
   std::mutex Lock;
   std::unordered_map<IndexedModuleID, ModuleDeps, IndexedModuleIDHasher>
       Modules;
-  std::unordered_map<ModuleID, std::string, ModuleIDHasher> PCMPaths;
+  std::unordered_map<ModuleID, ModuleOutputOptions, ModuleIDHasher>
+      ModuleOutputs;
   std::vector<InputDeps> Inputs;
+  bool SerializeDiags = false;
+  bool DependencyOutputFile = false;
 };
 
 static bool handleFullDependencyToolResult(
Index: clang/test/ClangScanDeps/removed-args.c
===================================================================
--- clang/test/ClangScanDeps/removed-args.c
+++ clang/test/ClangScanDeps/removed-args.c
@@ -29,6 +29,9 @@
 // CHECK-NOT:          "-fbuild-session-timestamp=
 // CHECK-NOT:          "-fmodules-prune-interval=
 // CHECK-NOT:          "-fmodules-prune-after=
+// CHECK-NOT:          "-dependency-file"
+// CHECK-NOT:          "-MT"
+// CHECK-NOT:          "-serialize-diagnostic-file"
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[HASH_MOD_HEADER:.*]]",
 // CHECK-NEXT:       "file-deps": [
@@ -50,6 +53,9 @@
 // CHECK-NOT:          "-fbuild-session-timestamp=
 // CHECK-NOT:          "-fmodules-prune-interval=
 // CHECK-NOT:          "-fmodules-prune-after=
+// CHECK-NOT:          "-dependency-file"
+// CHECK-NOT:          "-MT"
+// CHECK-NOT:          "-serialize-diagnostic-file"
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[HASH_MOD_TU:.*]]",
 // CHECK-NEXT:       "file-deps": [
Index: clang/test/ClangScanDeps/preserved-args.c
===================================================================
--- clang/test/ClangScanDeps/preserved-args.c
+++ clang/test/ClangScanDeps/preserved-args.c
@@ -10,13 +10,7 @@
 // CHECK-NEXT:     {
 // CHECK:            "command-line": [
 // CHECK-NEXT:         "-cc1"
-// CHECK:              "-serialize-diagnostic-file"
-// CHECK-NEXT:         "[[PREFIX]]/tu.dia"
 // CHECK:              "-fmodule-file=Foo=[[PREFIX]]/foo.pcm"
-// CHECK:              "-MT"
-// CHECK-NEXT:         "my_target"
-// CHECK:              "-dependency-file"
-// CHECK-NEXT:         "[[PREFIX]]/tu.d"
 // CHECK:            ],
 // CHECK:            "name": "Mod"
 // CHECK-NEXT:     }
Index: clang/test/ClangScanDeps/generate-modules-path-args.c
===================================================================
--- /dev/null
+++ clang/test/ClangScanDeps/generate-modules-path-args.c
@@ -0,0 +1,52 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: sed "s|DIR|%/t|g" %t/cdb_without.json.template > %t/cdb_without.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json \
+// RUN:   -format experimental-full -generate-modules-path-args > %t/deps.json
+// RUN: cat %t/deps.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
+// RUN: clang-scan-deps -compilation-database %t/cdb_without.json \
+// RUN:   -format experimental-full -generate-modules-path-args > %t/deps_without.json
+// RUN: cat %t/deps_without.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t -check-prefix=WITHOUT %s
+
+// CHECK:      {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT:     {
+// CHECK:            "command-line": [
+// CHECK-NEXT:         "-cc1"
+// CHECK:              "-serialize-diagnostic-file"
+// CHECK-NEXT:         "[[PREFIX]]{{.*}}Mod{{.*}}.diag"
+// CHECK:              "-dependency-file"
+// CHECK-NEXT:         "[[PREFIX]]{{.*}}Mod{{.*}}.d"
+// CHECK:            ],
+
+// WITHOUT:      {
+// WITHOUT-NEXT:   "modules": [
+// WITHOUT-NEXT:     {
+// WITHOUT:            "command-line": [
+// WITHOUT-NEXT:         "-cc1"
+// WITHOUT-NOT:          "-serialize-diagnostic-file"
+// WITHOUT-NOT:          "-dependency-file"
+// WITHOUT:            ],
+
+//--- cdb.json.template
+[{
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -serialize-diagnostics DIR/tu.diag -MD -MT tu -MF DIR/tu.d",
+  "file": "DIR/tu.c"
+}]
+
+//--- cdb_without.json.template
+[{
+  "directory": "DIR",
+  "command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache",
+  "file": "DIR/tu.c"
+}]
+
+//--- module.modulemap
+module Mod { header "Mod.h" }
+
+//--- Mod.h
+
+//--- tu.c
+#include "Mod.h"
Index: clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template
===================================================================
--- clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template
+++ clang/test/ClangScanDeps/Inputs/removed-args/cdb.json.template
@@ -1,7 +1,7 @@
 [
   {
     "directory": "DIR",
-    "command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-validate-once-per-build-session -fbuild-session-file=DIR/build-session -fmodules-prune-interval=123 -fmodules-prune-after=123 -fmodules-cache-path=DIR/cache -include DIR/header.h -grecord-command-line -o DIR/tu.o",
+    "command": "clang -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-validate-once-per-build-session -fbuild-session-file=DIR/build-session -fmodules-prune-interval=123 -fmodules-prune-after=123 -fmodules-cache-path=DIR/cache -include DIR/header.h -grecord-command-line -o DIR/tu.o -serialize-diagnostics DIR/tu.diag -MT tu -MD -MF DIR/tu.d",
     "file": "DIR/tu.c"
   }
 ]
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -56,6 +56,9 @@
   CI.getFrontendOpts().OutputFile.clear();
   CI.getCodeGenOpts().MainFileName.clear();
   CI.getCodeGenOpts().DwarfDebugFlags.clear();
+  CI.getDiagnosticOpts().DiagnosticSerializationFile.clear();
+  CI.getDependencyOutputOpts().OutputFile.clear();
+  CI.getDependencyOutputOpts().Targets.clear();
 
   CI.getFrontendOpts().ProgramAction = frontend::GenerateModule;
   CI.getLangOpts()->ModuleName = Deps.ID.ModuleName;
@@ -107,18 +110,33 @@
   return std::vector<std::string>{Args.begin(), Args.end()};
 }
 
-std::vector<std::string> ModuleDeps::getCanonicalCommandLine(
-    std::function<StringRef(ModuleID)> LookupPCMPath) const {
+Expected<std::vector<std::string>> ModuleDeps::getCanonicalCommandLine(
+    llvm::function_ref<Expected<const ModuleOutputOptions &>(const ModuleID &)>
+        LookupModuleOutputs) const {
   CompilerInvocation CI(BuildInvocation);
   FrontendOptions &FrontendOpts = CI.getFrontendOpts();
+  auto MO = LookupModuleOutputs(ID);
+  if (!MO)
+    return MO.takeError();
 
   InputKind ModuleMapInputKind(FrontendOpts.DashX.getLanguage(),
                                InputKind::Format::ModuleMap);
   FrontendOpts.Inputs.emplace_back(ClangModuleMapFile, ModuleMapInputKind);
-  FrontendOpts.OutputFile = std::string(LookupPCMPath(ID));
-
-  for (ModuleID MID : ClangModuleDeps)
-    FrontendOpts.ModuleFiles.emplace_back(LookupPCMPath(MID));
+  FrontendOpts.OutputFile = MO->ModuleFile;
+  if (MO->DiagnosticSerializationFile)
+    CI.getDiagnosticOpts().DiagnosticSerializationFile =
+        *MO->DiagnosticSerializationFile;
+  if (MO->DependencyFile)
+    CI.getDependencyOutputOpts().OutputFile = *MO->DependencyFile;
+  if (MO->DependencyTargets)
+    CI.getDependencyOutputOpts().Targets = *MO->DependencyTargets;
+
+  for (ModuleID MID : ClangModuleDeps) {
+    auto MO = LookupModuleOutputs(MID);
+    if (!MO)
+      return MO.takeError();
+    FrontendOpts.ModuleFiles.emplace_back(MO->ModuleFile);
+  }
 
   return serializeCompilerInvocation(CI);
 }
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -13,12 +13,17 @@
 namespace tooling {
 namespace dependencies {
 
-std::vector<std::string> FullDependencies::getCommandLine(
-    std::function<StringRef(ModuleID)> LookupPCMPath) const {
+Expected<std::vector<std::string>> FullDependencies::getCommandLine(
+    llvm::function_ref<Expected<const ModuleOutputOptions &>(const ModuleID &)>
+        LookupModuleOutputs) const {
   std::vector<std::string> Ret = getCommandLineWithoutModulePaths();
 
-  for (ModuleID MID : ClangModuleDeps)
-    Ret.push_back(("-fmodule-file=" + LookupPCMPath(MID)).str());
+  for (ModuleID MID : ClangModuleDeps) {
+    auto MO = LookupModuleOutputs(MID);
+    if (!MO)
+      return MO.takeError();
+    Ret.push_back("-fmodule-file=" + MO->ModuleFile);
+  }
 
   return Ret;
 }
Index: clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -65,6 +65,19 @@
   }
 };
 
+/// Options to use to set or override output paths when building a module.
+struct ModuleOutputOptions {
+  /// The path of the module file (.pcm). Required.
+  std::string ModuleFile;
+  /// The path of the dependency file (.d), if any.
+  Optional<std::string> DependencyFile;
+  /// A list of names to use as the targets in the dependency file; if provided,
+  /// this list must contain at least one entry.
+  Optional<std::vector<std::string>> DependencyTargets;
+  /// The path of the serialized diagnostic file (.dia), if any.
+  Optional<std::string> DiagnosticSerializationFile;
+};
+
 struct ModuleDeps {
   /// The identifier of the module.
   ModuleID ID;
@@ -109,12 +122,13 @@
 
   /// Gets the canonical command line suitable for passing to clang.
   ///
-  /// \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.
-  std::vector<std::string> getCanonicalCommandLine(
-      std::function<StringRef(ModuleID)> LookupPCMPath) const;
+  /// \param LookupModuleOutputs This function is called to fill in
+  ///                            "-fmodule-file=", "-o" and other output
+  ///                            arguments.
+  Expected<std::vector<std::string>> getCanonicalCommandLine(
+      llvm::function_ref<
+          Expected<const ModuleOutputOptions &>(const ModuleID &)>
+          LookupModuleOutputs) const;
 
   /// Gets the canonical command line suitable for passing to clang, excluding
   /// "-fmodule-file=" and "-o" arguments.
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -47,12 +47,13 @@
 
   /// Get the full command line.
   ///
-  /// \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.
-  std::vector<std::string>
-  getCommandLine(std::function<StringRef(ModuleID)> LookupPCMPath) const;
+  /// \param LookupModuleOutputs This function is called to fill in
+  ///                            "-fmodule-file=", "-o" and other output
+  ///                            arguments for dependencies.
+  Expected<std::vector<std::string>>
+  getCommandLine(llvm::function_ref<
+                 Expected<const ModuleOutputOptions &>(const ModuleID &)>
+                     LookupModuleOutputs) const;
 
   /// Get the full command line, excluding -fmodule-file=" arguments.
   std::vector<std::string> getCommandLineWithoutModulePaths() const;
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to