[PATCH] D84673: [clang][cli] Port DiagnosticOpts to new option parsing system

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.


================
Comment at: clang/include/clang/Driver/Options.td:1193-1195
+defm caret_diagnostics : BoolFOption<"caret-diagnostics",
+  "DiagnosticOpts->ShowCarets", DefaultsToTrue,
+  ChangedBy<NegFlag>, ResetBy<PosFlag>>, IsDiag;
----------------
dexonsmith wrote:
> There was one thing in the original patch that was a bit of sanity check for whether `IsDiag` was added to the wrong option: the `DiagnosticOpts->` part of the keypath was implicit. What do you think of adding that back? That would make the keypath here `ShowCarets`.
> 
> It highlights that diagnostics are special (since `DiagnosticOpts` is never mentioned in the file, and any mistakes could be found with a `grep`).
I agree that now, when we don't use the keypath as the "source of truth", the original solution where `DiagnosticOpts->` is implied by something else is cleaner. Thanks for pointing that out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84673