llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Jan Svoboda (jansvoboda11) <details> <summary>Changes</summary> This is an NFC patch that aims to simplify how the scanner calls `Consumer.handleBuildCommand()` by doing it directly in `DependencyScanningAction` instead of going through the `setLastCC1Arguments()` and `takeLastCC1Arguments()` dance with the client. --- Full diff: https://github.com/llvm/llvm-project/pull/169064.diff 3 Files Affected: - (modified) clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp (+12-5) - (modified) clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h (+2-17) - (modified) clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp (+9-13) ``````````diff diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp index 657547d299abd..3b28bcd40458b 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.cpp @@ -638,7 +638,7 @@ dependencies::initializeScanInstanceDependencyCollector( } bool DependencyScanningAction::runInvocation( - std::unique_ptr<CompilerInvocation> Invocation, + std::string Executable, std::unique_ptr<CompilerInvocation> Invocation, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, std::shared_ptr<PCHContainerOperations> PCHContainerOps, DiagnosticConsumer *DiagConsumer) { @@ -654,9 +654,12 @@ bool DependencyScanningAction::runInvocation( if (Scanned) { // Scanning runs once for the first -cc1 invocation in a chain of driver // jobs. For any dependent jobs, reuse the scanning result and just - // update the LastCC1Arguments to correspond to the new invocation. + // update the new invocation. // FIXME: to support multi-arch builds, each arch requires a separate scan - setLastCC1Arguments(std::move(OriginalInvocation)); + if (MDC) + MDC->applyDiscoveredDependencies(OriginalInvocation); + Consumer.handleBuildCommand( + {Executable, OriginalInvocation.getCC1CommandLine()}); return true; } @@ -701,8 +704,12 @@ bool DependencyScanningAction::runInvocation( // ExecuteAction is responsible for calling finish. DiagConsumerFinished = true; - if (Result) - setLastCC1Arguments(std::move(OriginalInvocation)); + if (Result) { + if (MDC) + MDC->applyDiscoveredDependencies(OriginalInvocation); + Consumer.handleBuildCommand( + {Executable, OriginalInvocation.getCC1CommandLine()}); + } return Result; } diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h index b94d1b472f920..254ff79acc4d3 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h +++ b/clang/lib/Tooling/DependencyScanning/DependencyScannerImpl.h @@ -38,7 +38,8 @@ class DependencyScanningAction { std::optional<StringRef> ModuleName = std::nullopt) : Service(Service), WorkingDirectory(WorkingDirectory), Consumer(Consumer), Controller(Controller), DepFS(std::move(DepFS)) {} - bool runInvocation(std::unique_ptr<CompilerInvocation> Invocation, + bool runInvocation(std::string Executable, + std::unique_ptr<CompilerInvocation> Invocation, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, std::shared_ptr<PCHContainerOperations> PCHContainerOps, DiagnosticConsumer *DiagConsumer); @@ -46,22 +47,7 @@ class DependencyScanningAction { bool hasScanned() const { return Scanned; } bool hasDiagConsumerFinished() const { return DiagConsumerFinished; } - /// Take the cc1 arguments corresponding to the most recent invocation used - /// with this action. Any modifications implied by the discovered dependencies - /// will have already been applied. - std::vector<std::string> takeLastCC1Arguments() { - std::vector<std::string> Result; - std::swap(Result, LastCC1Arguments); // Reset LastCC1Arguments to empty. - return Result; - } - private: - void setLastCC1Arguments(CompilerInvocation &&CI) { - if (MDC) - MDC->applyDiscoveredDependencies(CI); - LastCC1Arguments = CI.getCC1CommandLine(); - } - DependencyScanningService &Service; StringRef WorkingDirectory; DependencyConsumer &Consumer; @@ -69,7 +55,6 @@ class DependencyScanningAction { IntrusiveRefCntPtr<DependencyScanningWorkerFilesystem> DepFS; std::optional<CompilerInstance> ScanInstanceStorage; std::shared_ptr<ModuleDepCollector> MDC; - std::vector<std::string> LastCC1Arguments; bool Scanned = false; bool DiagConsumerFinished = false; }; diff --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp index 0bc17f9c80605..a0650155222b6 100644 --- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp +++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp @@ -84,18 +84,14 @@ static bool createAndRunToolInvocation( DependencyScanningAction &Action, IntrusiveRefCntPtr<llvm::vfs::FileSystem> FS, std::shared_ptr<clang::PCHContainerOperations> &PCHContainerOps, - DiagnosticsEngine &Diags, DependencyConsumer &Consumer) { + DiagnosticsEngine &Diags) { auto Invocation = createCompilerInvocation(CommandLine, Diags); if (!Invocation) return false; - if (!Action.runInvocation(std::move(Invocation), std::move(FS), - PCHContainerOps, Diags.getClient())) - return false; - - std::vector<std::string> Args = Action.takeLastCC1Arguments(); - Consumer.handleBuildCommand({CommandLine[0], std::move(Args)}); - return true; + return Action.runInvocation(CommandLine[0], std::move(Invocation), + std::move(FS), PCHContainerOps, + Diags.getClient()); } bool DependencyScanningWorker::scanDependencies( @@ -109,9 +105,9 @@ bool DependencyScanningWorker::scanDependencies( bool Success = false; if (CommandLine[1] == "-cc1") { - Success = createAndRunToolInvocation( - CommandLine, Action, FS, PCHContainerOps, - *DiagEngineWithCmdAndOpts.DiagEngine, Consumer); + Success = + createAndRunToolInvocation(CommandLine, Action, FS, PCHContainerOps, + *DiagEngineWithCmdAndOpts.DiagEngine); } else { Success = forEachDriverJob( CommandLine, *DiagEngineWithCmdAndOpts.DiagEngine, FS, @@ -125,7 +121,7 @@ bool DependencyScanningWorker::scanDependencies( return true; } - // Insert -cc1 comand line options into Argv + // Insert -cc1 command line options into Argv std::vector<std::string> Argv; Argv.push_back(Cmd.getExecutable()); llvm::append_range(Argv, Cmd.getArguments()); @@ -136,7 +132,7 @@ bool DependencyScanningWorker::scanDependencies( // dependency scanning filesystem. return createAndRunToolInvocation( std::move(Argv), Action, FS, PCHContainerOps, - *DiagEngineWithCmdAndOpts.DiagEngine, Consumer); + *DiagEngineWithCmdAndOpts.DiagEngine); }); } `````````` </details> https://github.com/llvm/llvm-project/pull/169064 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
