[PATCH] D121997: [clang][driver] Fix compilation database dump with multiple architectures

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

The "Fix compilation database dump" changes LGTM, assuming you split out `-driver-only` and land it separately ahead of the rest of this.

In D121997#3395562 <https://reviews.llvm.org/D121997#3395562>, @jansvoboda11 wrote:

> Introduce `-driver-only` to avoid running `-cc1`s.

`-driver-only` looks mostly great, but should really be committed (and tested) separately.

I added a comment inline about `-v` / CCPrintOptions.

For the testing, I suggest:

- Add driver-only.c, where you can test a normal compilation (unrelated to `-MJ`).
  - If you implement `-v`, you can check its output.
  - Add a `RUN: not %clang -driver-only` in some case where it should fail.
- If you implement CCPrintOptions with `-v`, update clang/test/Driver/cc-print-options.c with an additional `RUN` line.
- Update clang/test/Driver/gen-cdb-fragment.c to use `-driver-only` and remove `REQUIRES: x86-registered-target`.



================
Comment at: clang/lib/Driver/Driver.cpp:1600-1607
+  if (C.getArgs().hasArg(options::OPT_driver_only))
+    return 0;
+
   // Just print if -### was present.
   if (C.getArgs().hasArg(options::OPT__HASH_HASH_HASH)) {
     C.getJobs().Print(llvm::errs(), "\n", true);
     return 0;
----------------
I think if `OPT_v` then `OPT_driver_only` should call `C.getJobs().Print()` somehow... ideally, using the logic that's at the top of `Compilation::ExecuteCommand()`, also implementing the analogous CCPrintOptions, but that looks potentially awkward... you maybe want to call `ExecuteJobs` below with a flag that says "don't execute" or something.

Happy for those things to come in a follow-up though; just leave behind FIXME(s) if so.


================
Comment at: clang/lib/Driver/Driver.cpp:1609-1611
   // If there were errors building the compilation, quit now.
   if (Diags.hasErrorOccurred())
     return 1;
----------------
`-driver-only` should `return 1` in this case. Please add a test for this as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121997