On Mon, 2017-10-02 at 16:22 -0400, Paul Koning wrote:
> > On Oct 2, 2017, at 4:05 PM, David Malcolm <dmalc...@redhat.com>
> > wrote:
> >
> > ...
> > the C FE currently emits (trunk):
> >
> > test.c: In function 'caller':
> > test.c:5:25: warning: passing argument 2 of 'callee' makes pointer
> > from
> > integer without a cast [-Wint-conversion]
> > return callee (first, second, third);
> > ^~~~~~
> > test.c:1:12: note: expected 'const char *' but argument is of type
> > 'int'
> > extern int callee (int one, const char *two, float three);
> > ^~~~~~
> >
> > whereas with this patch the note underlines the pertinent param of
> > the callee:
> >
> > test.c: In function 'caller':
> > test.c:5:25: warning: passing argument 2 of 'callee' makes pointer
> > from
> > integer without a cast [-Wint-conversion]
> > return callee (first, second, third);
> > ^~~~~~
> > test.c:1:41: note: expected 'const char *' but argument is of type
> > 'int'
> > extern int callee (int one, const char *two, float three);
> > ~~~~~~~~~~~~^~~
> >
> > making the problem more obvious to the user.
>
> I'm not sure why you'd point at the formal parameter name, which
> isn't significant and might not be present.
The patch copes with this case:
test.c:1:25: note: expected ‘const char *’ but argument is of type
‘int’
extern int callee (int, const char *, float);
^~~~~~~~~~~~
> The issue is with the type, so pointing at the type ("const char *")
> would seem more obvious.
Yes, there's an argument for highlighting just the type, but if there's
a name it's arguably significant information also - to the user, at
least, in that hopefully it contains a clue to the user as to the
meaning of the param.
Currently there isn't a natural place to store the location of the
*type* of a param decl. At my talk at Cauldron I proposed a new
internal representation that would allow for pointing at just the type
("BLT"), and I've posted patches that could implement that [1] but
Jason and Nathan were strongly opposed to my implementation approach,
and wanted me to use and/or extend the pre-existing representation, so
I went with this approach for C++, which I hope to mirror with this C
patch.
Hence given that DECL_ARGUMENTS exists, and that the PARAM_DECL has a
DECL_SOURCE_LOCATION, this patch (like the C++ one) takes the approach
of extending the PARAM_DECL's DECL_SOURCE_LOCATION to cover the range
described above (previously it was just that of one token). In theory
we could add some kind of additional location_t field to PARAM_DECL to
cover the location of the *type* part of the decl, but I suspect that
it would be vetoed on the grounds of memory cost, and given that the
benefits seems to me to be marginal, I went with the approach in the
patch.
The patch gets much more useful as the number of params increases and
visual comma-counting becomes a pain.
Hope the above makes sense
Dave
[1]: [PATCH 06/17] C: use BLT to highlight parameter of callee decl for
mismatching types
https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01459.html
and [PATCH 07/17] C++: use BLT to highlight parameter of callee decl
for mismatching types
https://gcc.gnu.org/ml/gcc-patches/2017-07/msg01458.html
(these highlight the name as well as the type, but could be modified to
just highlight the type)
