[clang][deps] add support for dependency scanning with cc1 command line

[cpsughrue]

Summary

Allow users to run a dependency scan with a cc1 command line in addition to a driver command line. DependencyScanningAction was already being run with a cc1 command line, but DependencyScanningWorker::computeDependencies assumed that it was always provided a driver command line. Now DependencyScanningWorker::computeDependencies can handle cc1 command lines too.

[cpsughrue]

Fixed formatting issues

[jansvoboda11]

This looks pretty good!

I'm not sure unit testing is the best choice here, since we're not checking for low-level properties or hard-to-observe behavior. In general LIT tests are easier to write/maintain/understand and don't require recompiling, so I'd suggest to transform the existing unit test into something similar to tests in clang/test/ClangScanDeps/.

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

414

507

[cpsughrue]

Added llvm-lit test for commit and removed previously created unit test

[cpsughrue]

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

482

Is there a good way to validate cc1 command lines?

[Bigcheese]

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

482

Not really, you just have to parse it. Was there a particular reason you wanted to validate here?

[jansvoboda11]

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

482

I think that happens in ToolInvocation::run() that's called by createAndRunToolInvocation. Are you seeing cases where we don't emit a diagnostic for an invalid -cc1 command line?

clang/test/ClangScanDeps/Inputs/modules_cc1_cdb.json

1 ↗

(On Diff #545910)

I assume this was cargo-culted from clang/test/ClangScanDeps/Inputs/modules_cdb.json. I don't think we need multiple entries here and lots of the flags are unnecessary for just testing out -cc1 command lines work. I suggest having just one minimal -cc1 command line here.

clang/test/ClangScanDeps/modules-cc1.cpp

11

Recently, we've been using split-file to set up the file system for our tests. It's much easier to see what's going on in a glance. Can you transform the test into that format? You can take inspiration from clang/test/ClangScanDeps/modules-transitive.c for example.

19

Yeah, this is already covered by the other test, I suggest dropping this comment and the -j flag in the clang-scan-deps invocation below.

[cpsughrue]

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

482

@jansvoboda11 You're right, ToolInvocation::run() fails if there are any issues when the function creates the compiler invocation - I missed that. I've not seen any cases where diagnostics aren't outputted.

482

@Bigcheese No, I just wanted to make sure some sort of validation was happening and missed the work being done in ToolInvocation::run().

clang/test/ClangScanDeps/Inputs/modules_cc1_cdb.json

1 ↗

(On Diff #545910)

That is an accurate assumption. Are you good with the following command line clang -cc1 DIR/modules_cdb_input.cpp -IInputs -fimplicit-module-maps -o modules_cdb_input.o?

[cpsughrue]

Update llvm-lit test to use split-file

[jansvoboda11]

LGTM after Windows CI gets fixed.

[cpsughrue]

Fixing failed test on Windows CI

clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

if (llvm::Error E = driver::expandResponseFiles(Argv, CLMode, Alloc, FS)) {
     return false;
   }
 
   const std::unique_ptr<driver::Compilation> Compilation(
       Driver->BuildCompilation(llvm::ArrayRef(Argv)));
   if (!Compilation)
     return false;
 
+  if (Compilation->containsError())
+    return false;
+
   for (const driver::Command &Job : Compilation->getJobs()) {
     if (!Callback(Job))
       return false;
   }
   return true;
 }
 
+static bool createAndRunToolInvocation(
+    std::vector<std::string> CommandLine, DependencyScanningAction &Action,
+    FileManager &FM,
+    std::shared_ptr<clang::PCHContainerOperations> &PCHContainerOps,
+    DiagnosticsEngine &Diags, DependencyConsumer &Consumer) {
+
+  // Save executable path before providing CommandLine to ToolInvocation
+  std::string Executable = CommandLine[0];
+  ToolInvocation Invocation(std::move(CommandLine), &Action, &FM,
+                            PCHContainerOps);
+  Invocation.setDiagnosticConsumer(Diags.getClient());
+  Invocation.setDiagnosticOptions(&Diags.getDiagnosticOptions());
+  if (!Invocation.run())
+    return false;
+
+  std::vector<std::string> Args = Action.takeLastCC1Arguments();
+  Consumer.handleBuildCommand({std::move(Executable), std::move(Args)});
+  return true;
+}
+
 bool DependencyScanningWorker::computeDependencies(
     StringRef WorkingDirectory, const std::vector<std::string> &CommandLine,
     DependencyConsumer &Consumer, DependencyActionController &Controller,
     DiagnosticConsumer &DC, std::optional<StringRef> ModuleName) {
   // Reset what might have been modified in the previous worker invocation.
   BaseFS->setCurrentWorkingDirectory(WorkingDirectory);
 
   std::optional<std::vector<std::string>> ModifiedCommandLine;
bool DependencyScanningWorker::computeDependencies(
   Diags->setSourceManager(&SrcMgr);
   // DisableFree is modified by Tooling for running
   // in-process; preserve the original value, which is
   // always true for a driver invocation.
   bool DisableFree = true;
   DependencyScanningAction Action(WorkingDirectory, Consumer, Controller, DepFS,
                                   Format, OptimizeArgs, EagerLoadModules,
                                   DisableFree, ModuleName);
-  bool Success = forEachDriverJob(
+
+  bool Success = false;
+  if (FinalCommandLine[1] == "-cc1") {
+    Success = createAndRunToolInvocation(FinalCommandLine, Action, *FileMgr,
+                                         PCHContainerOps, *Diags, Consumer);
+  } else {
+    Success = forEachDriverJob(
         FinalCommandLine, *Diags, *FileMgr, [&](const driver::Command &Cmd) {
           if (StringRef(Cmd.getCreator().getName()) != "clang") {
             // Non-clang command. Just pass through to the dependency
             // consumer.
             Consumer.handleBuildCommand(
                 {Cmd.getExecutable(),
                  {Cmd.getArguments().begin(), Cmd.getArguments().end()}});
             return true;
           }
 
+          // Insert -cc1 comand line options into Argv
           std::vector<std::string> Argv;
           Argv.push_back(Cmd.getExecutable());
           Argv.insert(Argv.end(), Cmd.getArguments().begin(),
                       Cmd.getArguments().end());
 
           // Create an invocation that uses the underlying file
           // system to ensure that any file system requests that
           // are made by the driver do not go through the
           // dependency scanning filesystem.
-        ToolInvocation Invocation(std::move(Argv), &Action, &*FileMgr,
-                                  PCHContainerOps);
-        Invocation.setDiagnosticConsumer(Diags->getClient());
-        Invocation.setDiagnosticOptions(&Diags->getDiagnosticOptions());
-        if (!Invocation.run())
-          return false;
-
-        std::vector<std::string> Args = Action.takeLastCC1Arguments();
-        Consumer.handleBuildCommand({Cmd.getExecutable(), std::move(Args)});
-        return true;
+          return createAndRunToolInvocation(std::move(Argv), Action, *FileMgr,
+                                            PCHContainerOps, *Diags, Consumer);
         });
+  }
 
   if (Success && !Action.hasScanned())
     Diags->Report(diag::err_fe_expected_compiler_job)
         << llvm::join(FinalCommandLine, " ");
   return Success && Action.hasScanned();
 }
 
 DependencyActionController::~DependencyActionController() {}

clang/test/ClangScanDeps/modules-cc1.cpp

+// Check that clang-scan-deps works with cc1 command lines
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+
+//--- modules_cc1.cpp
+#include "header.h"
+
+//--- header.h
+
+//--- module.modulemap
+module header1 { header "header.h" }
+
+//--- cdb.json.template
+[{
+  "file": "DIR/modules_cc1.cpp",
+  "directory": "DIR",
+  "command": "clang -cc1 DIR/modules_cc1.cpp -fimplicit-module-maps -o modules_cc1.o"
+}]
+
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -j 1 -mode preprocess-dependency-directives > %t/result
+// RUN: cat %t/result | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+
+// CHECK: modules_cc1.o:
+// CHECK-NEXT: [[PREFIX]]/modules_cc1.cpp
+// CHECK-NEXT: [[PREFIX]]/module.modulemap
+// CHECK-NEXT: [[PREFIX]]/header.h