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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
