[clang][deps] Simplify consuming of the build command

[jansvoboda11]

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.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Jan Svoboda (jansvoboda11)

Changes

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 --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);
         });
   }
 

[github-actions]

🐧 Linux x64 Test Results

• 111762 tests passed
• 4476 tests skipped

✅ The build succeeded and all tests passed.

[qiongsiwu]

Love the clean up. Thanks! LGTM.