[PATCH] driver: Improve the generated help text for alias options with arguments

[PATCH] driver: Improve the generated help text for alias options with arguments

[Lewis Hyatt]

[-- Attachment #1: Type: text/plain, Size: 2435 bytes --]

Hello-

Currently, if an option is both undocumented and an alias for a second option,
the help text generated by gcc --help directs the reader to use the other
option instead. This makes sense for deprecated options, but it seems this
pattern is also used for another case, namely when an option exists so as to
pass a default argument to another option. For instance this one, from
common.opt:

-------------
fdiagnostics-color
Common Alias(fdiagnostics-color=,always,never)
;

fdiagnostics-color=
Driver Common Joined RejectNegative Var(flag_diagnostics_show_color) Enum(diagnostic_color_rule) Init(DIAGNOSTICS_COLOR_NO)
-fdiagnostics-color=[never|always|auto]	Colorize diagnostics.
-------------

This is nice because it means you can say -fdiagnostics-color as a shorthand
for -fdiagnostics-color=always, or -fno-diagnostics-color as a shorthand for
-fdiagnostics-color=never. However, the generated help text does not describe
it this way:

-------------
$ gcc --help=common | grep fdiagnostics-color
  -fdiagnostics-color         Same as -fdiagnostics-color=.  Use the latter option instead.
  -fdiagnostics-color=[never|always|auto] Colorize diagnostics.
-------------

Perhaps I am wrong and the non-argument usage is indeed meant to be deprecated,
but it feels more like it was intended as a convenience and could be documented
as such. What actually prompted this patch is that I am adding a new option for
GCC 11 with these never/always/auto semantics and I am a bit confused whether I
am supposed to add the aliased version or not. Feels like it's nice to add it,
but then the --help output says the opposite...

Anyway, the attached patch would change the help output to the following... If
that seems to be an improvement and closer to the intended behavior, please let
me know. Thanks!

-------------
  -fdiagnostics-color         Same as -fdiagnostics-color=always (or, in negated form, -fdiagnostics-color=never).
  -fdiagnostics-color=[never|always|auto] Colorize diagnostics.
-------------

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).

-Lewis

[-- Attachment #2: ChangeLog.txt --]
[-- Type: text/plain, Size: 135 bytes --]

2020-03-15  Lewis Hyatt  <lhyatt@gmail.com>

	* opts.c (print_filtered_help): Improve the help text for alias options
	with arguments.

[-- Attachment #3: alias_opt.patch --]
[-- Type: text/plain, Size: 1591 bytes --]

diff --git a/gcc/opts.c b/gcc/opts.c
index ac160ed8404..a120858d77b 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.  Use the latter option instead."),
+			  target->opt_text);
 	    }
 	  else
 	    {

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Richard Sandiford]

Lewis Hyatt via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Hello-
>
> Currently, if an option is both undocumented and an alias for a second option,
> the help text generated by gcc --help directs the reader to use the other
> option instead. This makes sense for deprecated options, but it seems this
> pattern is also used for another case, namely when an option exists so as to
> pass a default argument to another option. For instance this one, from
> common.opt:
>
> -------------
> fdiagnostics-color
> Common Alias(fdiagnostics-color=,always,never)
> ;
>
> fdiagnostics-color=
> Driver Common Joined RejectNegative Var(flag_diagnostics_show_color) Enum(diagnostic_color_rule) Init(DIAGNOSTICS_COLOR_NO)
> -fdiagnostics-color=[never|always|auto]	Colorize diagnostics.
> -------------
>
> This is nice because it means you can say -fdiagnostics-color as a shorthand
> for -fdiagnostics-color=always, or -fno-diagnostics-color as a shorthand for
> -fdiagnostics-color=never. However, the generated help text does not describe
> it this way:
>
> -------------
> $ gcc --help=common | grep fdiagnostics-color
>   -fdiagnostics-color         Same as -fdiagnostics-color=.  Use the latter option instead.
>   -fdiagnostics-color=[never|always|auto] Colorize diagnostics.
> -------------
>
> Perhaps I am wrong and the non-argument usage is indeed meant to be deprecated,
> but it feels more like it was intended as a convenience and could be documented
> as such. What actually prompted this patch is that I am adding a new option for
> GCC 11 with these never/always/auto semantics and I am a bit confused whether I
> am supposed to add the aliased version or not. Feels like it's nice to add it,
> but then the --help output says the opposite...
>
> Anyway, the attached patch would change the help output to the following... If
> that seems to be an improvement and closer to the intended behavior, please let
> me know. Thanks!
>
> -------------
>   -fdiagnostics-color         Same as -fdiagnostics-color=always (or, in negated form, -fdiagnostics-color=never).
>   -fdiagnostics-color=[never|always|auto] Colorize diagnostics.
> -------------
>
> 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.

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.

Thanks,
Richard

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Lewis Hyatt]

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.
> 
> 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).

Regarding -Wmissing-format-attribute, that is an interesting case, it's the only
instance in any *.opt that has an = sign in the Alias target. If I understand
correctly, this one can't use the 3-argument form of Alias() because
-Wno-missing-format-attribute is actually an alias for
-Wno-suggest-attribute=format, rather than -Wsuggest-attribute=<something>. I
think that one could be left as is or else my patch would need to check for an =
sign in the alias target and handle this case as well, but that may lead to
other surprises in the future.

Thanks!

-Lewis

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Lewis Hyatt]

On Mon, Mar 16, 2020 at 6:11 PM Lewis Hyatt <lhyatt@gmail.com> wrote:
> Regarding -Wmissing-format-attribute, that is an interesting case, it's the only
> instance in any *.opt that has an = sign in the Alias target. If I understand
> correctly, this one can't use the 3-argument form of Alias() because
> -Wno-missing-format-attribute is actually an alias for
> -Wno-suggest-attribute=format, rather than -Wsuggest-attribute=<something>. I
> think that one could be left as is or else my patch would need to check for an =
> sign in the alias target and handle this case as well, but that may lead to
> other surprises in the future.

Sorry, please disregard the above paragraph, that was my mistake...
Those kinds of aliases with an = are used frequently. I guess the
current behavior is probably best for them.

-Lewis

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Richard Sandiford]

Lewis Hyatt <lhyatt@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,
Richard

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Lewis Hyatt]

[-- Attachment #1: Type: text/plain, Size: 4782 bytes --]

On Tue, Mar 17, 2020 at 11:52:13AM +0000, Richard Sandiford wrote:
> Lewis Hyatt <lhyatt@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

[-- Attachment #2: ChangeLog.txt --]
[-- Type: text/plain, Size: 136 bytes --]

gcc/ChangeLog:

2020-03-19  Lewis Hyatt  <lhyatt@gmail.com>

	* opts.c (print_filtered_help): Improve the help text for alias
	options.

[-- Attachment #3: alias_opt_v2.patch --]
[-- Type: text/x-patch, Size: 1753 bytes --]

commit 7a74dd55098e2ec8c2b87dc172ac34f91eefc0c1
Author: Lewis Hyatt <lhyatt@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
 	    {

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Richard Sandiford]

Lewis Hyatt <lhyatt@gmail.com> writes:
> On Tue, Mar 17, 2020 at 11:52:13AM +0000, Richard Sandiford wrote:
>> Lewis Hyatt <lhyatt@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  <lhyatt@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 <lhyatt@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
>  	    {

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Martin Sebor]

On 3/17/20 5:52 AM, Richard Sandiford wrote:
> Lewis Hyatt <lhyatt@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.

The "Use the latter option" text was the outcome of the discussion
of the patch for PR 68043:
https://gcc.gnu.org/legacy-ml/gcc-patches/2015-10/msg01395.html
where Joseph wanted to steer users toward the alternatives.  I
don't feel too strongly about it but reviewing the thread might
be helpful.

FWIW, an enhancement to consider is making use of colors in
the output (under the same conditions as in diagnostics).  That
would make it possible to differentiate between recommended
options and their discouraged or deprecated counterparts (among
other things).

Martin

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Richard Sandiford]

Martin Sebor <msebor@gmail.com> writes:
> On 3/17/20 5:52 AM, Richard Sandiford wrote:
>> Lewis Hyatt <lhyatt@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.
>
> The "Use the latter option" text was the outcome of the discussion
> of the patch for PR 68043:
> https://gcc.gnu.org/legacy-ml/gcc-patches/2015-10/msg01395.html
> where Joseph wanted to steer users toward the alternatives.  I
> don't feel too strongly about it but reviewing the thread might
> be helpful.

Thanks for the pointer.  But I think Joseph's comment was more about
not reproducing the documentation of the alias target:

  I also think it would be better just to give the "Same as" message
  without also repeating the description of the canonical option.

on the basis that:

  Well, I think it might also encourage people to use the aliases, when
  for the most part we'd rather people used the canonical names (and so
  made it easier e.g. to search for other uses of the same option).

And Lewis's patch is still doing that.  It doesn't look like there was
a specific request to add extra text to steer the user away from the alias.

I can understand why adding that text seemed like a good idea,
but I think Lewis's patch shows that it can also produce some oddities.
IMO we should stick to what we know is correct: that the option is an
alias of some other option.

> FWIW, an enhancement to consider is making use of colors in
> the output (under the same conditions as in diagnostics).  That
> would make it possible to differentiate between recommended
> options and their discouraged or deprecated counterparts (among
> other things).

Yeah, sounds like it could be useful.

Thanks,
Richard

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Martin Sebor]

On 3/20/20 11:46 AM, Richard Sandiford wrote:
> Martin Sebor <msebor@gmail.com> writes:
>> On 3/17/20 5:52 AM, Richard Sandiford wrote:
>>> Lewis Hyatt <lhyatt@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.
>>
>> The "Use the latter option" text was the outcome of the discussion
>> of the patch for PR 68043:
>> https://gcc.gnu.org/legacy-ml/gcc-patches/2015-10/msg01395.html
>> where Joseph wanted to steer users toward the alternatives.  I
>> don't feel too strongly about it but reviewing the thread might
>> be helpful.
> 
> Thanks for the pointer.  But I think Joseph's comment was more about
> not reproducing the documentation of the alias target:
> 
>    I also think it would be better just to give the "Same as" message
>    without also repeating the description of the canonical option.
> 
> on the basis that:
> 
>    Well, I think it might also encourage people to use the aliases, when
>    for the most part we'd rather people used the canonical names (and so
>    made it easier e.g. to search for other uses of the same option).
> 
> And Lewis's patch is still doing that.  It doesn't look like there was
> a specific request to add extra text to steer the user away from the alias.
> 
> I can understand why adding that text seemed like a good idea,
> but I think Lewis's patch shows that it can also produce some oddities.
> IMO we should stick to what we know is correct: that the option is an
> alias of some other option.

Sure.  I have no objection.  I just wanted to give some background
on why the code is the way it is.

FWIW, I expect the majority or users, especially those looking for
help with options, will refer to the more verbose text in the manual
far more often than to the --help output.  I think any guidance we
give will be more effective in the former.

Martin

> 
>> FWIW, an enhancement to consider is making use of colors in
>> the output (under the same conditions as in diagnostics).  That
>> would make it possible to differentiate between recommended
>> options and their discouraged or deprecated counterparts (among
>> other things).
> 
> Yeah, sounds like it could be useful.
> 
> Thanks,
> Richard
> 

Re: [PATCH] driver: Improve the generated help text for alias options with arguments

[Lewis Hyatt]

On Fri, Mar 20, 2020 at 01:16:42PM -0600, Martin Sebor wrote:
> On 3/20/20 11:46 AM, Richard Sandiford wrote:
> > Martin Sebor <msebor@gmail.com> writes:
> > > On 3/17/20 5:52 AM, Richard Sandiford wrote:
> > > > Lewis Hyatt <lhyatt@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.
> > > 
> > > The "Use the latter option" text was the outcome of the discussion
> > > of the patch for PR 68043:
> > > https://gcc.gnu.org/legacy-ml/gcc-patches/2015-10/msg01395.html
> > > where Joseph wanted to steer users toward the alternatives.  I
> > > don't feel too strongly about it but reviewing the thread might
> > > be helpful.
> > 
> > Thanks for the pointer.  But I think Joseph's comment was more about
> > not reproducing the documentation of the alias target:
> > 
> >    I also think it would be better just to give the "Same as" message
> >    without also repeating the description of the canonical option.
> > 
> > on the basis that:
> > 
> >    Well, I think it might also encourage people to use the aliases, when
> >    for the most part we'd rather people used the canonical names (and so
> >    made it easier e.g. to search for other uses of the same option).
> > 
> > And Lewis's patch is still doing that.  It doesn't look like there was
> > a specific request to add extra text to steer the user away from the alias.
> > 
> > I can understand why adding that text seemed like a good idea,
> > but I think Lewis's patch shows that it can also produce some oddities.
> > IMO we should stick to what we know is correct: that the option is an
> > alias of some other option.
> 
> Sure.  I have no objection.  I just wanted to give some background
> on why the code is the way it is.
> 
> FWIW, I expect the majority or users, especially those looking for
> help with options, will refer to the more verbose text in the manual
> far more often than to the --help output.  I think any guidance we
> give will be more effective in the former.

Yeah, agreed that most users would be looking at the manual. Hopefully this is
still a small improvement. Thank you both for considering it! I will
plan to push it in a couple days unless I hear otherwise. 

-Lewis