[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

265

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

118–119

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/include/clang/Frontend/DependencyOutputOptions.h

public:
   std::string OutputFile;
 
   /// The file to write header include output to. This is orthogonal to
   /// ShowHeaderIncludes (-H) and will include headers mentioned in the
   /// predefines buffer. If the output file is "-", output will be sent to
   /// stderr.
   std::string HeaderIncludeOutputFile;
 
-  /// A list of names to use as the targets in the dependency file; this list
-  /// must contain at least one entry.
+  /// A list of names to use as the targets in the dependency file.
   std::vector<std::string> Targets;
 
   /// A list of filenames to be used as extra dependencies for every target.
   std::vector<std::string> ExtraDeps;
 
   /// In /showIncludes mode, pretend the main TU is a header with this name.
   std::string ShowIncludesPretendHeader;
 

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

bool runInvocation(std::shared_ptr<CompilerInvocation> Invocation,
     // Create the dependency collector that will collect the produced
     // 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()));
-    // We need at least one -MT equivalent for the generator to work.
-    if (Opts->Targets.empty())
-      Opts->Targets = {"clang-scan-deps dependency"};
     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();
     return Result;

clang/test/ClangScanDeps/Inputs/subframework_header_dir_symlink_cdb.json

 [
 {
   "directory": "DIR",
   "command": "clang -E DIR/subframework_header_dir_symlink_input.m -D EMPTY -iframework Inputs/frameworks",
-  "file": "DIR/subframework_header_dir_symlink.m"
+  "file": "DIR/subframework_header_dir_symlink_input.m"
 },
 {
   "directory": "DIR",
   "command": "clang -E DIR/subframework_header_dir_symlink_input2.m -FInputs/frameworks -iframework Inputs/frameworks_symlink",
-  "file": "DIR/subframework_header_dir_symlink2.m"
+  "file": "DIR/subframework_header_dir_symlink_input2.m"
 }
 ]

clang/test/ClangScanDeps/Inputs/symlink_cdb.json

 [
 {
   "directory": "DIR",
   "command": "clang -E DIR/symlink_input.cpp -IInputs",
-  "file": "DIR/symlink.cpp"
+  "file": "DIR/symlink_input.cpp"
 },
 {
   "directory": "DIR",
   "command": "clang -E DIR/symlink_input2.cpp -IInputs",
-  "file": "DIR/symlink2.cpp"
+  "file": "DIR/symlink_input2.cpp"
 }
 ]

clang/test/ClangScanDeps/Inputs/vfsoverlay_cdb.json

 [
 {
   "directory": "DIR",
   "command": "clang -E DIR/vfsoverlay_input.cpp -IInputs -ivfsoverlay DIR/vfsoverlay.yaml",
-  "file": "DIR/vfsoverlay.cpp"
+  "file": "DIR/vfsoverlay_input.cpp"
 }
 ]

clang/test/ClangScanDeps/header_stat_before_open.m

 // RUN: rm -rf %t.dir
 // RUN: rm -rf %t.cdb
 // RUN: mkdir -p %t.dir
 // RUN: cp %s %t.dir/header_stat_before_open_input.m
 // RUN: mkdir %t.dir/Inputs
 // RUN: cp -R %S/Inputs/frameworks %t.dir/Inputs/frameworks
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/header_stat_before_open_cdb.json > %t.cdb
 //
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | \
 // RUN:   FileCheck %s
 
 #include "Framework/Framework.h"
 #include "Framework/PrivateHeader.h"
 
-// CHECK: clang-scan-deps dependency
+// CHECK: header_stat_before_open_input.o
 // CHECK-NEXT: header_stat_before_open_input.m
 // CHECK-NEXT: Inputs{{/|\\}}frameworks{{/|\\}}Framework.framework{{/|\\}}Headers{{/|\\}}Framework.h
 // CHECK-NEXT: Inputs{{/|\\}}frameworks{{/|\\}}Framework.framework{{/|\\}}PrivateHeaders{{/|\\}}PrivateHeader.h

clang/test/ClangScanDeps/subframework_header_dir_symlink.m

 // RUN:   FileCheck %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -reuse-filemanager=1 | \
 // RUN:   FileCheck %s
 
 #ifndef EMPTY
 #include "Framework/Framework.h"
 #endif
 
-// CHECK: clang-scan-deps dependency
+// CHECK: subframework_header_dir_symlink_input.o
 // CHECK-NEXT: subframework_header_dir_symlink_input.m
-// CHECK: clang-scan-deps dependency
+// CHECK: subframework_header_dir_symlink_input2.o
 // CHECK-NEXT: subframework_header_dir_symlink_input2.m
 // CHECK-NEXT: Inputs{{/|\\}}frameworks_symlink{{/|\\}}Framework.framework{{/|\\}}Headers{{/|\\}}Framework.h

clang/test/ClangScanDeps/vfsoverlay.cpp

 // RUN: rm -rf %t.dir
 // RUN: rm -rf %t.cdb
 // RUN: mkdir -p %t.dir
 // RUN: cp %s %t.dir/vfsoverlay_input.cpp
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/vfsoverlay.yaml > %t.dir/vfsoverlay.yaml
 // RUN: mkdir %t.dir/Inputs
 // RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/vfsoverlay_cdb.json > %t.cdb
 //
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 | \
 // RUN:   FileCheck %s
 
 #include "not_real.h"
 
-// CHECK: clang-scan-deps dependency
+// CHECK: vfsoverlay_input.o
 // CHECK-NEXT: vfsoverlay_input.cpp
 // CHECK-NEXT: Inputs{{/|\\}}header.h

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/Path.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
 #include <mutex>
 #include <thread>
 
 using namespace clang;
 using namespace tooling::dependencies;
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 = {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
 
+/// \returns object-file name derived from source-file name.
+static std::string getObjFileName(StringRef SrcFile) {
+  SmallString<128> ObjFileName(llvm::sys::path::filename(SrcFile));
+  llvm::sys::path::replace_extension(ObjFileName, "o");
+  return ObjFileName.str();
+}
+
 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();
 
+  struct DependencyScanInput {
+    std::string FileName;
+    std::string 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;
+  std::size_t CompilationComandIdx = 0;
+  for (const auto &Command : CompilationComands) {
+    Inputs.push_back(
+        {Command.Filename, Command.Directory,
+         std::to_string(CompilationComandIdx++) + "_" +
+             (Command.Output.empty() ? getObjFileName(Command.Filename)
+                                     : 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;
       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;
 }