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

[Connor Sughrue via Phabricator via cfe-commits]

cpsughrue added inline comments.


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:482
+  bool Success = false;
+  if (FinalCommandLine[1] == "-cc1") {
+    Success = createAndRunToolInvocation(FinalCommandLine, Action, *FileMgr,
----------------
Bigcheese wrote:
> jansvoboda11 wrote:
> > 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?
> Not really, you just have to parse it. Was there a particular reason you wanted to validate here?
@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.


================
Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp:482
+  bool Success = false;
+  if (FinalCommandLine[1] == "-cc1") {
+    Success = createAndRunToolInvocation(FinalCommandLine, Action, *FileMgr,
----------------
cpsughrue wrote:
> Bigcheese wrote:
> > jansvoboda11 wrote:
> > > 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?
> > Not really, you just have to parse it. Was there a particular reason you wanted to validate here?
> @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.
@Bigcheese No, I just wanted to make sure some sort of validation was happening and missed the work being done in `ToolInvocation::run()`.


================
Comment at: clang/test/ClangScanDeps/Inputs/modules_cc1_cdb.json:1
+[
+{
----------------
jansvoboda11 wrote:
> 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.
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`?


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

https://reviews.llvm.org/D156234