From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 38855 invoked by alias); 31 May 2016 20:42:17 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 38846 invoked by uid 89); 31 May 2016 20:42:17 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.3 required=5.0 tests=BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 spammy=106219, misspelled, colour, testc X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Tue, 31 May 2016 20:42:06 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id C772F6333D for ; Tue, 31 May 2016 20:42:05 +0000 (UTC) Received: from vpn-237-158.phx2.redhat.com (vpn-237-158.phx2.redhat.com [10.3.237.158]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u4VKg5bM012921 for ; Tue, 31 May 2016 16:42:05 -0400 Message-ID: <1464727324.11895.9.camel@redhat.com> Subject: Re: [PATCH 4/4] C: add fixit hint to misspelled field names From: David Malcolm To: gcc-patches@gcc.gnu.org Date: Tue, 31 May 2016 23:37:00 -0000 In-Reply-To: <1461853698-43954-4-git-send-email-dmalcolm@redhat.com> References: <1461853698-43954-1-git-send-email-dmalcolm@redhat.com> <1461853698-43954-4-git-send-email-dmalcolm@redhat.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 7bit X-IsSubscribed: yes X-SW-Source: 2016-05/txt/msg02461.txt.bz2 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 "" } */