From: Michael Spencer Date: Wed, 30 Oct 2019 21:04:11 +0000 (-0700) Subject: [clang][ScanDeps] Fix issue with multiple commands with the same input. X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=d816d9bdc585bbf77a7a1c47a7199fd9e0c34402;p=platform%2Fupstream%2Fllvm.git [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 --- diff --git a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h index 78b49e4..a3a8c69 100644 --- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h +++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h @@ -26,9 +26,7 @@ public: /// /// \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 @@ public: /// /// \returns A \c StringError with the diagnostic output if clang errors /// occurred, dependency file contents otherwise. - llvm::Expected getDependencyFile(const std::string &Input, - StringRef CWD); + llvm::Expected + 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 4b10f24..f643c53 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 -DependencyScanningTool::getDependencyFile(const std::string &Input, - StringRef CWD) { +llvm::Expected 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 b851a30..902c0b7 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 00bc2ea..e4e0525 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 5ba37e0..8fb9435 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 a6abb4a..3d3c76c 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 + 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 @@ int main(int argc, const char **argv) { 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 @@ int main(int argc, const char **argv) { #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 @@ 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 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; } };