Re: [PATCH 2/2] C++ FE: handle misspelled identifiers and typenames

[Manuel LÃpez-IbÃÃez <lopezibanez at gmail dot com>]

On 30/06/16 19:53, David Malcolm wrote:
> This is a port of the C frontend's r237714 [1] to the C++ frontend:
>    https://gcc.gnu.org/ml/gcc-patches/2016-06/msg01052.html
> offering spelling suggestions for misspelled identifiers, macro names,
> and some keywords (e.g. "singed" vs "signed" aka PR c/70339).

Cool!

> -      error_at (location, "%qE does not name a type", id);
> +      const char *suggestion = NULL;
> +      if (TREE_CODE (id) == IDENTIFIER_NODE)
> +        suggestion = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME);
> +      if (suggestion)
> +	{
> +	  gcc_rich_location richloc (location);
> +	  richloc.add_fixit_misspelled_id (location, suggestion);
> +	  error_at_rich_loc (&richloc,
> +			     "%qE does not name a type; did you mean %qs?",
> +			     id, suggestion);
> +	}
> +      else
> +	error_at (location, "%qE does not name a type", id);

It should be possible to encapsulate all this suggestion logic very deep in 
diagnostics.c and replace all the above with something similar to:

const char *suggestion = NULL;
if (TREE_CODE (id) == IDENTIFIER_NODE)
    suggestion = lookup_name_fuzzy (id, FUZZY_LOOKUP_TYPENAME);

error_with_suggestion(location, suggestion, "%qE does not name a type",
                      "%qE does not name a type; did you mean %qs?", id);

Perhaps with a bit more effort, even make the language-specific printers handle 
the task of looking for the suggestion, replacing the above with something like:

error_with_suggestion(location, FUZZY_LOOKUP_TYPENAME,
                      "%qE does not name a type",
                      "%qE does not name a type; did you mean %qs?", id);


> diff --git a/gcc/diagnostic-show-locus.c b/gcc/diagnostic-show-locus.c
> index 7aab658..49f7f11 100644
> --- a/gcc/diagnostic-show-locus.c
> +++ b/gcc/diagnostic-show-locus.c
> @@ -1280,9 +1280,18 @@ diagnostic_show_locus (diagnostic_context * context,
>   {
>     pp_newline (context->printer);
>
> -  if (!context->show_caret
> -      || diagnostic_location (diagnostic, 0) <= BUILTINS_LOCATION
> -      || diagnostic_location (diagnostic, 0) == context->last_location)
> +  /* Do nothing if source-printing has been disabled.  */
> +  if (!context->show_caret)
> +    return;
> +
> +  /* Don't attempt to print source for UNKNOWN_LOCATION and for builtins.  */
> +  if (diagnostic_location (diagnostic, 0) <= BUILTINS_LOCATION)
> +    return;
> +
> +  /* Don't print the same source location twice in a row, unless we have
> +     fix-it hints.  */
> +  if (diagnostic_location (diagnostic, 0) == context->last_location
> +      && diagnostic->richloc->get_num_fixit_hints () == 0)
>       return;
>
>     context->last_location = diagnostic_location (diagnostic, 0);

This seems independent of the suggestion stuff and could be committed separately.

> --- a/gcc/spellcheck-tree.c
> +++ b/gcc/spellcheck-tree.c
> @@ -22,6 +22,7 @@ along with GCC; see the file COPYING3.  If not see
>   #include "coretypes.h"
>   #include "tm.h"
>   #include "tree.h"
> +#include "cpplib.h"

You make a language-independent file depend on cpplib.h. Wouldn't it be better 
to move c-specific stuff to a new c-family/c-spellcheck.c ?

Cheers,

Manuel.