Re: [PATCH 4/4] C: add fixit hint to misspelled field names
On Tue, 31 May 2016, David Malcolm wrote: > Ping: > https://gcc.gnu.org/ml/gcc-patches/2016-04/msg01834.html OK. What about field names in designated initializers (both C99-style and old-style)? -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 4/4] C: add fixit hint to misspelled field names
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 @@
[PATCH 4/4] C: add fixit hint to misspelled field names
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,
[PATCH 4/4] C: add fixit hint to misspelled field names
Similar to the C++ case in the previous patch, 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.
I don't know if I can argue that this is a bug fix, but it's a
user-visible improvement, low risk, and a (distantly-related) version
of it was posted in September:
https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00747.html
Successfully bootstrapped on x86_64-pc-linux-gnu;
adds new 3 PASS results to gcc.sum.
OK for trunk in stage 3?
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 124c30b..774354a 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -7543,8 +7543,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,
@@ -7570,9 +7571,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
@@ -8046,7 +8048,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;
@@ -8163,7 +8165,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");
@@ -8175,7
