[clang] d816d9b - [clang][ScanDeps] Fix issue with multiple commands with the same input.
Michael Spencer via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 31 14:22:19 PDT 2019
Author: Michael Spencer
Date: 2019-10-31T14:22:01-07:00
New Revision: d816d9bdc585bbf77a7a1c47a7199fd9e0c34402
URL: https://github.com/llvm/llvm-project/commit/d816d9bdc585bbf77a7a1c47a7199fd9e0c34402
DIFF: https://github.com/llvm/llvm-project/commit/d816d9bdc585bbf77a7a1c47a7199fd9e0c34402.diff
LOG: [clang][ScanDeps] Fix issue with multiple commands with the same input.
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.
Differential Revision: https://reviews.llvm.org/D69643
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 78b49e4fa0c5..a3a8c6988819 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -26,9 +26,7 @@ class DependencyScanningTool {
///
/// \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,13 +34,13 @@ class DependencyScanningTool {
///
/// \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:
const ScanningOutputFormat Format;
DependencyScanningWorker Worker;
- const tooling::CompilationDatabase &Compilations;
};
} // end namespace dependencies
diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 4b10f24167a8..f643c538f8f9 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -23,14 +23,12 @@ namespace tooling{
namespace dependencies{
DependencyScanningTool::DependencyScanningTool(
- DependencyScanningService &Service,
- const tooling::CompilationDatabase &Compilations)
- : Format(Service.getFormat()), Worker(Service), Compilations(Compilations) {
+ DependencyScanningService &Service)
+ : Format(Service.getFormat()), 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 MakeDependencyPrinterConsumer : public DependencyConsumer {
public:
@@ -140,6 +138,16 @@ DependencyScanningTool::getDependencyFile(const std::string &Input,
std::string ContextHash;
};
+
+ // 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;
+
if (Format == ScanningOutputFormat::Make) {
MakeDependencyPrinterConsumer Consumer;
auto Result =
diff --git a/clang/test/ClangScanDeps/Inputs/regular_cdb.json b/clang/test/ClangScanDeps/Inputs/regular_cdb.json
index b851a30c3281..902c0b7761fb 100644
--- a/clang/test/ClangScanDeps/Inputs/regular_cdb.json
+++ b/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"
}
]
diff --git a/clang/test/ClangScanDeps/error.cpp b/clang/test/ClangScanDeps/error.cpp
index 00bc2eac97da..e4e052527890 100644
--- a/clang/test/ClangScanDeps/error.cpp
+++ b/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
diff --git a/clang/test/ClangScanDeps/regular_cdb.cpp b/clang/test/ClangScanDeps/regular_cdb.cpp
index 5ba37e05cfef..8fb94350e4c2 100644
--- a/clang/test/ClangScanDeps/regular_cdb.cpp
+++ b/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
diff --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index a6abb4a9600b..3d3c76c7714a 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -108,6 +108,24 @@ static std::string getObjFilePath(StringRef SrcFile) {
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.
///
@@ -147,11 +165,6 @@ int main(int argc, const char **argv) {
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>(
@@ -221,8 +234,12 @@ int main(int argc, const char **argv) {
#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);
@@ -237,20 +254,22 @@ int main(int argc, const char **argv) {
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;
}
};
More information about the cfe-commits
mailing list