This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [C PATCH] Ignore invisible bindings for misspelling hints (PR c/71858)


On Thu, Jul 14, 2016 at 01:22:27PM -0400, David Malcolm wrote:
> I wrote a patch for this, similar to yours, but with a slightly
> different approach for handling builtins.
> 
> I think it's reasonable to offer suggestions for misspellings of e.g.
> "snprintf" even if the pertinent header hasn't been included, since it
> will help guide a novice developer to the missing header.  For example,
> if the user has:
> 
> void
> test_7 (int i, int j)
> {
>   int buffer[100];
>   snprint (buffer, 100, "%i of %i", i, j);
> }
> 
> ...then they get:
> 
> test.c: In function ‘test_7’:
> test.c:5:3: warning: implicit declaration of function ‘snprint’; did
> you mean ‘snprintf’? [-Wimplicit-function-declaration]
>    snprint (buffer, 100, "%i of %i", i, j);
>    ^~~~~~~
>    snprintf
> 
> ...and on correcting it, they get the "you forgot <stdio.h>" warning:

I find it weird.  What is special on the builtins?  Lots of standard headers
contain lots of functions that aren't backed by builtins, some of them used
more often than the builtins; the reason for builtins is often just that gcc
wants to be able to optimize them somehow.

E.g. note fopen is not a builtin, so you still will not suggest it if
stdio.h is not included.

Not all the builtins out there are even enabled in all compilation modes
when the containing header is included, it can depend on the language
version, feature test macros, etc.  So, by considering invisible builtins
you could e.g. suggest C99 builtins in strict C89 compilation mode where the
header would not define it, etc.

In any case, this is your baby, so it is up to the FE maintainers and you to
get agreement on, I won't fight on this.

> What I find surprising is the suggestion of "carg"; I've seen this a
> few times as a suggestion.   I confess to having to look up carg; IMHO
> "carg" is much less likely to be a useful suggestion than, say,
> "printf".

For the short identifiers, the question is if the maximum Damerau-Levenshtein
distance (do you handle transpositions now?) shouldn't be lowered (say for at
most 4 chars identifiers only consider levenshtein distance 1 and not more,
e.g. for 3 character identifiers allowing distance of 2 or 3 means you pretty
much suggest any other 3 char identifier), regardless of if it is a builtin or not.

> @@ -3992,6 +3995,26 @@ lookup_name_fuzzy (tree name, enum lookup_name_fuzzy_kind kind)
>  	if (kind == FUZZY_LOOKUP_TYPENAME)
>  	  if (TREE_CODE (binding->decl) != TYPE_DECL)
>  	    continue;
> +	if (kind == FUZZY_LOOKUP_FUNCTION_NAME)
> +	  if (TREE_CODE (binding->decl) != FUNCTION_DECL)
> +	    continue;

Note this is handled in the second patch I've posted, and this one doesn't
handle function pointer variables/parameters.

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]