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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
