[clang-scan-deps] Add proper dependency targets

[jkorous]

Summary

I'd like to get actual targets of dependencies in the output instead of fixed "clang-scan-deps dependency" string. My primary motivation is ease of testing the tool despite non-deterministic order of targets in its output but I assume this might be useful in general.

[arphaman]

Instead of basing the new -MT name off the output, can you base it on the name of the input filename instead to simplify this logic?

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

271

Nit: Doesn't need zero initialization.

[arphaman]

Why "We need at least one -MT equivalent for the generator to work."?

See DependencyFileGenerator::attachToPreprocessor for the current logic.

[jkorous]

In D67197#1658553, @arphaman wrote:

Instead of basing the new -MT name off the output, can you base it on the name of the input filename instead to simplify this logic?

I can but I'd like to eventually use something more robust as this will be incorrect if there's -o specified in the compilation command.

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

190

Ooops, wrong default.

[jkorous]

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

122

Now that Targets can remain empty before DependencyPrinterConsumer::printDependencies is called (https://reviews.llvm.org/D67308) I can remove this.

[arphaman]

In D67197#1661857, @jkorous wrote:

In D67197#1658553, @arphaman wrote:

Instead of basing the new -MT name off the output, can you base it on the name of the input filename instead to simplify this logic?

I can but I'd like to eventually use something more robust as this will be incorrect if there's -o specified in the compilation command.

You can use the number of the compilation database entry as unique ID that won't collide.

[jkorous]

• rebased
• updated tests

TODO: use compilation commands database index as target

[jkorous]

• prefix dependency targets with CDB index

[arphaman]

LGTM, but please: add a CHECK somewhere that a CompilationComandIdx is actually emitted as a prefix (I don't see one now).

[tschuett]

Thanks for doing that! But it looks odd to attach the dependencies to the output file.

clang -MD -c foo.c -o foo.o

will attach the headers/dependencies to foo.c. foo.c includes the headers. It does not attach the headers to foo.o
It will work for make-like build systems, but it looks odd.

[jkorous]

In D67197#1664565, @tschuett wrote:

Thanks for doing that! But it looks odd to attach the dependencies to the output file.

clang -MD -c foo.c -o foo.o

will attach the headers/dependencies to foo.c. foo.c includes the headers. It does not attach the headers to foo.o
It will work for make-like build systems, but it looks odd.

That's a good point. I didn't think about headers and modules at all. Would you be fine with me addressing this in a separate patch?

[tschuett]

No worries.

[jkorous]

I realized there was an issue with adding the -o /dev/null to arguemnts in case when there was a -MD flag as Driver later adds -MT with the last -o value in case there's no -MT or -MQ. That means that for headers the target would always be /dev/null.
@dexonsmith pointed out that we could just conditionally add -MT in the adjustment. Also, we can use build directory instead of CDB index.

[jkorous]

Abandoned in favor of: https://reviews.llvm.org/D67475

...which is both cleaner and uses appropriate target value for header file dependencies.

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
     // dependencies.
     //
     // This also moves the existing dependency output options from the
     // invocation to the collector. The options in the invocation are reset,
     // which ensures that the compiler won't create new dependency collectors,
     // and thus won't write out the extra '.d' files to disk.
     auto Opts = std::make_unique<DependencyOutputOptions>(
         std::move(Compiler.getInvocation().getDependencyOutputOpts()));
+    // FIXME: This is a hack.
     // We need at least one -MT equivalent for the generator to work.
     if (Opts->Targets.empty())
-      Opts->Targets = {"clang-scan-deps dependency"};
+      Opts->Targets = {"__THIS_IS_A_HACK__"};
     Compiler.addDependencyCollector(
         std::make_shared<DependencyConsumerForwarder>(std::move(Opts),
                                                       Consumer));
 
     auto Action = std::make_unique<PreprocessOnlyAction>();
     const bool Result = Compiler.ExecuteAction(*Action);
     if (!DepFS)
       FileMgr->clearStatCache();

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

 //===- ClangScanDeps.cpp - Implementation of clang-scan-deps --------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Tooling/CommonOptionsParser.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
 #include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
 #include "clang/Tooling/JSONCompilationDatabase.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/InitLLVM.h"
 #include "llvm/Support/Options.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
 #include <mutex>
 #include <thread>
 
DependencyScanningTool(DependencyScanningService &Service,
       : Worker(Service), Compilations(Compilations), OS(OS), Errs(Errs) {}
 
   /// Print out the dependency information into a string using the dependency
   /// file format that is specified in the options (-MD is the default) and
   /// return it.
   ///
   /// \returns A \c StringError with the diagnostic output if clang errors
   /// occurred, dependency file contents otherwise.
-  llvm::Expected<std::string> getDependencyFile(const std::string &Input,
-                                                StringRef CWD) {
+  llvm::Expected<std::string> getDependencyFile(StringRef Input, StringRef CWD,
+                                                StringRef CompilationOutput) {
     /// Prints out all of the gathered dependencies into a string.
     class DependencyPrinterConsumer : public DependencyConsumer {
     public:
       void handleFileDependency(const DependencyOutputOptions &Opts,
                                 StringRef File) override {
         if (!this->Opts)
           this->Opts = std::make_unique<DependencyOutputOptions>(Opts);
         Dependencies.push_back(File);
       }
 
-      void printDependencies(std::string &S) {
+      void printDependencies(StringRef CompilationOutput, std::string &S) {
         if (!Opts)
           return;
 
+        if (Opts->Targets.empty() ||
+            Opts->Targets == std::vector<std::string>{"__THIS_IS_A_HACK__"})
+          Opts->Targets = {CompilationOutput};
+
         class DependencyPrinter : public DependencyFileGenerator {
         public:
           DependencyPrinter(DependencyOutputOptions &Opts,
                             ArrayRef<std::string> Dependencies)
               : DependencyFileGenerator(Opts) {
             for (const auto &Dep : Dependencies)
               addDependency(Dep);
           }
llvm::Expected<std::string> getDependencyFile(StringRef Input, StringRef CWD,
     };
 
     DependencyPrinterConsumer Consumer;
     auto Result =
         Worker.computeDependencies(Input, CWD, Compilations, Consumer);
     if (Result)
       return std::move(Result);
     std::string Output;
-    Consumer.printDependencies(Output);
+
+    Consumer.printDependencies(CompilationOutput, Output);
+
     return Output;
   }
 
   /// Computes the dependencies for the given file and prints them out.
   ///
   /// \returns True on error.
-  bool runOnFile(const std::string &Input, StringRef CWD) {
-    auto MaybeFile = getDependencyFile(Input, CWD);
+  bool runOnFile(StringRef Input, StringRef CWD, StringRef Output) {
+    auto MaybeFile = getDependencyFile(Input, CWD, Output);
     if (!MaybeFile) {
       llvm::handleAllErrors(
           MaybeFile.takeError(), [this, &Input](llvm::StringError &Err) {
             Errs.applyLocked([&](raw_ostream &OS) {
               OS << "Error while scanning dependencies for " << Input << ":\n";
               OS << Err.getMessage();
             });
           });

 
 llvm::cl::opt<bool> ReuseFileManager(
     "reuse-filemanager",
     llvm::cl::desc("Reuse the file manager and its cache between invocations."),
     llvm::cl::init(true), llvm::cl::cat(DependencyScannerCategory));
 
 } // end anonymous namespace
 
+static std::string getOutputFromCompilationCommandLine(
+    const std::vector<std::string> &CommandLine) {
+  bool NextOneIsTheOutput = false;
+  for (const auto &arg : CommandLine) {
+    if (NextOneIsTheOutput) {
+      return arg;
+    }
+    NextOneIsTheOutput = (arg == "-o");
+  }
+  // FIXME
+  return "a.out";
+}
+
 int main(int argc, const char **argv) {
   llvm::InitLLVM X(argc, argv);
   llvm::cl::HideUnrelatedOptions(DependencyScannerCategory);
   if (!llvm::cl::ParseCommandLineOptions(argc, argv))
     return 1;
 
   std::string ErrorMessage;
   std::unique_ptr<tooling::JSONCompilationDatabase> Compilations =
       tooling::JSONCompilationDatabase::loadFromFile(
           CompilationDB, ErrorMessage,
           tooling::JSONCommandLineSyntax::AutoDetect);
   if (!Compilations) {
     llvm::errs() << "error: " << ErrorMessage << "\n";
     return 1;
   }
 
   llvm::cl::PrintOptionValues();
 
+  // Non-owning set of strings backed by some storage.
+  struct DependencyScanInput {
+    StringRef FileName;
+    StringRef Directory;
+    std::string Output;
+  };
+
   // By default the tool runs on all inputs in the CDB.
-  std::vector<std::pair<std::string, std::string>> Inputs;
-  for (const auto &Command : Compilations->getAllCompileCommands())
-    Inputs.emplace_back(Command.Filename, Command.Directory);
+  auto CompilationComands = Compilations->getAllCompileCommands();
+  std::vector<DependencyScanInput> Inputs;
+  for (const auto &Command : CompilationComands) {
+    Inputs.push_back(
+        {Command.Filename, Command.Directory,
+         Command.Output.empty()
+             ? getOutputFromCompilationCommandLine(Command.CommandLine)
+             : Command.Output});
+  }
 
   // The command options are rewritten to run Clang in preprocessor only mode.
   auto AdjustingCompilations =
       std::make_unique<tooling::ArgumentsAdjustingCompilations>(
           std::move(Compilations));
   AdjustingCompilations->appendArgumentsAdjuster(
       [](const tooling::CommandLineArguments &Args, StringRef /*unused*/) {
         tooling::CommandLineArguments AdjustedArgs = Args;
#endif
   std::atomic<bool> HadErrors(false);
   std::mutex Lock;
   size_t Index = 0;
 
   llvm::outs() << "Running clang-scan-deps on " << Inputs.size()
                << " files using " << NumWorkers << " workers\n";
   for (unsigned I = 0; I < NumWorkers; ++I) {
     auto Worker = [I, &Lock, &Index, &Inputs, &HadErrors, &WorkerTools]() {
+      size_t IndexToProcess = 0;
       while (true) {
-        std::string Input;
-        StringRef CWD;
         // Take the next input.
         {
           std::unique_lock<std::mutex> LockGuard(Lock);
-          if (Index >= Inputs.size())
-            return;
-          const auto &Compilation = Inputs[Index++];
-          Input = Compilation.first;
-          CWD = Compilation.second;
+          IndexToProcess = Index++;
         }
+        if (IndexToProcess >= Inputs.size())
+          return;
+
+        const auto &Compilation = Inputs[IndexToProcess];
         // Run the tool on it.
-        if (WorkerTools[I]->runOnFile(Input, CWD))
+        if (WorkerTools[I]->runOnFile(Compilation.FileName,
+                                      Compilation.Directory,
+                                      Compilation.Output))
           HadErrors = true;
       }
     };
 #if LLVM_ENABLE_THREADS
     WorkerThreads.emplace_back(std::move(Worker));
 #else
     // Run the worker without spawning a thread when threads are disabled.
     Worker();
 #endif
   }
   for (auto &W : WorkerThreads)
     W.join();
 
   return HadErrors;
 }