On 06/14/2016 09:15 AM, David Malcolm wrote:
This patch introduces a new lookup_name_fuzzy function to the
C frontend and uses it to provides suggestions for various error
messages that may be due to misspellings, and also for the
warnings in implicit_decl_warning.
This latter part may be controversial. So far, we've only provided
spelling suggestions during error-handling, and I think there's
a strong case for spending the extra cycles to give a good error
message given that the compilation is going to fail. For a *warning*,
this tradeoff may not be so clear. In my experience, the
"implicit declaration of function" warning usually means that
either the compile or link is going to fail, so it may be that these
cases are usually effectively errors (hence the suggestions in this
patch).
Alternatively, the call to lookup_name_fuzzy could be somehow guarded
so that we don't do work upfront to generate the suggestion
if the warning is going to be discarded internally within diagnostics.c.
(if the user has supplied -Wno-implicit-function-declaration, they're
presumably working with code that uses implicit function decls, I
suppose).
Amongst other things, builtins get offered as suggestions (hence the
suggestion of "nanl" for "name" in one existing test case, which I
initially found surprising).
The patch includes PR c/70339: "singed" vs "signed" hints, looking
at reserved words that could start typenames.
The patch also considers preprocessor macros when considering
spelling candidates: if the user misspells a macro name, this is
typically seen by the frontend as an unrecognized identifier, so
we can offer suggestions like this:
spellcheck-identifiers.c: In function 'test_4':
spellcheck-identifiers.c:64:10: warning: implicit declaration of
function 'IDENTIFIER_PTR'; did you mean 'IDENTIFIER_POINTER'?
[-Wimplicit-function-declaration]
return IDENTIFIER_PTR (node);
^~~~~~~~~~~~~~
IDENTIFIER_POINTER
Doing so uses the "best_match" class added in a prior patch, merging
in results between C scopes and the preprocessor, using the
optimizations there to minimize the work done. Merging these
results required some minor surgery to class best_match.
In c-c++-common/attributes-1.c, the error message for:
void* my_calloc(unsigned, unsigned) __attribute__((alloc_size(1,bar))); /* { dg-warning
"outside range" } */
gains a suggestion of "carg", becoming:
attributes-1.c:4:65: error: 'bar' undeclared here (not in a function); did
you mean 'carg'?
attributes-1.c:4:1: warning: alloc_size parameter outside range [-Wattributes]
This is an unhelpful suggestion, given that alloc_size expects an integer
value identifying a parameter, which the builtin
double carg (double complex z);
is not. It's not clear to me what the best way to fix this is:
if I'm reading things right, c_parser_attributes parses expressions
using c_parser_expr_list without regard to which attribute it's
handling, so there's (currently) no way to "tune" the attribute parser
based on the attribute (and I don't know if that's a complexity we
want to take on).
Successfully bootstrapped®rtested in combination with the rest of
the kit on x86_64-pc-linux-gnu
Successful -fself-test of stage1 on powerpc-ibm-aix7.1.3.0 (in
combination with the rest of the kit).
OK for trunk?
gcc/c-family/ChangeLog:
PR c/70339
* c-common.h (enum lookup_name_fuzzy_kind): New enum.
(lookup_name_fuzzy): New prototype.
gcc/c/ChangeLog:
PR c/70339
* c-decl.c: Include spellcheck-tree.h and gcc-rich-location.h.
(implicit_decl_warning): When issuing warnings for implicit
declarations, attempt to provide a suggestion via
lookup_name_fuzzy.
(undeclared_variable): Likewise when issuing errors.
(lookup_name_in_scope): Likewise.
(struct edit_distance_traits<cpp_hashnode *>): New struct.
(best_macro_match): New typedef.
(find_closest_macro_cpp_cb): New function.
(lookup_name_fuzzy): New function.
* c-parser.c: Include gcc-rich-location.h.
(c_token_starts_typename): Split out case CPP_KEYWORD into...
(c_keyword_starts_typename): ...this new function.
(c_parser_declaration_or_fndef): When issuing errors about
missing "struct" etc, add a fixit. For other kinds of errors,
attempt to provide a suggestion via lookup_name_fuzzy.
(c_parser_parms_declarator): When looking ahead to detect typos in
type names, also reject CPP_KEYWORD.
(c_parser_parameter_declaration): When issuing errors about
unknown type names, attempt to provide a suggestion via
lookup_name_fuzzy.
* c-tree.h (c_keyword_starts_typename): New prototype.
gcc/ChangeLog:
PR c/70339
* diagnostic-core.h (pedwarn_at_rich_loc): New prototype.
* diagnostic.c (pedwarn_at_rich_loc): New function.
* spellcheck.h (best_match::best_match): Add a
"best_distance_so_far" optional parameter.
(best_match::set_best_so_far): New method.
(best_match::get_best_distance): New accessor.
(best_match::get_best_candidate_length): New accessor.
gcc/testsuite/ChangeLog:
PR c/70339
* c-c++-common/attributes-1.c: Update dg-prune-output to include
hint.
* gcc.dg/diagnostic-token-ranges.c (undeclared_identifier): Update
expected results due to builtin "nanl" now being suggested for
"name".
* gcc.dg/pr67580.c: Update expected messages.
* gcc.dg/spellcheck-identifiers.c: New testcase.
* gcc.dg/spellcheck-typenames.c: New testcase.
I think this is good and isn't terribly controversial in the modern era.
I suspect things would be different had you proposed something like
this 15+ years ago when implicit declarations were common.
I wouldn't be terribly surprised if we get some flak if/when folks try
to build old codebases with gcc-7. So based on feedback in the spring
as we approach the release, we may need to revisit whether or not to
guard this somehow to avoid noise on old codebases.
jeff
Jeff
