On Thu, 2017-07-27 at 13:51 -0600, Martin Sebor wrote:
> I think this is great. My overall question is: is the new BLT
> representation available in the middle-end?
Given that, err, I'm not freeing it anywhere yet (ahem), the answer is
yes. Although it lives in the "gcc" dir, the generated tree is
naturally very language-dependent; any kind of walking of it is going
to be making language-specific assumptions.
> If not, do you
> have plans to make it available? (I think it might be especially
> useful there, to either accurately highlight the source of
> a problem when it's far removed from the problem site, or to
> track the flow of data from its source to the sink.)
Note that at the moment in the parsers it captures lots of the stuff
outside of function bodies, which we weren't really capturing before
(params of fn decls, locations of individual tokens e.g. commas, etc),
since that's my primary motivation for the patch kit.
But currently IIRC it doesn't capture all of the stuff inside of
function bodies, as we mostly do a reasonable job there (the exception
is for various "leaf" expressions such as constants and uses of
decls/vars, which don't have a location_t; I'm working on a different
patch kit to try to address that issue).
The FE patches could be extended to capture more if desired (there may
be a memory tradeoff here).
A blt_node can be associated with a "tree" node, but it's not going to
help for things like a VAR_DECL leaf expression, since that can appear
multiple times in a so-called "tree" (actually a digraph). It could
potentially help if you know the parent tree node, but that's messy.
> > =====================
> > (b) Examples of usage
> > =====================
> >
> > Patches 6-10 in the kit update various diagnostics to use
> > the improved location information where available:
> >
> > * C and C++: highlighting the pertinent parameter of a function
> > decl when there's a mismatched type in a call
>
> Very nice!
>
> >
> > * C and C++: highlighting the return type in the function defn
> > when compaining about mismatch in the body (e.g. highlighting
> > the "void" when we see a "return EXPR;" within a void function).
> >
> > * C++: add a fix-it hint to -Wsuggest-override
> >
> > I have plenty of ideas for other uses of this infrastructure
> > (but which aren't implemented yet), e.g.:
> >
> > * C++: highlight the "const" token (or suggest a fix-it hint)
> > when you have a missing "const" on the *definition* of a member
> > function that was declared as "const" (I make this mistake
> > all the time).
> >
> > * C++: add a fix-it hint to -Wsuggest-final-methods
>
> To answer your call for other ideas below: There are other
> suggestions that GCC could offer to help improve code, including
>
> * to add the const qualifier to function pointer and reference
> arguments (including member functions)
When there's a mismatch? Consider e.g.
void test (const char *dst, char *src)
{
memcpy (dst, src, 1024);
}
warning: passing argument 1 of ‘memcpy’ discards ‘const’ qualifier from
pointer target type [-Wdiscarded-qualifiers]
memcpy (dst, src, 1024);
^~~
note: expected ‘void *’ but argument is of type ‘const char *’
void test (const char *dst, char *src)
^~~~~
------
(here I've added the underlining of the "const" and the fix-it hint
suggestion deletion of "const" on param aren't).
plus we could add:
warning: argument 2 could be marked as 'const'
void test (const char *dst, char *src)
^~~~~~~~~~
const char * src
> * to add restrict where appropriate (especially to const pointer
> and reference arguments)
When is it appropriate? Is this something that we can infer?
> * to delete or default the default copy ctor or copy assignment
> operator if/when it would benefit
Nice idea; is there a way we can tell when this is appropriate?
> * to help highlight how to optimize data layout (with a new
> hypothetical feature that offered suggestions to reorder
> struct or class members for space efficiency or data
> locality)
Good idea.
> >
> > * highlight bogus attributes
>
> This would be very nice. The diagnostics in this area are weak
> to put it mildly, and the highlighting is completely bogus. It
> would be great to also be able to highlight attribute operands.
>
> >
> > * add fix-it hints suggesting missing attributes
> >
> > ...etc, plus those "cousins of a compiler" ideas mentioned above.
> >
> > Any other ideas?
>
> This may be outside the scope of your work but when a declaration
> is found to conflict in some way with one seen earlier on in a file,
> it would be great to be able to point to the actual source of the
> conflict rather than to the immediately preceding declaration as
> a whole. As in:
>
> int __attribute__ ((noinline)) foo (int);
>
> int foo (int);
>
> int __attribute ((always_inline)) foo (int);
>
> x.c:5:35: warning: declaration of ‘int foo(int)’ with attribute
> ‘always_inline’ follows declaration with attribute ‘noinline’ [
> -Wattributes]
> int __attribute ((always_inline)) foo (int);
> ^~~
>
> Rather than printing a note like this:
>
> x.c:3:5: note: previous declaration of ‘int foo(int)’ was here
> int foo (int);
> ^~~
>
> print this:
>
> x.c:1:5: note: previous declaration of ‘int foo(int)’ was here
> int __attribute__ ((noinline)) foo (int);
> ^~~
>
> (preferably with the attribute underlined).
>
> I'm sure there are many others.
> Martin
Thanks
Dave
