From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3369 invoked by alias); 13 Nov 2018 21:34:48 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 3330 invoked by uid 89); 13 Nov 2018 21:34:47 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 spammy=H*Ad:D*edu X-HELO: mail-ot1-f68.google.com Received: from mail-ot1-f68.google.com (HELO mail-ot1-f68.google.com) (209.85.210.68) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 13 Nov 2018 21:34:43 +0000 Received: by mail-ot1-f68.google.com with SMTP id u3so7695499ota.5 for ; Tue, 13 Nov 2018 13:34:42 -0800 (PST) MIME-Version: 1.0 References: <1541805824-3745-1-git-send-email-dmalcolm@redhat.com> <4d63c323-14d1-59b7-aead-55a24ee68a85@gmail.com> <1541970156.4619.19.camel@redhat.com> <01497f9d-ff8a-ca8c-2bc4-3aabd6b9361a@gmail.com> In-Reply-To: <01497f9d-ff8a-ca8c-2bc4-3aabd6b9361a@gmail.com> From: Jason Merrill Date: Tue, 13 Nov 2018 21:34:00 -0000 Message-ID: Subject: Re: [PATCH] C/C++: add fix-it hints for missing '&' and '*' (PR c++/87850) To: Martin Sebor Cc: David Malcolm , Eric Gallager , gcc-patches List Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2018-11/txt/msg01152.txt.bz2 On Mon, Nov 12, 2018 at 4:32 PM Martin Sebor wrote: > On 11/11/2018 02:02 PM, David Malcolm wrote: > > On Sun, 2018-11-11 at 11:01 -0700, Martin Sebor wrote: > >> On 11/10/2018 12:01 AM, Eric Gallager wrote: > >>> On 11/9/18, David Malcolm wrote: > >>>> This patch adds a fix-it hint to various pointer-vs-non-pointer > >>>> diagnostics, suggesting the addition of a leading '&' or '*'. > >>>> > >>>> For example, note the ampersand fix-it hint in the following: > >>>> > >>>> demo.c:5:22: error: invalid conversion from 'pthread_key_t' {aka > >>>> 'unsigned > >>>> int'} > >>>> to 'pthread_key_t*' {aka 'unsigned int*'} [-fpermissive] > >>>> 5 | pthread_key_create(key, NULL); > >>>> | ^~~ > >>>> | | > >>>> | pthread_key_t {aka unsigned int} > >>>> | & > >>> > >>> Having both the type and the fixit underneath the caret looks kind > >>> of confusing > >> > >> I agree it's rather subtle. Keeping the diagnostics separate from > >> the suggested fix should avoid the confusion. > > > > FWIW, the fix-it hint is in a different color (assuming that gcc is > > invoked in an environment that prints that...) > > I figured it would be, but I'm still not sure it's good design > to be relying on color alone to distinguish between the problem > and the suggested fix. Especially when they are so close to one > another and the fix is just a single character with no obvious > relationship to the rest of the text on the screen. In other > warnings there's at least the "did you forget the '@'?" part > to give a clue, even though even there the connection between > the "did you forget" and the & several lines down wouldn't > necessarily be immediately apparent. Agreed, something along those lines would help to understand why the compiler is throwing a random & into the diagnostic. Jason