From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by sourceware.org (Postfix) with ESMTP id CAECD385F02A for ; Fri, 20 Mar 2020 17:46:45 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CAECD385F02A Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=arm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=richard.sandiford@arm.com Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 7F9AD1FB; Fri, 20 Mar 2020 10:46:45 -0700 (PDT) Received: from localhost (e121540-lin.manchester.arm.com [10.32.98.126]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id E2C0F3F305; Fri, 20 Mar 2020 10:46:44 -0700 (PDT) From: Richard Sandiford To: Martin Sebor Mail-Followup-To: Martin Sebor , Lewis Hyatt , gcc-patches@gcc.gnu.org, richard.sandiford@arm.com Cc: Lewis Hyatt , gcc-patches@gcc.gnu.org Subject: Re: [PATCH] driver: Improve the generated help text for alias options with arguments References: <20200315135300.GA32701@ldh-macbook.local> <20200316221151.GA98641@ldh.local> Date: Fri, 20 Mar 2020 17:46:43 +0000 In-Reply-To: (Martin Sebor's message of "Fri, 20 Mar 2020 10:06:38 -0600") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00, KAM_DMARC_STATUS, KAM_SHORT, 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 17:46:47 -0000 Martin Sebor writes: > On 3/17/20 5:52 AM, 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. > > 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