How to handle -f flags

There are some special -f options that are not really flags. For
example, -fprofile-dir=PATH. It takes a value and there is no such
thing as -fno-profile-dir. For these special cases it looks like we
have a fairly reasonable infrastructure, since they will need some
custom handling anyway.

But the vast majority are just simple flags that have the form of the
pair -ffoo and -fno-foo. Right now we have quiet a large amount of
boilerplate to implement those.

* Add both to Options.td:

def fshow_column : Flag<["-"], "fshow-column">, Group<f_Group>,
Flags<[CC1Option]>;
def fno_show_column : Flag<["-"], "fno-show-column">, Group<f_Group>,
Flags<[CC1Option]>,
  HelpText<"Do not include column number on diagnostics">;

* Add it to some .def file.

DIAGOPT(ShowColumn, 1, 1) /// Show column number on diagnostics.

* Parse the option:

Opts.ShowColumn = Args.hasFlag(OPT_fshow_column,
                                 OPT_fno_show_column,
                                 /*Default=*/true);

* Actually use the option for what it is intended.

Because of this, most -f options have some bug in their
implementation. In the above fshow-column example the help text is out
of sync, the negative of -ffunction-sections is missing completely,
etc.

I think that to fix this we need something more like the way we handle warnings:

* Add a catch all -f* option to Options.td.
* Add a include/Basic/FFlags.td with entries like

def show_column : FFlag<"Include column number on diagnostics", 1>;

Where 1 is the default value.

* Have tablegen produce a FFlags.def file and a function for setting
the flags from the command line options.

* Produce a -f specific error if we see one that we don't know what it
is ("unknown flag: 'foobar'", instead of "unknown argument:
'-ffoobar'").

Can anyone see a problem with this? The main difference (other than
the refactoring) would be that all the f flags are in singe .def file
and object, not split in CodeGenOptions.def/CodeGenOptions,
DiagnosticOptions.def/DiagnosticOptions, etc.

Cheers,
Rafael

How about we model “-f” as a prefix similar to “–”, and then use some tablegen construct to create the -f and -fno- versions, as well as potentially populating a .def file from it like you suggest?

The only problem I see with this is that I commonly search the .td files for “ffoo”, and that wouldn’t fire anymore.

I don't think this part of it is really an issue... this plus the bit of
code in lib/Frontend is sort of boilerplate, but I don't think it's worth
increasing the scope of the patch to cover it. I would lean towards
keeping the options in the driver.

-Eli

* Add both to Options.td:

def fshow_column : Flag<["-"], "fshow-column">, Group<f_Group>,
Flags<[CC1Option]>;
def fno_show_column : Flag<["-"], "fno-show-column">, Group<f_Group>,
Flags<[CC1Option]>,
  HelpText<"Do not include column number on diagnostics">;

* Add it to some .def file.

DIAGOPT(ShowColumn, 1, 1) /// Show column number on diagnostics.

I don't think this part of it is really an issue... this plus the bit of
code in lib/Frontend is sort of boilerplate, but I don't think it's worth
increasing the scope of the patch to cover it. I would lean towards keeping
the options in the driver.

I agree that .td is currently the weakest link and I would try to fix
that first. My inclination was to pass all -f options to -cc1 since it
will have to parse them anyway, but I could also check them in the
driver just to make sure all options are valid and not even run -cc1
if they are not.

What advantage do you see in parsing the -f options in the driver?

Cheers,
Rafael

-Eli