[PATCH] D69643: [clang][ScanDeps] Fix issue with multiple commands with the same input.

2019-10-31 Thread Michael Spencer via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd816d9bdc585: [clang][ScanDeps] Fix issue with multiple 
commands with the same input. (authored by Bigcheese).

Changed prior to commit:
  https://reviews.llvm.org/D69643?vs=227178&id=227347#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69643

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/error.cpp
  clang/test/ClangScanDeps/regular_cdb.cpp
  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
@@ -108,6 +108,24 @@
   return ObjFileName.str();
 }
 
+class SingleCommandCompilationDatabase : public tooling::CompilationDatabase {
+public:
+  SingleCommandCompilationDatabase(tooling::CompileCommand Cmd)
+  : Command(std::move(Cmd)) {}
+
+  virtual std::vector
+  getCompileCommands(StringRef FilePath) const {
+return {Command};
+  }
+
+  virtual std::vector getAllCompileCommands() const {
+return {Command};
+  }
+
+private:
+  tooling::CompileCommand Command;
+};
+
 /// Takes the result of a dependency scan and prints error / dependency files
 /// based on the result.
 ///
@@ -147,11 +165,6 @@
 
   llvm::cl::PrintOptionValues();
 
-  // By default the tool runs on all inputs in the CDB.
-  std::vector> Inputs;
-  for (const auto &Command : Compilations->getAllCompileCommands())
-Inputs.emplace_back(Command.Filename, Command.Directory);
-
   // The command options are rewritten to run Clang in preprocessor only mode.
   auto AdjustingCompilations =
   std::make_unique(
@@ -221,8 +234,12 @@
 #endif
   std::vector> WorkerTools;
   for (unsigned I = 0; I < NumWorkers; ++I)
-WorkerTools.push_back(std::make_unique(
-Service, *AdjustingCompilations));
+WorkerTools.push_back(std::make_unique(Service));
+
+  std::vector Inputs;
+  for (tooling::CompileCommand Cmd :
+   AdjustingCompilations->getAllCompileCommands())
+Inputs.emplace_back(Cmd);
 
   std::vector WorkerThreads;
   std::atomic HadErrors(false);
@@ -237,20 +254,22 @@
 auto Worker = [I, &Lock, &Index, &Inputs, &HadErrors, &WorkerTools,
&DependencyOS, &Errs]() {
   while (true) {
-std::string Input;
-StringRef CWD;
+const SingleCommandCompilationDatabase *Input;
+std::string Filename;
+std::string CWD;
 // Take the next input.
 {
   std::unique_lock LockGuard(Lock);
   if (Index >= Inputs.size())
 return;
-  const auto &Compilation = Inputs[Index++];
-  Input = Compilation.first;
-  CWD = Compilation.second;
+  Input = &Inputs[Index++];
+  tooling::CompileCommand Cmd = Input->getAllCompileCommands()[0];
+  Filename = std::move(Cmd.Filename);
+  CWD = std::move(Cmd.Directory);
 }
 // Run the tool on it.
-auto MaybeFile = WorkerTools[I]->getDependencyFile(Input, CWD);
-if (handleDependencyToolResult(Input, MaybeFile, DependencyOS, Errs))
+auto MaybeFile = WorkerTools[I]->getDependencyFile(*Input, CWD);
+if (handleDependencyToolResult(Filename, MaybeFile, DependencyOS, Errs))
   HadErrors = true;
   }
 };
Index: clang/test/ClangScanDeps/regular_cdb.cpp
===
--- clang/test/ClangScanDeps/regular_cdb.cpp
+++ clang/test/ClangScanDeps/regular_cdb.cpp
@@ -9,11 +9,11 @@
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
 //
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources \
-// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   -skip-excluded-pp-ranges=0 | FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 //
 // Make sure we didn't produce any dependency files!
 // RUN: not cat %t.dir/regular_cdb.d
@@ -40,6 +40,9 @@
 // CHECK1-NEXT: Inputs{{/|\\}}header.h
 // CHECK1-NEXT: Inputs{{/|\\}}header2.h
 
+// CHECK3: regular_cdb_input.o
 // CHECK2: regular_cdb_input.cpp
 // CHECK2-NEXT: Inputs{{/|\\}}header.h
 // CHECK

[PATCH] D69643: [clang][ScanDeps] Fix issue with multiple commands with the same input.

2019-10-30 Thread Alex Lorenz via Phabricator via cfe-commits
arphaman accepted this revision.
arphaman added a comment.
This revision is now accepted and ready to land.

LGTM, Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69643



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D69643: [clang][ScanDeps] Fix issue with multiple commands with the same input.

2019-10-30 Thread Michael Spencer via Phabricator via cfe-commits
Bigcheese created this revision.
Bigcheese added reviewers: arphaman, klimek.
Bigcheese added a project: clang.
Herald added a subscriber: dexonsmith.

Previously, given a CompilationDatabase with two commands for the same
source file we would report that file twice with the union of the
dependencies for each command both times.

This was due to the way `ClangTool` runs actions given an input source
file (see the comment in `DependencyScanningTool.cpp`). This commit adds
a `SingleCommandCompilationDatabase` that is created with each
`CompileCommand` in the original CDB, which is then used for each
`ClangTool` invocation. This gives us a single run of
`DependencyScanningAction` per `CompileCommand`.

I looked at using `AllTUsToolExecutor` which is a parallel tool
executor, but I'm not sure it's suitable for `clang-scan-deps` as it
does a lot more sharing of state than `AllTUsToolExecutor` expects.

I've added Manuel as a reviewer to see if there's an obviously better way to do 
this with libTooling.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69643

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/test/ClangScanDeps/Inputs/regular_cdb.json
  clang/test/ClangScanDeps/error.cpp
  clang/test/ClangScanDeps/regular_cdb.cpp
  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
@@ -97,6 +97,24 @@
   return ObjFileName.str();
 }
 
+class SingleCommandCompilationDatabase : public tooling::CompilationDatabase {
+public:
+  SingleCommandCompilationDatabase(tooling::CompileCommand Cmd)
+  : Command(std::move(Cmd)) {}
+
+  virtual std::vector
+  getCompileCommands(StringRef FilePath) const {
+return {Command};
+  }
+
+  virtual std::vector getAllCompileCommands() const {
+return {Command};
+  }
+
+private:
+  tooling::CompileCommand Command;
+};
+
 /// Takes the result of a dependency scan and prints error / dependency files
 /// based on the result.
 ///
@@ -136,11 +154,6 @@
 
   llvm::cl::PrintOptionValues();
 
-  // By default the tool runs on all inputs in the CDB.
-  std::vector> Inputs;
-  for (const auto &Command : Compilations->getAllCompileCommands())
-Inputs.emplace_back(Command.Filename, Command.Directory);
-
   // The command options are rewritten to run Clang in preprocessor only mode.
   auto AdjustingCompilations =
   std::make_unique(
@@ -210,8 +223,12 @@
 #endif
   std::vector> WorkerTools;
   for (unsigned I = 0; I < NumWorkers; ++I)
-WorkerTools.push_back(std::make_unique(
-Service, *AdjustingCompilations));
+WorkerTools.push_back(std::make_unique(Service));
+
+  std::vector Inputs;
+  for (tooling::CompileCommand Cmd :
+   AdjustingCompilations->getAllCompileCommands())
+Inputs.emplace_back(Cmd);
 
   std::vector WorkerThreads;
   std::atomic HadErrors(false);
@@ -226,20 +243,22 @@
 auto Worker = [I, &Lock, &Index, &Inputs, &HadErrors, &WorkerTools,
&DependencyOS, &Errs]() {
   while (true) {
-std::string Input;
-StringRef CWD;
+const SingleCommandCompilationDatabase *Input;
+std::string Filename;
+std::string CWD;
 // Take the next input.
 {
   std::unique_lock LockGuard(Lock);
   if (Index >= Inputs.size())
 return;
-  const auto &Compilation = Inputs[Index++];
-  Input = Compilation.first;
-  CWD = Compilation.second;
+  Input = &Inputs[Index++];
+  tooling::CompileCommand Cmd = Input->getAllCompileCommands()[0];
+  Filename = std::move(Cmd.Filename);
+  CWD = std::move(Cmd.Directory);
 }
 // Run the tool on it.
-auto MaybeFile = WorkerTools[I]->getDependencyFile(Input, CWD);
-if (handleDependencyToolResult(Input, MaybeFile, DependencyOS, Errs))
+auto MaybeFile = WorkerTools[I]->getDependencyFile(*Input, CWD);
+if (handleDependencyToolResult(Filename, MaybeFile, DependencyOS, Errs))
   HadErrors = true;
   }
 };
Index: clang/test/ClangScanDeps/regular_cdb.cpp
===
--- clang/test/ClangScanDeps/regular_cdb.cpp
+++ clang/test/ClangScanDeps/regular_cdb.cpp
@@ -9,11 +9,11 @@
 // RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/regular_cdb.json > %t.cdb
 //
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-minimized-sources | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO,CHECK3 %s
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess | \
-// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+// RUN:   Fil