From: Lewis Hyatt <lhyatt@gmail.com>
To: gcc-patches@gcc.gnu.org
Subject: Re: [PATCH] driver: Improve the generated help text for alias options with arguments
Date: Fri, 20 Mar 2020 08:24:42 -0400 [thread overview]
Message-ID: <20200320122442.GA1620@ldh.local> (raw)
In-Reply-To: <mptk13j5hci.fsf@arm.com>
[-- 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
{
next prev parent reply other threads:[~2020-03-20 12:24 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-03-15 13:53 Lewis Hyatt
2020-03-16 18:11 ` Richard Sandiford
2020-03-16 22:11 ` Lewis Hyatt
2020-03-16 22:21 ` Lewis Hyatt
2020-03-17 11:52 ` Richard Sandiford
2020-03-20 12:24 ` Lewis Hyatt [this message]
2020-03-20 12:54 ` Richard Sandiford
2020-03-20 16:06 ` Martin Sebor
2020-03-20 17:46 ` Richard Sandiford
2020-03-20 19:16 ` Martin Sebor
2020-03-20 21:02 ` Lewis Hyatt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200320122442.GA1620@ldh.local \
--to=lhyatt@gmail.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).