[clang][deps] Add -o flag to specify output path

[jansvoboda11]

This makes it possible to pass "-o /dev/null" to clang-scan-deps and skip some potentially expensive work, making timings less noisy. Also removes the need for stream redirection.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

This makes it possible to pass "-o /dev/null" to clang-scan-deps and skip some potentially expensive work, making timings less noisy. Also removes the need for stream redirection.

---

Full diff: https://github.com/llvm/llvm-project/pull/88767.diff

3 Files Affected:

• (modified) clang/test/ClangScanDeps/module-format.c (+1-1)
• (modified) clang/tools/clang-scan-deps/ClangScanDeps.cpp (+29-4)
• (modified) clang/tools/clang-scan-deps/Opts.td (+3-1)

diff --git a/clang/test/ClangScanDeps/module-format.c b/clang/test/ClangScanDeps/module-format.c
index 001a011ae0b597..0a6abec80dd909 100644
--- a/clang/test/ClangScanDeps/module-format.c
+++ b/clang/test/ClangScanDeps/module-format.c
@@ -16,7 +16,7 @@
 // RUN: rm -f %t/cdb_pch.json
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_pch.json > %t/cdb_pch.json
 // RUN: clang-scan-deps -compilation-database %t/cdb_pch.json -format experimental-full \
-// RUN:   -module-files-dir %t/build > %t/result_pch.json
+// RUN:   -module-files-dir %t/build -o %t/result_pch.json
 
 // Explicitly build the PCH:
 //
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index eaa76dd43e41dd..90a3ee50c1be60 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -72,6 +72,7 @@ enum ResourceDirRecipeKind {
   RDRK_InvokeCompiler,
 };
 
+static std::string OutputFileName = "-";
 static ScanningMode ScanMode = ScanningMode::DependencyDirectivesScan;
 static ScanningOutputFormat Format = ScanningOutputFormat::Make;
 static ScanningOptimizations OptimizeArgs;
@@ -175,6 +176,9 @@ static void ParseArgs(int argc, char **argv) {
   if (const llvm::opt::Arg *A = Args.getLastArg(OPT_module_files_dir_EQ))
     ModuleFilesDir = A->getValue();
 
+  if (const llvm::opt::Arg *A = Args.getLastArg(OPT_o))
+    OutputFileName = A->getValue();
+
   EagerLoadModules = Args.hasArg(OPT_eager_load_pcm);
 
   if (const llvm::opt::Arg *A = Args.getLastArg(OPT_j)) {
@@ -426,6 +430,11 @@ class FullDeps {
   }
 
   void printFullOutput(raw_ostream &OS) {
+    // Skip sorting modules and constructing the JSON object if the output
+    // cannot be observed anyway. This makes timings less noisy.
+    if (&OS == &llvm::nulls())
+      return;
+
     // Sort the modules by name to get a deterministic order.
     std::vector<IndexedModuleID> ModuleIDs;
     for (auto &&M : Modules)
@@ -864,8 +873,24 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
       });
 
   SharedStream Errs(llvm::errs());
-  // Print out the dependency results to STDOUT by default.
-  SharedStream DependencyOS(llvm::outs());
+
+  std::optional<llvm::raw_fd_ostream> FileOS;
+  llvm::raw_ostream &ThreadUnsafeDependencyOS = [&]() -> llvm::raw_ostream & {
+    if (OutputFileName == "-")
+      return llvm::outs();
+
+    if (OutputFileName == "/dev/null")
+      return llvm::nulls();
+
+    std::error_code EC;
+    FileOS.emplace(OutputFileName, EC);
+    if (EC) {
+      llvm::errs() << llvm::errorCodeToError(EC) << '\n';
+      std::exit(1);
+    }
+    return *FileOS;
+  }();
+  SharedStream DependencyOS(ThreadUnsafeDependencyOS);
 
   std::vector<tooling::CompileCommand> Inputs =
       AdjustingCompilations->getAllCompileCommands();
@@ -1006,9 +1031,9 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) {
       HadErrors = true;
 
   if (Format == ScanningOutputFormat::Full)
-    FD->printFullOutput(llvm::outs());
+    FD->printFullOutput(ThreadUnsafeDependencyOS);
   else if (Format == ScanningOutputFormat::P1689)
-    PD.printDependencies(llvm::outs());
+    PD.printDependencies(ThreadUnsafeDependencyOS);
 
   return HadErrors;
 }
diff --git a/clang/tools/clang-scan-deps/Opts.td b/clang/tools/clang-scan-deps/Opts.td
index 5cd5d1a9fb37bc..4837ce6f070d73 100644
--- a/clang/tools/clang-scan-deps/Opts.td
+++ b/clang/tools/clang-scan-deps/Opts.td
@@ -11,6 +11,8 @@ multiclass Eq<string name, string help> {
 def help : Flag<["--"], "help">, HelpText<"Display this help">;
 def version : Flag<["--"], "version">, HelpText<"Display the version">;
 
+def o : Arg<"o", "Destination of the primary output">;
+
 defm mode : Eq<"mode", "The preprocessing mode used to compute the dependencies">;
 
 defm format : Eq<"format", "The output format for the dependencies">;
@@ -37,4 +39,4 @@ def verbose : F<"v", "Use verbose output">;
 
 def round_trip_args : F<"round-trip-args", "verify that command-line arguments are canonical by parsing and re-serializing">;
 
-def DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>;
\ No newline at end of file
+def DASH_DASH : Option<["--"], "", KIND_REMAINING_ARGS>;

[benlangmuir]

LGTM with a couple minor comments

[benlangmuir]

I don't feel strongly about whether you should change this code or not -- maybe it's clearer the way it is? -- but raw_fd_ostream("-", EC) is going to be the same as outs() anyway.

[benlangmuir]

Would be good to include the fact it's failing during open and what the path is in this error. Filesystem errrors can be hard to understand without any context