On Thu, Jul 13, 2017 at 11:42:15AM -0600, Martin Sebor wrote:
> On 07/13/2017 08:18 AM, Marek Polacek wrote:
> > This patch improves diagnostic in the C FE by printing the types when
> > reporting
> > a problem with a conversion. E.g., instead of
> >
> > warning: assignment from incompatible pointer type
> >
> > you'll now get
> >
> > warning: assignment to 'int *' from incompatible pointer type 'char *'
> >
> > or instead of
> >
> > warning: initialization makes integer from pointer without a cast
> >
> > this
> >
> > warning: initialization of 'int *' from 'int' makes pointer from integer
> > without a cast
> >
> > I've been wanting this for a long time and here it is. Two snags: I had to
> > make pedwarn_init to take '...' for which I had to introduce
> > emit_diagnostic_valist; you can't pass varargs from one vararg function to
> > another vararg function (and a macro with __VA_ARGS__ didn't work here).
> > Also,
> > PEDWARN_FOR_ASSIGNMENT didn't work with the addition of printing TYPE and
> > RHSTYPE so I just decided to unroll the macro instead of making it even more
> > ugly. This patch is long but it's mainly because of the testsuite fallout.
> >
> > If you have better ideas about the wording, let me know.
>
> It looks pretty good as is. My only wording suggestion is to
> consider simply mentioning conversion in the text of the warnings:
>
> warning: conversion to T* from an incompatible type U*
>
> I'm not sure that being explicit about the context where the bad
> conversion takes place (initialization vs assignment vs returning
> a value) is terribly helpful. That would not only simplify the
> code and make all the messages consistent, but it would also make
> it possible to get rid of the note when passing arguments.
Yeah, I agree, actually. We print the expressions in question (although,
we could probably do even better), and I don't see why it would be
necessary to mention whether it's an initialization or an assignment.
I think I'll just drop this patch and do something along the lines you
suggest.
David, do you agree with this? (Joseph's on PTO but I'd of course like
to hear his opinion, too.)
One more thing: for
int *q = p;
int i = q;
we should be able to provide a fix-it hint, something like 'did you mean
to dereference q?'. With the current PEDWARN_FOR_ASSIGNMENT macro it
would be awkward to implement that.
> > There are still more warnings to improve but I think better to do this
> > incrementally rather than a single humongous patch.
>
> That makes sense. I was going to mention that it would be nice
> to also improve:
>
> warning: comparison of distinct pointer types lacks a cast
>
> If you take the conversion suggestion I think this warning would
> need to be phrased in terms of "conversion between T* and U*"
> rather than "conversion from T* to U*". (A similar change could
> be made to the error message printed when incompatible pointers
> are subtracted from one another.)
Sure.
Marek
