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<tooling::CompileCommand>
+ getCompileCommands(StringRef FilePath) const {
+ return {Command};
+ }
+
+ virtual std::vector<tooling::CompileCommand> 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<std::pair<std::string, std::string>> 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<tooling::ArgumentsAdjustingCompilations>(
@@ -210,8 +223,12 @@
#endif
std::vector<std::unique_ptr<DependencyScanningTool>> WorkerTools;
for (unsigned I = 0; I < NumWorkers; ++I)
- WorkerTools.push_back(std::make_unique<DependencyScanningTool>(
- Service, *AdjustingCompilations));
+ WorkerTools.push_back(std::make_unique<DependencyScanningTool>(Service));
+
+ std::vector<SingleCommandCompilationDatabase> Inputs;
+ for (tooling::CompileCommand Cmd :
+ AdjustingCompilations->getAllCompileCommands())
+ Inputs.emplace_back(Cmd);
std::vector<std::thread> WorkerThreads;
std::atomic<bool> 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<std::mutex> 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
// CHECK2NO-NOT: header2
+
+// CHECK3: adena.o
Index: clang/test/ClangScanDeps/error.cpp
===================================================================
--- clang/test/ClangScanDeps/error.cpp
+++ clang/test/ClangScanDeps/error.cpp
@@ -18,4 +18,8 @@
// CHECK-NEXT: fatal error: 'missing.h' file not found
// CHECK-NEXT: "missing.h"
// CHECK-NEXT: ^
+// CHECK-NEXT: Error while scanning dependencies
+// CHECK-NEXT: fatal error: 'missing.h' file not found
+// CHECK-NEXT: "missing.h"
+// CHECK-NEXT: ^
// CHECK-NEXT: EOF
Index: clang/test/ClangScanDeps/Inputs/regular_cdb.json
===================================================================
--- clang/test/ClangScanDeps/Inputs/regular_cdb.json
+++ clang/test/ClangScanDeps/Inputs/regular_cdb.json
@@ -8,5 +8,10 @@
"directory": "DIR",
"command": "clang -E DIR/regular_cdb_input.cpp -IInputs",
"file": "DIR/regular_cdb_input.cpp"
+},
+{
+ "directory": "DIR",
+ "command": "clang -E DIR/regular_cdb_input.cpp -IInputs -o adena.o",
+ "file": "DIR/regular_cdb_input.cpp"
}
]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===================================================================
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -14,13 +14,11 @@
namespace dependencies{
DependencyScanningTool::DependencyScanningTool(
- DependencyScanningService &Service,
- const tooling::CompilationDatabase &Compilations)
- : Worker(Service), Compilations(Compilations) {}
+ DependencyScanningService &Service)
+ : Worker(Service) {}
-llvm::Expected<std::string>
-DependencyScanningTool::getDependencyFile(const std::string &Input,
- StringRef CWD) {
+llvm::Expected<std::string> DependencyScanningTool::getDependencyFile(
+ const tooling::CompilationDatabase &Compilations, StringRef CWD) {
/// Prints out all of the gathered dependencies into a string.
class DependencyPrinterConsumer : public DependencyConsumer {
public:
@@ -59,6 +57,14 @@
std::vector<std::string> Dependencies;
};
+ // We expect a single command here because if a source file occurs multiple
+ // times in the original CDB, then `computeDependencies` would run the
+ // `DependencyScanningAction` once for every time the input occured in the
+ // CDB. Instead we split up the CDB into single command chunks to avoid this
+ // behavior.
+ assert(Compilations.getAllCompileCommands().size() == 1 &&
+ "Expected a compilation database with a single command!");
+ std::string Input = Compilations.getAllCompileCommands().front().Filename;
DependencyPrinterConsumer Consumer;
auto Result =
Worker.computeDependencies(Input, CWD, Compilations, Consumer);
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
===================================================================
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -26,9 +26,7 @@
///
/// \param Compilations The reference to the compilation database that's
/// used by the clang tool.
- DependencyScanningTool(
- DependencyScanningService &Service,
- const clang::tooling::CompilationDatabase &Compilations);
+ DependencyScanningTool(DependencyScanningService &Service);
/// Print out the dependency information into a string using the dependency
/// file format that is specified in the options (-MD is the default) and
@@ -36,12 +34,12 @@
///
/// \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(const tooling::CompilationDatabase &Compilations,
+ StringRef CWD);
private:
DependencyScanningWorker Worker;
- const tooling::CompilationDatabase &Compilations;
};
} // end namespace dependencies
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
