[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 wrote:
> > 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?
> Thanks for the quick feedback! Initially when I saw `CC1Option` in `CC1Option`/`OptOutFFlag`, I immediately decided to explore alternatives. But I agree with everything that you said and am happy to generalize these multiclasses (instead of adding `EmptyKPM`). I don't see why it wouldn't work for what we currently need in Flang. I should be able to update this patch tomorrow.
> 
> Btw, what's the key benefit of the marshaling infra? Is that for being able to restore the original compiler invocation from instances of `CompilerInvocation`? We are still busy with other things, but definitely don't want Flang to miss out here.
Cool, happy to help.

We're using marshalling for "implicitly discovered, explicitly built Clang modules" via `clang-scan-deps`. We're able to change the `CompilerInvocation` of the original TU for the purpose of explicit build of the discovered modules, serialize it to `clang -cc1` command-line and report it to the build system.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881