From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x741.google.com (mail-qk1-x741.google.com [IPv6:2607:f8b0:4864:20::741]) by sourceware.org (Postfix) with ESMTPS id 3FB5F385F02A for ; Fri, 20 Mar 2020 21:02:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 3FB5F385F02A Received: by mail-qk1-x741.google.com with SMTP id f3so8589474qkh.1 for ; Fri, 20 Mar 2020 14:02:15 -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=75rdlRjpWEz7A5A1Il+2f55FF7h0M9KgIJK0pHA031I=; b=g9bR9c2j5lXqpEePcyRzzoMJl3deCh+jJE8uXwCA5bYBOG+1eNtRGFxBfw5LM017y2 vb9NvgY15KXyjMvC1xxZS8tEPBjFT4ZADr304qynfTqIJLRsi1l08zFWgbVtgusEnxgc rRAsmnXu9zdxGqZzALLJpoMW7kal6yakKiJc6wjd8DdrSSZcejSNjOiO5C08GnxyHZaU vcxFagLqtTjIl3oxtJfoEMEofLXaz6vr2Rsv/MnWdVDBx3yuba7PA0KQctMScZBfP6LK JJdTazXD72kixOUv9V1N0cUaLjGedKkw0UvUmwGrEZpRpmwmgHTh90snylMp2W9mOF/A slhQ== X-Gm-Message-State: ANhLgQ2MNoYBrNpYqmUMyJX+pakYDSYkbR1DclbLniKxKIXREI+xHKE0 t5rLya0aI1GVgmZTUCW3Lu/VA4rP X-Google-Smtp-Source: ADFU+vuc6K1l59imYkLjOFBFP9uyVodJTn4kcfhTl/vGFA8M5i7ZJHvBc+xcBAcq08HPt7g+kxR37A== X-Received: by 2002:a37:9a83:: with SMTP id c125mr9974819qke.25.1584738134380; Fri, 20 Mar 2020 14:02:14 -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 h27sm5822843qte.37.2020.03.20.14.02.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 20 Mar 2020 14:02:13 -0700 (PDT) Date: Fri, 20 Mar 2020 17:02:12 -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: <20200320210212.GA3373@ldh.local> References: <20200315135300.GA32701@ldh-macbook.local> <20200316221151.GA98641@ldh.local> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.12.1 (2019-06-15) X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, KAM_SHORT, 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 21:02:17 -0000 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 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. > > 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