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

Wed Aug 4 12:31:15 PDT 2021

awarzynski added a comment.

In D105881#2919575 <https://reviews.llvm.org/D105881#2919575>, @jansvoboda11 wrote:

> The `clang` driver always accepts both the positive and negative options, and it always only considers the argument that appeared last on the command-line (using `ArgList::getLastArg(Pos, Neg)`). This allows users to take a random `clang` driver command-line, append `-feliminate-unused-debug-types` and get the desired behavior, even though the original command-line might have contained `-fno-eliminate-unused-debug-types`.
>
> For the pair of options you mentioned, the `clang` driver knows they are "opt-out" in `clang -cc1`. The behavior is enabled by default in `-cc1` and can be disabled/opted-out through the negative `-fno-eliminate-unused-debug-types`. The positive option is **not** accepted by `clang -cc1`, since explicitly enabling a behavior that's already enabled by default is redundant.
>
> If driver sees `-fno-eliminate-unused-debug-types` as the last option, it will forward it to `clang -cc1`. If it sees `-feliminate-unused-debug-types`, it will ignore it.
>
> Does this make sense?

Yes. Thanks for the context and the rationale! This is much clearer now and indeed, makes sense. It's hard to extract this stuff from `Options.td` so I really appreciate you taking the time to explain it.

> The `HelpText` doesn't play a role in option being accepted or not in `clang` or `clang -cc1`.

We are 100% in agreement with regard to `HelpText`.

> I think we're in agreement that the current distinction between `HelpText<"">` and no `HelpText` is not particularly useful when printing help.

Yes. Updating printHelp <https://github.com/llvm/llvm-project/blob/main/llvm/lib/Option/OptTable.cpp#L575> wouldn't be too difficult. Would you be in favor?

> I agree we shouldn't mix Clang and Flang options together in `-help`. But to make sure we're on the same page: this doesn't have to do anything with the presence/absence of `HelpText`, this is controlled by `FlangOption`, `FC1Option`, `CC1Option` etc.

Agreed.

I've experimented with a few more approaches and feel that the cleanest approach would be to:

- rename `OptOutFFlag`/`OptInFFlag` as `OptOutCC1FFlag` and `OptInCC1FFlag`
- introduce `OptOutFC1FFlag` and `OptInFC1FFlag`.

We will end-up with a bit of duplication in Options.td, but the long term goal is to split it into multiple files anyway. Also, I think that in this case code re-use would lead to a rather convoluted implementation. I'm will send an updated patch shortly.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105881