On Mon, Nov 12, 2018 at 4:32 PM Martin Sebor <mse...@gmail.com> 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 <dmalc...@redhat.com> 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
