Ping:
  https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01834.html


On Thu, 2016-04-28 at 10:28 -0400, David Malcolm wrote:
> Similar to the C++ case, but more involved as the location of the
> pertinent token isn't readily available.  The patch adds it as a
> param
> to build_component_ref.  All callers are updated to provide the info,
> apart from objc_build_component_ref; fixing the latter would lead to
> a cascade of other changes, so it's simplest to provide
> UNKNOWN_LOCATION
> there and have build_component_ref fall back gracefully for this case
> to the old behavior of showing a hint in the message, without a fixit
> replacement in the source view.
> 
> This does slightly change the location of the error; before we had:
> 
> test.c:11:13: error: 'union u' has no member named 'colour'; did you
> mean 'color'?
>    return ptr->colour;
>              ^~
> 
> with the patch we have:
> 
> test.c:11:15: error: 'union u' has no member named 'colour'; did you
> mean 'color'?
>    return ptr->colour;
>                ^~~~~~
>                color
> 
> I think the location change is an improvement.
> 
> Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?
> 
> gcc/c/ChangeLog:
>       * c-parser.c (c_parser_postfix_expression): In
> __builtin_offsetof
>       and structure element reference, capture the location of the
>       element name token and pass it to build_component_ref.
>       (c_parser_postfix_expression_after_primary): Likewise for
>       structure element dereference.
>       (c_parser_omp_variable_list): Likewise for
>       OMP_CLAUSE_{_CACHE, MAP, FROM, TO},
>       * c-tree.h (build_component_ref): Add location_t param.
>       * c-typeck.c (build_component_ref): Add location_t param
>       COMPONENT_LOC.  Use it, if available, when issuing hints about
>       mispelled member names to provide a fixit replacement hint.
> 
> gcc/objc/ChangeLog:
>       * objc-act.c (objc_build_component_ref): Update call
>       to build_component_ref for added param, passing
> UNKNOWN_LOCATION.
> 
> gcc/testsuite/ChangeLog:
>       * gcc.dg/spellcheck-fields-2.c: New test case.
> ---
>  gcc/c/c-parser.c                           | 34
> +++++++++++++++++++++---------
>  gcc/c/c-tree.h                             |  2 +-
>  gcc/c/c-typeck.c                           | 26 +++++++++++++++++++-
> ---
>  gcc/objc/objc-act.c                        |  3 ++-
>  gcc/testsuite/gcc.dg/spellcheck-fields-2.c | 19 +++++++++++++++++
>  5 files changed, 68 insertions(+), 16 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.dg/spellcheck-fields-2.c
> 
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index 36c44ab..19e6772 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -7707,8 +7707,9 @@ c_parser_postfix_expression (c_parser *parser)
>              accept sub structure and sub array references.  */
>           if (c_parser_next_token_is (parser, CPP_NAME))
>             {
> +             c_token *comp_tok = c_parser_peek_token (parser);
>               offsetof_ref = build_component_ref
> -               (loc, offsetof_ref, c_parser_peek_token (parser)
> ->value);
> +               (loc, offsetof_ref, comp_tok->value, comp_tok
> ->location);
>               c_parser_consume_token (parser);
>               while (c_parser_next_token_is (parser, CPP_DOT)
>                      || c_parser_next_token_is (parser,
> @@ -7734,9 +7735,10 @@ c_parser_postfix_expression (c_parser *parser)
>                           c_parser_error (parser, "expected
> identifier");
>                           break;
>                         }
> +                     c_token *comp_tok = c_parser_peek_token
> (parser);
>                       offsetof_ref = build_component_ref
> -                       (loc, offsetof_ref,
> -                        c_parser_peek_token (parser)->value);
> +                       (loc, offsetof_ref, comp_tok->value,
> +                        comp_tok->location);
>                       c_parser_consume_token (parser);
>                     }
>                   else
> @@ -8213,7 +8215,7 @@ c_parser_postfix_expression_after_primary
> (c_parser *parser,
>  {
>    struct c_expr orig_expr;
>    tree ident, idx;
> -  location_t sizeof_arg_loc[3];
> +  location_t sizeof_arg_loc[3], comp_loc;
>    tree sizeof_arg[3];
>    unsigned int literal_zero_mask;
>    unsigned int i;
> @@ -8327,7 +8329,11 @@ c_parser_postfix_expression_after_primary
> (c_parser *parser,
>         c_parser_consume_token (parser);
>         expr = default_function_array_conversion (expr_loc, expr);
>         if (c_parser_next_token_is (parser, CPP_NAME))
> -         ident = c_parser_peek_token (parser)->value;
> +         {
> +           c_token *comp_tok = c_parser_peek_token (parser);
> +           ident = comp_tok->value;
> +           comp_loc = comp_tok->location;
> +         }
>         else
>           {
>             c_parser_error (parser, "expected identifier");
> @@ -8339,7 +8345,8 @@ c_parser_postfix_expression_after_primary
> (c_parser *parser,
>         start = expr.get_start ();
>         finish = c_parser_peek_token (parser)->get_finish ();
>         c_parser_consume_token (parser);
> -       expr.value = build_component_ref (op_loc, expr.value,
> ident);
> +       expr.value = build_component_ref (op_loc, expr.value,
> ident,
> +                                         comp_loc);
>         set_c_expr_source_range (&expr, start, finish);
>         expr.original_code = ERROR_MARK;
>         if (TREE_CODE (expr.value) != COMPONENT_REF)
> @@ -8359,7 +8366,11 @@ c_parser_postfix_expression_after_primary
> (c_parser *parser,
>         c_parser_consume_token (parser);
>         expr = convert_lvalue_to_rvalue (expr_loc, expr, true,
> false);
>         if (c_parser_next_token_is (parser, CPP_NAME))
> -         ident = c_parser_peek_token (parser)->value;
> +         {
> +           c_token *comp_tok = c_parser_peek_token (parser);
> +           ident = comp_tok->value;
> +           comp_loc = comp_tok->location;
> +         }
>         else
>           {
>             c_parser_error (parser, "expected identifier");
> @@ -8375,7 +8386,7 @@ c_parser_postfix_expression_after_primary
> (c_parser *parser,
>                                           build_indirect_ref
> (op_loc,
>                                                               expr
> .value,
>                                                               RO_A
> RROW),
> -                                         ident);
> +                                         ident, comp_loc);
>         set_c_expr_source_range (&expr, start, finish);
>         expr.original_code = ERROR_MARK;
>         if (TREE_CODE (expr.value) != COMPONENT_REF)
> @@ -10621,9 +10632,12 @@ c_parser_omp_variable_list (c_parser
> *parser,
>                     t = error_mark_node;
>                     break;
>                   }
> -               tree ident = c_parser_peek_token (parser)->value;
> +
> +               c_token *comp_tok = c_parser_peek_token (parser);
> +               tree ident = comp_tok->value;
> +               location_t comp_loc = comp_tok->location;
>                 c_parser_consume_token (parser);
> -               t = build_component_ref (op_loc, t, ident);
> +               t = build_component_ref (op_loc, t, ident,
> comp_loc);
>               }
>             /* FALLTHROUGH  */
>           case OMP_CLAUSE_DEPEND:
> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
> index 4633182..3201a71 100644
> --- a/gcc/c/c-tree.h
> +++ b/gcc/c/c-tree.h
> @@ -604,7 +604,7 @@ extern struct c_expr convert_lvalue_to_rvalue
> (location_t, struct c_expr,
>                                              bool, bool);
>  extern void mark_exp_read (tree);
>  extern tree composite_type (tree, tree);
> -extern tree build_component_ref (location_t, tree, tree);
> +extern tree build_component_ref (location_t, tree, tree,
> location_t);
>  extern tree build_array_ref (location_t, tree, tree);
>  extern tree build_external_ref (location_t, tree, int, tree *);
>  extern void pop_maybe_used (bool);
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index 58c2139..8aece1a 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -2309,10 +2309,12 @@ should_suggest_deref_p (tree datum_type)
>  
>  /* Make an expression to refer to the COMPONENT field of structure
> or
>     union value DATUM.  COMPONENT is an IDENTIFIER_NODE.  LOC is the
> -   location of the COMPONENT_REF.  */
> +   location of the COMPONENT_REF.  COMPONENT_LOC is the location
> +   of COMPONENT.  */
>  
>  tree
> -build_component_ref (location_t loc, tree datum, tree component)
> +build_component_ref (location_t loc, tree datum, tree component,
> +                  location_t component_loc)
>  {
>    tree type = TREE_TYPE (datum);
>    enum tree_code code = TREE_CODE (type);
> @@ -2344,8 +2346,24 @@ build_component_ref (location_t loc, tree
> datum, tree component)
>       {
>         tree guessed_id = lookup_field_fuzzy (type, component);
>         if (guessed_id)
> -         error_at (loc, "%qT has no member named %qE; did you
> mean %qE?",
> -                   type, component, guessed_id);
> +         {
> +           /* Attempt to provide a fixit replacement hint, if
> +              we have a valid range for the component.  */
> +           location_t reported_loc
> +             = (component_loc != UNKNOWN_LOCATION) ?
> component_loc : loc;
> +           rich_location rich_loc (line_table, reported_loc);
> +           if (component_loc != UNKNOWN_LOCATION)
> +             {
> +               source_range component_range =
> +                 get_range_from_loc (line_table, component_loc);
> +               rich_loc.add_fixit_replace (component_range,
> +                                           IDENTIFIER_POINTER
> (guessed_id));
> +             }
> +           error_at_rich_loc
> +             (&rich_loc,
> +              "%qT has no member named %qE; did you mean %qE?",
> +              type, component, guessed_id);
> +         }
>         else
>           error_at (loc, "%qT has no member named %qE", type,
> component);
>         return error_mark_node;
> diff --git a/gcc/objc/objc-act.c b/gcc/objc/objc-act.c
> index 4856457..44f01d2 100644
> --- a/gcc/objc/objc-act.c
> +++ b/gcc/objc/objc-act.c
> @@ -2654,7 +2654,8 @@ objc_build_component_ref (tree datum, tree
> component)
>    return finish_class_member_access_expr (datum, component, false,
>                                            tf_warning_or_error);
>  #else
> -  return build_component_ref (input_location, datum, component);
> +  return build_component_ref (input_location, datum, component,
> +                           UNKNOWN_LOCATION);
>  #endif
>  }
>  
> diff --git a/gcc/testsuite/gcc.dg/spellcheck-fields-2.c
> b/gcc/testsuite/gcc.dg/spellcheck-fields-2.c
> new file mode 100644
> index 0000000..d6ebff1
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/spellcheck-fields-2.c
> @@ -0,0 +1,19 @@
> +/* { dg-options "-fdiagnostics-show-caret" } */
> +
> +union u
> +{
> +  int color;
> +  int shape;
> +};
> +
> +int test (union u *ptr)
> +{
> +  return ptr->colour; /* { dg-error "did you mean .color.?" } */
> +}
> +
> +/* Verify that we get an underline and a fixit hint.  */
> +/* { dg-begin-multiline-output "" }
> +   return ptr->colour;
> +               ^~~~~~
> +               color
> +   { dg-end-multiline-output "" } */

Reply via email to