dexonsmith 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;
----------------
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`).
================
Comment at: clang/lib/Frontend/CompilerInvocation.cpp:3212
+
+#define DIAG_OPTION_WITH_MARSHALLING OPTION_WITH_MARSHALLING
+
----------------
Making `DiagnosticOpts->` implicit requires a little more code here:
```
#define DIAG_OPTION_WITH_MARSHALLING( \
PREFIX_TYPE, NAME, ID, KIND, GROUP, ALIAS, ALIASARGS, FLAGS, PARAM, \
HELPTEXT, METAVAR, VALUES, SPELLING, ALWAYS_EMIT, KEYPATH, DEFAULT_VALUE, \
IMPLIED_CHECK, IMPLIED_VALUE, NORMALIZER, DENORMALIZER, MERGER, EXTRACTOR, \
TABLE_INDEX) \
GENERATE_OPTION_WITH_MARSHALLING( \
Args, SA, KIND, FLAGS, SPELLING, ALWAYS_EMIT, DiagnosticOpts->KEYPATH, \
DEFAULT_VALUE, IMPLIED_CHECK, IMPLIED_VALUE, DENORMALIZER, EXTRACTOR, \
TABLE_INDEX)
```
But I think it might be worth it for the sanity check.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84673/new/
https://reviews.llvm.org/D84673
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
