From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qv1-xf43.google.com (mail-qv1-xf43.google.com [IPv6:2607:f8b0:4864:20::f43]) by sourceware.org (Postfix) with ESMTPS id 3C3C8385F024 for ; Fri, 20 Mar 2020 12:24:46 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3C3C8385F024 Received: by mail-qv1-xf43.google.com with SMTP id v38so2796833qvf.6 for ; Fri, 20 Mar 2020 05:24:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ME37qnEF9rmX/mCgbfOFrKg/HEfNWi53q1VMi0otoyc=; b=q1b1/4VBpyV3py3EwmhQNWZZQ7VCfIEFJ4LJ8uB3yvtBW/vGCWCPThZnVXDpjIOs5N 7V1DgABt/HDP7sZgIM//5rRkeoDjzffQTZr4mwUDbul+15afpsWCMUS5gYxoYHMaz0+7 sZZGP2kojjs9G9gUYTPLMqyYHbKA8/FqD1ojNOQ/+O/YDXlA1LymX2s30GvtbKZx0ZcT r8QCiM/XHMufukmZeBlFt6Lxo5ilfUNSxJ4/pGivXMBu6i06kGFqCeXpkscFRLdSz0cT RTx0dtgJGzFn4TXDN3NI9fNrBUOJJ5Ctx72Hl7CJBfhwuRwLAR7aBhqe4W4ePMURNqpR et+Q== X-Gm-Message-State: ANhLgQ3XeMdZ0Ng2zyVaBHHiyoyzxALTOwj6FmwBa0pZUnFfjEqhiqHc fiDOGjGSqbf5Q4Aamp4G1PmDcdPx X-Google-Smtp-Source: ADFU+vvxnoYCfh3dFtrU9lj5Ztgt692gJczOh7tmIiDrisFUs7GRcD5oaSeGxc2p/uQ+tu2qVVUbMA== X-Received: by 2002:a0c:e886:: with SMTP id b6mr7744733qvo.31.1584707085159; Fri, 20 Mar 2020 05:24:45 -0700 (PDT) Received: from ldh.local (pool-173-54-19-91.nwrknj.fios.verizon.net. [173.54.19.91]) by smtp.gmail.com with ESMTPSA id q1sm699487qtn.69.2020.03.20.05.24.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Mar 2020 05:24:43 -0700 (PDT) Date: Fri, 20 Mar 2020 08:24:42 -0400 From: Lewis Hyatt To: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] driver: Improve the generated help text for alias options with arguments Message-ID: <20200320122442.GA1620@ldh.local> References: <20200315135300.GA32701@ldh-macbook.local> <20200316221151.GA98641@ldh.local> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="AqsLC8rIMeq19msA" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.1 (2019-06-15) X-Spam-Status: No, score=-27.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 20 Mar 2020 12:24:47 -0000 --AqsLC8rIMeq19msA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Mar 17, 2020 at 11:52:13AM +0000, Richard Sandiford wrote: > Lewis Hyatt writes: > > On Mon, Mar 16, 2020 at 06:11:08PM +0000, Richard Sandiford wrote: > >> Lewis Hyatt via Gcc-patches 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 --AqsLC8rIMeq19msA Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="ChangeLog.txt" gcc/ChangeLog: 2020-03-19 Lewis Hyatt * opts.c (print_filtered_help): Improve the help text for alias options. --AqsLC8rIMeq19msA Content-Type: text/x-patch; charset=us-ascii Content-Disposition: inline; filename="alias_opt_v2.patch" commit 7a74dd55098e2ec8c2b87dc172ac34f91eefc0c1 Author: Lewis Hyatt 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 { --AqsLC8rIMeq19msA--