[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

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

10

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.

18

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

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/Inputs/modules_cc1_cdb.json

+[
+{
+  "directory": "DIR",
+  "command": "clang -cc1 -E DIR/modules_cdb_input2.cpp -IInputs -D INCLUDE_HEADER2 -fmodules -MT modules_cdb_input2.o -dependency-file DIR/modules_cdb2.d -fmodules-cache-path=DIR/module-cache -fcoverage-compilation-dir=DIR -fimplicit-module-maps -fmodules-validate-system-headers",
+  "file": "DIR/modules_cdb_input2.cpp"
+},
+{
+  "directory": "DIR",
+  "command": "clang -cc1 -E DIR/modules_cdb_input.cpp -IInputs -fmodules -fmodules-cache-path=DIR/module-cache -fcoverage-compilation-dir=DIR -fdebug-compilation-dir=DIR -fimplicit-module-maps -fmodules-validate-system-headers",
+  "file": "DIR/modules_cdb_input.cpp"
+},
+{
+  "directory": "DIR",
+  "command": "clang -cc1 -E DIR/modules_cdb_input.cpp -IInputs -fmodules -fmodules-cache-path=DIR/module-cache -fcoverage-compilation-dir=DIR -fdebug-compilation-dir=DIR -fimplicit-module-maps -fmodules-validate-system-headers -o a.o",
+  "file": "DIR/modules_cdb_input.cpp"
+},
+{
+  "directory": "DIR",
+  "command": "clang -cc1 -E DIR/modules_cdb_input.cpp -IInputs -fmodules -fmodules-cache-path=DIR/module-cache -fcoverage-compilation-dir=DIR -fdebug-compilation-dir=DIR -fimplicit-module-maps -fmodules-validate-system-headers -o b.o",
+  "file": "DIR/modules_cdb_input.cpp"
+}
+]

clang/test/ClangScanDeps/modules-cc1.cpp

+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: rm -rf %t_clangcl.cdb
+// RUN: rm -rf %t.module-cache
+// RUN: rm -rf %t.module-cache_clangcl
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/modules_cdb_input.cpp
+// RUN: cp %s %t.dir/modules_cdb_input2.cpp
+// RUN: mkdir %t.dir/Inputs
+// RUN: cp %S/Inputs/header.h %t.dir/Inputs/header.h
+// RUN: cp %S/Inputs/header2.h %t.dir/Inputs/header2.h
+// RUN: cp %S/Inputs/module.modulemap %t.dir/Inputs/module.modulemap
+// RUN: sed -e "s|DIR|%/t.dir|g" %S/Inputs/modules_cc1_cdb.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -mode preprocess-dependency-directives | \
+// RUN:   FileCheck --check-prefixes=CHECK1,CHECK2,CHECK2NO %s
+//
+// The output order is non-deterministic when using more than one thread,
+// so check the output using two runs. Note that the 'NOT' check is not used
+// as it might fail if the results for `modules_cdb_input.cpp` are reported before
+// `modules_cdb_input2.cpp`.
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess-dependency-directives | \
+// RUN:   FileCheck --check-prefix=CHECK1 %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess | \
+// RUN:   FileCheck --check-prefix=CHECK1 %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess-dependency-directives | \
+// RUN:   FileCheck --check-prefix=CHECK2 %s
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 2 -mode preprocess | \
+// RUN:   FileCheck --check-prefix=CHECK2 %s
+
+#include "header.h"
+
+// CHECK1: modules_cdb_input2.o:
+// CHECK1-NEXT: modules_cdb_input2.cpp
+// CHECK1-NEXT: Inputs{{/|\\}}module.modulemap
+// CHECK1-NEXT: Inputs{{/|\\}}header2.h
+// CHECK1: Inputs{{/|\\}}header.h
+
+// CHECK2: {{(modules_cdb_input)|(a)|(b)}}.o:
+// CHECK2-NEXT: modules_cdb_input.cpp
+// CHECK2-NEXT: Inputs{{/|\\}}module.modulemap
+// CHECK2-NEXT: Inputs{{/|\\}}header.h
+// CHECK2NO-NOT: header2
 No newline at end of file