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

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:482
+  bool Success = false;
+  if (FinalCommandLine[1] == "-cc1") {
+    Success = createAndRunToolInvocation(FinalCommandLine, Action, *FileMgr,
----------------
cpsughrue wrote:
> Is there a good way to validate cc1 command lines?
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?


================
Comment at: 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.


================
Comment at: clang/test/ClangScanDeps/modules-cc1.cpp:10
+// 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
----------------
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.


================
Comment at: clang/test/ClangScanDeps/modules-cc1.cpp:18
+//
+// 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
----------------
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.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D156234/new/

https://reviews.llvm.org/D156234