From 04bb9f44bfdbcc653391ededeb675caccf497a24 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 16 Apr 2024 19:47:52 -0700 Subject: [PATCH 1/2] [clang][deps] Support single-file mode for all formats (#88764) The `clang-scan-deps` tool can be used for fast scanning of batches of compilation commands passed in via the `-compilation-database` option. This gets awkward in our tests where we have to resort to using `.in`/`.template` JSON files and running them through `sed` in order to embed LIT's `%t` variable into them. However, most of our tests only need to pass single compilation command, so this dance is entirely unnecessary. This patch makes sure the existing "per-file" mode (where the compilation command is passed in-line after the `--` argument) works for all output formats, not only `P1689`. (cherry picked from commit eafd515ecaaa100623eebc7fa4d7c36a361bf708) --- clang/test/ClangScanDeps/error.cpp | 41 ++++++------- clang/tools/clang-scan-deps/ClangScanDeps.cpp | 57 +++++++------------ 2 files changed, 39 insertions(+), 59 deletions(-) diff --git a/clang/test/ClangScanDeps/error.cpp b/clang/test/ClangScanDeps/error.cpp index d94ba4c03e508..593dbf35edca5 100644 --- a/clang/test/ClangScanDeps/error.cpp +++ b/clang/test/ClangScanDeps/error.cpp @@ -1,26 +1,21 @@ -// RUN: rm -rf %t.dir -// RUN: rm -rf %t.cdb -// RUN: mkdir -p %t.dir -// RUN: cp %s %t.dir/regular_cdb_input.cpp -// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb -// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb_clangcl.json > %t_clangcl.cdb -// -// RUN: not clang-scan-deps -compilation-database %t.cdb -j 1 2>%t.dir/errs -// RUN: echo EOF >> %t.dir/errs -// RUN: FileCheck %s --input-file %t.dir/errs - -// RUN: not clang-scan-deps -compilation-database %t_clangcl.cdb -j 1 2>%t.dir/errs_clangcl -// RUN: echo EOF >> %t.dir/errs_clangcl -// RUN: FileCheck %s --input-file %t.dir/errs_clangcl +// RUN: rm -rf %t +// RUN: split-file %s %t +//--- missing_header.c #include "missing.h" -// CHECK: Error while scanning dependencies -// CHECK-NEXT: error: no such file or directory: -// CHECK-NEXT: error: no input files -// CHECK-NEXT: error: -// CHECK-NEXT: Error while scanning dependencies -// CHECK-NEXT: fatal error: 'missing.h' file not found -// CHECK-NEXT: Error while scanning dependencies -// CHECK-NEXT: fatal error: 'missing.h' file not found -// CHECK-NEXT: EOF +// RUN: not clang-scan-deps -- %clang -c %t/missing_tu.c 2>%t/missing_tu.errs +// RUN: echo EOF >> %t/missing_tu.errs +// RUN: cat %t/missing_tu.errs | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-MISSING-TU -DPREFIX=%/t +// CHECK-MISSING-TU: Error while scanning dependencies for [[PREFIX]]/missing_tu.c +// CHECK-MISSING-TU-NEXT: error: no such file or directory: '[[PREFIX]]/missing_tu.c' +// CHECK-MISSING-TU-NEXT: error: no input files +// CHECK-MISSING-TU-NEXT: error: +// CHECK-MISSING-TU-NEXT: EOF + +// RUN: not clang-scan-deps -- %clang -c %t/missing_header.c 2>%t/missing_header.errs +// RUN: echo EOF >> %t/missing_header.errs +// RUN: cat %t/missing_header.errs | sed 's:\\\\\?:/:g' | FileCheck %s --check-prefix=CHECK-MISSING-HEADER -DPREFIX=%/t +// CHECK-MISSING-HEADER: Error while scanning dependencies for [[PREFIX]]/missing_header.c +// CHECK-MISSING-HEADER-NEXT: fatal error: 'missing.h' file not found +// CHECK-MISSING-HEADER-NEXT: EOF diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp index 503b8329a201c..0256a20307359 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -115,8 +115,8 @@ static bool RoundTripArgs = DoRoundTripDefault; static void ParseArgs(int argc, char **argv) { ScanDepsOptTable Tbl; llvm::StringRef ToolName = argv[0]; - llvm::BumpPtrAllocator A; - llvm::StringSaver Saver{A}; + llvm::BumpPtrAllocator Alloc; + llvm::StringSaver Saver{Alloc}; llvm::opt::InputArgList Args = Tbl.parseArgs(argc, argv, OPT_UNKNOWN, Saver, [&](StringRef Msg) { llvm::errs() << Msg << '\n'; @@ -207,14 +207,8 @@ static void ParseArgs(int argc, char **argv) { } } - if (const llvm::opt::Arg *A = Args.getLastArg(OPT_compilation_database_EQ)) { + if (const llvm::opt::Arg *A = Args.getLastArg(OPT_compilation_database_EQ)) CompilationDB = A->getValue(); - } else if (Format != ScanningOutputFormat::P1689) { - llvm::errs() << ToolName - << ": for the --compiilation-database option: must be " - "specified at least once!"; - std::exit(1); - } if (const llvm::opt::Arg *A = Args.getLastArg(OPT_module_name_EQ)) ModuleName = A->getValue(); @@ -265,9 +259,8 @@ static void ParseArgs(int argc, char **argv) { RoundTripArgs = Args.hasArg(OPT_round_trip_args); - if (auto *A = Args.getLastArgNoClaim(OPT_DASH_DASH)) - CommandLine.insert(CommandLine.end(), A->getValues().begin(), - A->getValues().end()); + if (const llvm::opt::Arg *A = Args.getLastArgNoClaim(OPT_DASH_DASH)) + CommandLine.assign(A->getValues().begin(), A->getValues().end()); } class SharedStream { @@ -975,39 +968,29 @@ static std::string getModuleCachePath(ArrayRef Args) { return std::string(Path); } -// getCompilationDataBase - If -compilation-database is set, load the -// compilation database from the specified file. Otherwise if the we're -// generating P1689 format, trying to generate the compilation database -// form specified command line after the positional parameter "--". +/// Attempts to construct the compilation database from '-compilation-database' +/// or from the arguments following the positional '--'. static std::unique_ptr -getCompilationDataBase(int argc, char **argv, std::string &ErrorMessage) { +getCompilationDatabase(int argc, char **argv, std::string &ErrorMessage) { llvm::InitLLVM X(argc, argv); ParseArgs(argc, argv); + if (!(CommandLine.empty() ^ CompilationDB.empty())) { + llvm::errs() << "The compilation command line must be provided either via " + "'-compilation-database' or after '--'."; + return nullptr; + } + if (!CompilationDB.empty()) return tooling::JSONCompilationDatabase::loadFromFile( CompilationDB, ErrorMessage, tooling::JSONCommandLineSyntax::AutoDetect); - if (Format != ScanningOutputFormat::P1689) { - llvm::errs() << "the --compilation-database option: must be specified at " - "least once!"; - return nullptr; - } - - // Trying to get the input file, the output file and the command line options - // from the positional parameter "--". - char **DoubleDash = std::find(argv, argv + argc, StringRef("--")); - if (DoubleDash == argv + argc) { - llvm::errs() << "The command line arguments is required after '--' in " - "P1689 per file mode."; - return nullptr; - } - llvm::IntrusiveRefCntPtr Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions); driver::Driver TheDriver(CommandLine[0], llvm::sys::getDefaultTargetTriple(), *Diags); + TheDriver.setCheckInputsExist(false); std::unique_ptr C( TheDriver.BuildCompilation(CommandLine)); if (!C) @@ -1022,7 +1005,8 @@ getCompilationDataBase(int argc, char **argv, std::string &ErrorMessage) { FrontendOptions &FEOpts = CI->getFrontendOpts(); if (FEOpts.Inputs.size() != 1) { - llvm::errs() << "Only one input file is allowed in P1689 per file mode."; + llvm::errs() + << "Exactly one input file is required in the per-file mode ('--').\n"; return nullptr; } @@ -1031,8 +1015,9 @@ getCompilationDataBase(int argc, char **argv, std::string &ErrorMessage) { auto LastCmd = C->getJobs().end(); LastCmd--; if (LastCmd->getOutputFilenames().size() != 1) { - llvm::errs() << "The command line should provide exactly one output file " - "in P1689 per file mode.\n"; + llvm::errs() + << "Exactly one output file is required in the per-file mode ('--').\n"; + return nullptr; } StringRef OutputFile = LastCmd->getOutputFilenames().front(); @@ -1072,7 +1057,7 @@ getCompilationDataBase(int argc, char **argv, std::string &ErrorMessage) { int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) { std::string ErrorMessage; std::unique_ptr Compilations = - getCompilationDataBase(argc, argv, ErrorMessage); + getCompilationDatabase(argc, argv, ErrorMessage); if (!Compilations) { llvm::errs() << ErrorMessage << "\n"; return 1; From efbeb4182f114c3fd525dcc29682f84e9103e640 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Tue, 16 Apr 2024 19:49:07 -0700 Subject: [PATCH 2/2] [clang][deps] Add `-o` flag to specify output path (#88767) 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. (cherry picked from commit 6a4eaf9b33d8091b7d09b2a30a3fc8993a01db31) --- clang/test/ClangScanDeps/module-format.c | 2 +- clang/tools/clang-scan-deps/ClangScanDeps.cpp | 56 +++++++++++++------ clang/tools/clang-scan-deps/Opts.td | 2 + 3 files changed, 42 insertions(+), 18 deletions(-) diff --git a/clang/test/ClangScanDeps/module-format.c b/clang/test/ClangScanDeps/module-format.c index 001a011ae0b59..0a6abec80dd90 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 0256a20307359..fe18f5e30d840 100644 --- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp +++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp @@ -81,6 +81,7 @@ enum ResourceDirRecipeKind { RDRK_InvokeCompiler, }; +static std::string OutputFileName = "-"; static ScanningMode ScanMode = ScanningMode::DependencyDirectivesScan; static ScanningOutputFormat Format = ScanningOutputFormat::Make; static ScanningOptimizations OptimizeArgs; @@ -196,6 +197,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)) { @@ -514,7 +518,7 @@ handleMakeDependencyToolResult(const std::string &Input, static bool handleTreeDependencyToolResult( llvm::cas::ObjectStore &CAS, const std::string &Input, - llvm::Expected &MaybeTree, SharedStream &OS, + llvm::Expected &MaybeTree, llvm::raw_ostream &OS, SharedStream &Errs) { if (!MaybeTree) { llvm::handleAllErrors( @@ -527,9 +531,7 @@ static bool handleTreeDependencyToolResult( }); return true; } - OS.applyLocked([&](llvm::raw_ostream &OS) { - OS << "tree " << MaybeTree->getID() << " for '" << Input << "'\n"; - }); + OS << "tree " << MaybeTree->getID() << " for '" << Input << "'\n"; return false; } @@ -537,7 +539,7 @@ static bool handleIncludeTreeToolResult(llvm::cas::ObjectStore &CAS, const std::string &Input, Expected &MaybeTree, - SharedStream &OS, SharedStream &Errs) { + llvm::raw_ostream &OS, SharedStream &Errs) { if (!MaybeTree) { llvm::handleAllErrors( MaybeTree.takeError(), [&Input, &Errs](llvm::StringError &Err) { @@ -559,11 +561,9 @@ handleIncludeTreeToolResult(llvm::cas::ObjectStore &CAS, }; std::optional E; - OS.applyLocked([&](llvm::raw_ostream &OS) { - MaybeTree->getID().print(OS); - OS << " - " << Input << "\n"; - E = MaybeTree->print(OS); - }); + MaybeTree->getID().print(OS); + OS << " - " << Input << "\n"; + E = MaybeTree->print(OS); if (*E) return printError(std::move(*E)); return false; @@ -682,6 +682,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 ModuleIDs; for (auto &&M : Modules) @@ -1142,8 +1147,25 @@ 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 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() << "Failed to open output file '" << OutputFileName + << "': " << llvm::errorCodeToError(EC) << '\n'; + std::exit(1); + } + return *FileOS; + }(); + SharedStream DependencyOS(ThreadUnsafeDependencyOS); auto DiagsConsumer = std::make_unique( llvm::errs(), new DiagnosticOptions(), false); @@ -1361,23 +1383,23 @@ int clang_scan_deps_main(int argc, char **argv, const llvm::ToolContext &) { if (Format == ScanningOutputFormat::Tree) { for (auto &TreeResult : TreeResults) { if (handleTreeDependencyToolResult(*CAS, TreeResult.Filename, - *TreeResult.MaybeTree, DependencyOS, - Errs)) + *TreeResult.MaybeTree, + ThreadUnsafeDependencyOS, Errs)) HadErrors = true; } } else if (Format == ScanningOutputFormat::IncludeTree) { for (auto &TreeResult : TreeResults) { if (handleIncludeTreeToolResult(*CAS, TreeResult.Filename, *TreeResult.MaybeIncludeTree, - DependencyOS, Errs)) + ThreadUnsafeDependencyOS, Errs)) HadErrors = true; } } else if (Format == ScanningOutputFormat::Full || Format == ScanningOutputFormat::FullTree || Format == ScanningOutputFormat::FullIncludeTree) { - 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 2d0c242b9d4b4..cc479c12c965d 100644 --- a/clang/tools/clang-scan-deps/Opts.td +++ b/clang/tools/clang-scan-deps/Opts.td @@ -11,6 +11,8 @@ multiclass Eq { 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">;