[clang] [clang][deps] Simplify consuming of the build command (PR #169064)

[via cfe-commits]

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