Lewis Hyatt <lhy...@gmail.com> writes:
> On Tue, Mar 17, 2020 at 11:52:13AM +0000, Richard Sandiford wrote:
>> Lewis Hyatt <lhy...@gmail.com> writes:
>> > On Mon, Mar 16, 2020 at 06:11:08PM +0000, Richard Sandiford wrote:
>> >> Lewis Hyatt via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
>> > ...
>> >> > FWIW there are three other options currently affected by this change
>> >> > (-Wimplicit-fallthrough, -fcf-protection, and -flive-patching). The
>> >> > change for
>> >> > -Wimplicit-fallthrough I think is particularly helpful:
>> >> >
>> >> > -Wimplicit-fallthrough Same as -Wimplicit-fallthrough=. Use the
>> >> > latter option instead.
>> >> > becomes
>> >> > -Wimplicit-fallthrough Same as -Wimplicit-fallthrough=3 (or, in
>> >> > negated form, -Wimplicit-fallthrough=0).
>> >>
>> >> I also see:
>> >>
>> >> - -ftail-call-workaround Same as -ftail-call-workaround=. Use the
>> >> latter option instead.
>> >> + -ftail-call-workaround Same as -ftail-call-workaround=1 (or, in
>> >> negated form, -ftail-call-workaround=0).
>> >> -ftail-call-workaround=<0,2> Disallow tail call optimization when a
>> >> calling routine may have omitted character lengths.
>> >> ...
>> >> --imacros Same as -imacros. Use the latter option
>> >> instead.
>> >> --imacros= Same as -imacros. Use the latter option
>> >> instead.
>> >> --include Same as -include. Use the latter option
>> >> instead.
>> >> - --include-barrier Same as -I. Use the latter option instead.
>> >> + --include-barrier Same as -I-.
>> >> --include-directory Same as -I. Use the latter option instead.
>> >> --include-directory-after Same as -idirafter. Use the latter option
>> >> instead.
>> >> --include-directory-after= Same as -idirafter. Use the latter option
>> >> instead.
>> >> ...
>> >> - -Wnormalized Same as -Wnormalized=. Use the latter
>> >> option instead.
>> >> + -Wnormalized Same as -Wnormalized=nfc (or, in negated
>> >> form, -Wnormalized=none).
>> >> -Wnormalized=[none|id|nfc|nfkc] Warn about non-normalized Unicode
>> >> strings.
>> >>
>> >> I agree all of these look like improvements, especially the
>> >> --include-barrier one. But I think the include ones also show
>> >> that the "Use the latter option instead." decision is independent
>> >> of whether the option is defined to be an alias.
>>
>> Gah, I meant an Alias() with an argument here.
>>
>> >>
>> >> FWIW, there's also:
>> >>
>> >> Wmissing-format-attribute
>> >> C ObjC C++ ObjC++ Warning Alias(Wsuggest-attribute=format)
>> >> ;
>> >>
>> >> which still ends up as:
>> >>
>> >> -Wmissing-format-attribute Same as -Wsuggest-attribute=format. Use
>> >> the latter option instead.
>> >>
>> >> Not really my area though, so I don't have any specific suggestion
>> >> about how to separate the cases.
>> >>
>> >
>> > Right, sorry, in my first email I only mentioned the options output by
>> > --help=common, but there were a few more as well. Thanks very much for
>> > trying
>> > it out and for the feedback.
>> >
>> > The rule I implemented was to change the help output for all alias options
>> > with no documentation if they also specify the extra 2nd option (or 2nd and
>> > 3rd options) to the Alias directive. For example, -include-barrier is like
>> > this:
>> >
>> > -include-barrier C ObjC C++ ObjC++ Alias(I, -)
>> >
>> > It serves to provide the argument '-' to the option '-I', so it is
>> > eligible for
>> > the new text. The others are like this one:
>> >
>> > -include-directory-after C ObjC C++ ObjC++ Separate Alias(idirafter)
>> > MissingArgError(missing path after %qs)
>> >
>> > Since that one doesn't pass the extra args to Alias, I interpreted it to
>> > mean this is the case for which the "Use the latter option" directive was
>> > intended to apply. (-idirafter has been designated as preferable to
>> > -include-directory-after).
>>
>> Yeah, I get why you did it like this. It's just that the end effect
>> is to make --include-barrier seem less disparaged than the other
>> --include-* options, but I'm not sure there's supposed to be any
>> difference between them in that respect.
>>
>> Perhaps we should drop the "Use the latter option instead." thing
>> altogether for aliases. I'm not sure that it really helps, and this
>> thread shows that adding it automatically can lead to some odd corner
>> cases.
>>
>> In practice we shouldn't remove any of these aliases unless we're
>> also removing the option that they're an alias of. And if we do that,
>> the options should go through the usual deprecation cycle, just like
>> options without aliases.
>>
>> If there are specific options that we want to steer users away
>> from without deprecation, then we should probably have a specific
>> tag for that.
>
> Thanks, it makes sense to me. That would amount to changing just one line of
> the patch then, so it would look instead like the attached.
>
> -Lewis
>
> gcc/ChangeLog:
>
> 2020-03-19 Lewis Hyatt <lhy...@gmail.com>
>
> * opts.c (print_filtered_help): Improve the help text for alias
> options.
LGTM, thanks. OK if noone objects in 48 hrs.
Richard
>
> commit 7a74dd55098e2ec8c2b87dc172ac34f91eefc0c1
> Author: Lewis Hyatt <lhy...@gmail.com>
> Date: Wed Feb 12 13:52:28 2020 -0500
>
> driver: Improve the generated help text for alias options
>
> diff --git a/gcc/opts.c b/gcc/opts.c
> index ac160ed8404..5dc7d65dedd 100644
> --- a/gcc/opts.c
> +++ b/gcc/opts.c
> @@ -1315,14 +1315,31 @@ print_filtered_help (unsigned int include_flags,
> if (option->alias_target < N_OPTS
> && cl_options [option->alias_target].help)
> {
> + const struct cl_option *target = cl_options + option->alias_target;
> if (option->help == NULL)
> {
> - /* For undocumented options that are aliases for other options
> - that are documented, point the reader to the other option in
> - preference of the former. */
> - snprintf (new_help, sizeof new_help,
> - _("Same as %s. Use the latter option instead."),
> - cl_options [option->alias_target].opt_text);
> + /* The option is undocumented but is an alias for an option that
> + is documented. If the option has alias arguments, then its
> + purpose is to provide certain arguments to the other option, so
> + inform the reader of this. Otherwise, point the reader to the
> + other option in preference to the former. */
> +
> + if (option->alias_arg)
> + {
> + if (option->neg_alias_arg)
> + snprintf (new_help, sizeof new_help,
> + _("Same as %s%s (or, in negated form, %s%s)."),
> + target->opt_text, option->alias_arg,
> + target->opt_text, option->neg_alias_arg);
> + else
> + snprintf (new_help, sizeof new_help,
> + _("Same as %s%s."),
> + target->opt_text, option->alias_arg);
> + }
> + else
> + snprintf (new_help, sizeof new_help,
> + _("Same as %s."),
> + target->opt_text);
> }
> else
> {
