[PATCH] D105881: [flang][driver] Switch to `BoolFOption` for boolean options

[Jan Svoboda via Phabricator via cfe-commits]

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