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

[Martin Sebor <msebor@gmail.com>]

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
>