jansvoboda11 added inline comments.
================
Comment at: clang/include/clang/Driver/Options.td:246
+class EmptyKPM<string base> : KeyPathAndMacro<"", "", ""> {}
class DiagnosticOpts<string base>
----------------
awarzynski wrote:
> @jansvoboda11 , is this the right approach here? I'd like use `BoolFOption`
> in Flang, but we don't need `KeyPathAndMacro` just yet. We may do in the
> future.
There was an `EmptyKPM` class present in the code before, but only as an
implementation detail of the marshalling system. People started using it with
`BoolFOption` to conveniently declare pairs Clang driver flags. I'm not a fan
since the whole `BooFOption` is designed around the keypath and its
marshalling. Putting a "null" keypath in `BoolFOption` and then talking about
its defaults and the effect of each flag seems highly unintuitive to me and
it's essentially dead code. I'd prefer a different approach in this patch
unless you have plans to switch to the marshalling infrastructure and remove
`EmptyKPM` in the short term.
Would something similar to `OptInFFlag` and `OptOutFFlag` work for you? You
could generalize it (remove `Flags<[CC1Option]>`), make use of it for Flang's
options directly and forward the current `Opt{In,Out}FFlag` to the
generalization while specifying the `CC1Option`. WDYT?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105881/new/
https://reviews.llvm.org/D105881
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
