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