* [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization @ 2016-04-28 14:04 David Malcolm 2016-04-28 14:03 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm ` (3 more replies) 0 siblings, 4 replies; 14+ messages in thread From: David Malcolm @ 2016-04-28 14:04 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm This is a resend of a patch kit I sent in stage 3; the original post was here: https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01933.html I've rebased the patches against yesterday's trunk and retested them. They add various fix-it hints to existing diagnostics (PR 62314 is a catch-all for adding fix-its). The first patch in the kit adds a fix-it insertion hint for missing "template <> " in explicit specializations, and improves the reported range of the type name by capturing the full range, rather than just one token within it. I note that clang (http://clang.llvm.org/diagnostics.html) suggests inserting template<> whereas our diagnostic talks about template <> hence I have the fixit suggest inserting that. Should we change our wording instead, and lose the space? Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? gcc/cp/ChangeLog: PR c++/62314 * parser.c (cp_parser_class_head): Capture the start location; use it to emit a fix-it insertion hint when complaining about missing "template <> " in explicit specializations. gcc/testsuite/ChangeLog: PR c++/62314 * g++.dg/pr62314.C: New test case. --- gcc/cp/parser.c | 18 ++++++++++++++++-- gcc/testsuite/g++.dg/pr62314.C | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr62314.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index 98a0cd4..ff16f73 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -21655,6 +21655,8 @@ cp_parser_class_head (cp_parser* parser, if (class_key == none_type) return error_mark_node; + location_t class_head_start_location = input_location; + /* Parse the attributes. */ attributes = cp_parser_attributes_opt (parser); @@ -21871,8 +21873,20 @@ cp_parser_class_head (cp_parser* parser, && parser->num_template_parameter_lists == 0 && template_id_p) { - error_at (type_start_token->location, - "an explicit specialization must be preceded by %<template <>%>"); + /* Build a location of this form: + struct typename <ARGS> + ^~~~~~~~~~~~~~~~~~~~~~ + with caret==start at the start token, and + finishing at the end of the type. */ + location_t reported_loc + = make_location (class_head_start_location, + class_head_start_location, + get_finish (type_start_token->location)); + rich_location richloc (line_table, reported_loc); + richloc.add_fixit_insert (class_head_start_location, "template <> "); + error_at_rich_loc + (&richloc, + "an explicit specialization must be preceded by %<template <>%>"); invalid_explicit_specialization_p = true; /* Take the same action that would have been taken by cp_parser_explicit_specialization. */ diff --git a/gcc/testsuite/g++.dg/pr62314.C b/gcc/testsuite/g++.dg/pr62314.C new file mode 100644 index 0000000..ebe75ec --- /dev/null +++ b/gcc/testsuite/g++.dg/pr62314.C @@ -0,0 +1,17 @@ +// { dg-options "-fdiagnostics-show-caret" } + +template <typename T> +struct iterator_traits {}; + +struct file_iterator; + +struct iterator_traits<file_iterator> { // { dg-error "explicit specialization must be preceded by .template" } +}; + +/* Verify that we emit a fixit hint for this case. */ + +/* { dg-begin-multiline-output "" } + struct iterator_traits<file_iterator> + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + template <> + { dg-end-multiline-output "" } */ -- 1.8.5.3 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" 2016-04-28 14:04 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm @ 2016-04-28 14:03 ` David Malcolm 2016-05-02 10:40 ` Bernd Schmidt 2016-04-28 14:03 ` [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names David Malcolm ` (2 subsequent siblings) 3 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2016-04-28 14:03 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm Looking over the discussion of missing semicolons in "Quality of Implementation and Attention to Detail" within http://clang.llvm.org/diagnostics.html and comparing with https://gcc.gnu.org/wiki/ClangDiagnosticsComparison I noticed that of the cases we do handle [1], there's room for improvement; we currently emit: test.c:2:11: error: expected ';' after struct definition struct a {} ^ whereas clang reportedly emits: test.c:2:12: error: expected ';' after struct struct a {} ^ ; (note the offset of the location, and the fix-it hint) The following patch gives us the latter, more readable output. Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? [1] I've also filed PR c++/68970 about a case given on the clang page that we still don't handle. gcc/cp/ChangeLog: PR c++/62314 * parser.c (cp_parser_class_specifier_1): When reporting missing semicolons, use a fixit-hint to suggest insertion of a semicolon immediately after the closing brace, offsetting the reported column accordingly. gcc/testsuite/ChangeLog: PR c++/62314 * gcc/testsuite/g++.dg/parse/error5.C: Update column number of missing semicolon error. * g++.dg/pr62314-2.C: New test case. --- gcc/cp/parser.c | 19 ++++++++++++++++--- gcc/testsuite/g++.dg/parse/error5.C | 2 +- gcc/testsuite/g++.dg/pr62314-2.C | 22 ++++++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr62314-2.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index ff16f73..e3133d0 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -21440,17 +21440,30 @@ cp_parser_class_specifier_1 (cp_parser* parser) closing brace. */ if (closing_brace && TYPE_P (type) && want_semicolon) { + /* Locate the closing brace. */ cp_token_position prev = cp_lexer_previous_token_position (parser->lexer); cp_token *prev_token = cp_lexer_token_at (parser->lexer, prev); location_t loc = prev_token->location; + /* We want to suggest insertion of a ';' immediately *after* the + closing brace, so, if we can, offset the location by 1 column. */ + location_t next_loc = loc; + if (!linemap_location_from_macro_expansion_p (line_table, loc)) + next_loc = linemap_position_for_loc_and_offset (line_table, loc, 1); + + rich_location richloc (line_table, next_loc); + richloc.add_fixit_insert (next_loc, ";"); + if (CLASSTYPE_DECLARED_CLASS (type)) - error_at (loc, "expected %<;%> after class definition"); + error_at_rich_loc (&richloc, + "expected %<;%> after class definition"); else if (TREE_CODE (type) == RECORD_TYPE) - error_at (loc, "expected %<;%> after struct definition"); + error_at_rich_loc (&richloc, + "expected %<;%> after struct definition"); else if (TREE_CODE (type) == UNION_TYPE) - error_at (loc, "expected %<;%> after union definition"); + error_at_rich_loc (&richloc, + "expected %<;%> after union definition"); else gcc_unreachable (); diff --git a/gcc/testsuite/g++.dg/parse/error5.C b/gcc/testsuite/g++.dg/parse/error5.C index eb1f9c7..d14a476 100644 --- a/gcc/testsuite/g++.dg/parse/error5.C +++ b/gcc/testsuite/g++.dg/parse/error5.C @@ -13,7 +13,7 @@ class Foo { int foo() return 0; } }; // need make cp_parser_error() report more accurate column numbers. // { dg-error "30:expected '\{' at end of input" "brace" { target *-*-* } 4 } -// { dg-error "33:expected ';' after class definition" "semicolon" {target *-*-* } 4 } +// { dg-error "34:expected ';' after class definition" "semicolon" {target *-*-* } 4 } // { dg-error "35:expected declaration before '\}' token" "declaration" {target *-*-* } 4 } diff --git a/gcc/testsuite/g++.dg/pr62314-2.C b/gcc/testsuite/g++.dg/pr62314-2.C new file mode 100644 index 0000000..deb0cb7 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr62314-2.C @@ -0,0 +1,22 @@ +// { dg-options "-fdiagnostics-show-caret" } + +template<class T> +class a {} // { dg-error "11: expected .;. after class definition" } +class temp {}; +a<temp> b; +struct b { +} // { dg-error "2: expected .;. after struct definition" } + +/* Verify that we emit fixit hints. */ + +/* { dg-begin-multiline-output "" } + class a {} + ^ + ; + { dg-end-multiline-output "" } */ + +/* { dg-begin-multiline-output "" } + } + ^ + ; + { dg-end-multiline-output "" } */ -- 1.8.5.3 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" 2016-04-28 14:03 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm @ 2016-05-02 10:40 ` Bernd Schmidt 2016-07-01 17:40 ` David Malcolm 0 siblings, 1 reply; 14+ messages in thread From: Bernd Schmidt @ 2016-05-02 10:40 UTC (permalink / raw) To: David Malcolm, gcc-patches On 04/28/2016 04:28 PM, David Malcolm wrote: > whereas clang reportedly emits: > > test.c:2:12: error: expected ';' after struct > struct a {} > ^ > ; > > (note the offset of the location, and the fix-it hint) > > The following patch gives us the latter, more readable output. Huh. Only the non-C++ parts remain to be reviewed, and I have no technical objections, but do people really want this? To me that looks like unnecessary visual clutter that eats up vertical space for no reason. I know what a semicolon looks like without the compiler telling me twice. Bernd ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" 2016-05-02 10:40 ` Bernd Schmidt @ 2016-07-01 17:40 ` David Malcolm 2016-07-04 10:15 ` Bernd Schmidt 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2016-07-01 17:40 UTC (permalink / raw) To: Bernd Schmidt, gcc-patches On Mon, 2016-05-02 at 12:40 +0200, Bernd Schmidt wrote: > On 04/28/2016 04:28 PM, David Malcolm wrote: > > whereas clang reportedly emits: > > > > test.c:2:12: error: expected ';' after struct > > struct a {} > > ^ > > ; > > > > (note the offset of the location, and the fix-it hint) > > > > The following patch gives us the latter, more readable output. > > Huh. Only the non-C++ parts remain to be reviewed, and I have no > technical objections, but do people really want this? To me that > looks > like unnecessary visual clutter that eats up vertical space for no > reason. I know what a semicolon looks like without the compiler > telling > me twice. My own opinion is that it's worth spending the extra line to get the semicolon under the caret, as (IMHO) it makes things slightly clearer. A better argument is that as of r237712 we now have -fdiagnostics -parseable-fixits. This allows for an IDE to offer to automatically apply a fix-it hint. Hence by providing a fix-it here, an IDE can potentially insert the semicolon itself: $ ./xgcc -B. ../../src/gcc/testsuite/g++.dg/pr62314-2.C \ -fdiagnostics-parseable-fixits ../../src/gcc/testsuite/g++.dg/pr62314-2.C:4:11: error: expected â;â after class definition class a {} // { dg-error "11: expected .;. after class definition" } ^ ; fix-it:"../../src/gcc/testsuite/g++.dg/pr62314-2.C":{4:11-4:11}:";" ../../src/gcc/testsuite/g++.dg/pr62314-2.C:8:2: error: expected â;â after struct definition } // { dg-error "2: expected .;. after struct definition" } ^ ; fix-it:"../../src/gcc/testsuite/g++.dg/pr62314-2.C":{8:2-8:2}:";" I believe that on building this within a sufficiently recent version of Xcode that Xcode can offer to insert the semicolon directly. I'm hoping someone implements this for Emacs. In that light, is the patch OK? Thanks Dave ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" 2016-07-01 17:40 ` David Malcolm @ 2016-07-04 10:15 ` Bernd Schmidt 0 siblings, 0 replies; 14+ messages in thread From: Bernd Schmidt @ 2016-07-04 10:15 UTC (permalink / raw) To: David Malcolm, gcc-patches On 07/01/2016 07:40 PM, David Malcolm wrote: > > A better argument is that as of r237712 we now have -fdiagnostics > -parseable-fixits. This allows for an IDE to offer to automatically > apply a fix-it hint. Hence by providing a fix-it here, an IDE can > potentially insert the semicolon itself: > In that light, is the patch OK? Yeah, that's an argument I can buy. Bernd ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names 2016-04-28 14:04 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm 2016-04-28 14:03 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm @ 2016-04-28 14:03 ` David Malcolm 2016-04-28 14:04 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm 2016-04-28 14:31 ` [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization Trevor Saunders 3 siblings, 0 replies; 14+ messages in thread From: David Malcolm @ 2016-04-28 14:03 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm When we emit a hint about a misspelled member name, it will slightly aid readability if we use a fixit-hint to show the proposed name in context within the source code (and in the future this might support some kind of auto-apply in an IDE). This patch adds such a hint to the C++ frontend, taking us from: test.cc:10:15: error: 'struct foo' has no member named 'colour'; did you mean 'color'? return ptr->colour; ^~~~~~ to: test.cc:10:15: error: 'struct foo' has no member named 'colour'; did you mean 'color'? return ptr->colour; ^~~~~~ color Successfully bootstrapped & regrtested on x86_64-pc-linux-gnu. OK for trunk? gcc/cp/ChangeLog: PR c++/62314 * typeck.c (finish_class_member_access_expr): When giving a hint about a possibly-misspelled member name, add a fix-it replacement hint. gcc/testsuite/ChangeLog: PR c++/62314 * g++.dg/spellcheck-fields-2.C: New test case. --- gcc/cp/typeck.c | 18 +++++++++++++++--- gcc/testsuite/g++.dg/spellcheck-fields-2.C | 19 +++++++++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/spellcheck-fields-2.C diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 7e12009..95c777d 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -2817,9 +2817,21 @@ finish_class_member_access_expr (cp_expr object, tree name, bool template_p, tree guessed_id = lookup_member_fuzzy (access_path, name, /*want_type=*/false); if (guessed_id) - error ("%q#T has no member named %qE; did you mean %qE?", - TREE_CODE (access_path) == TREE_BINFO - ? TREE_TYPE (access_path) : object_type, name, guessed_id); + { + location_t bogus_component_loc = input_location; + rich_location rich_loc (line_table, bogus_component_loc); + source_range bogus_component_range = + get_range_from_loc (line_table, bogus_component_loc); + rich_loc.add_fixit_replace + (bogus_component_range, + IDENTIFIER_POINTER (guessed_id)); + error_at_rich_loc + (&rich_loc, + "%q#T has no member named %qE; did you mean %qE?", + TREE_CODE (access_path) == TREE_BINFO + ? TREE_TYPE (access_path) : object_type, name, + guessed_id); + } else error ("%q#T has no member named %qE", TREE_CODE (access_path) == TREE_BINFO diff --git a/gcc/testsuite/g++.dg/spellcheck-fields-2.C b/gcc/testsuite/g++.dg/spellcheck-fields-2.C new file mode 100644 index 0000000..eb10b44 --- /dev/null +++ b/gcc/testsuite/g++.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 "" } */ -- 1.8.5.3 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] C: add fixit hint to misspelled field names 2016-04-28 14:04 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm 2016-04-28 14:03 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm 2016-04-28 14:03 ` [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names David Malcolm @ 2016-04-28 14:04 ` David Malcolm 2016-05-31 23:37 ` David Malcolm 2016-04-28 14:31 ` [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization Trevor Saunders 3 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2016-04-28 14:04 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm 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_ARROW), - 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 "" } */ -- 1.8.5.3 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] C: add fixit hint to misspelled field names 2016-04-28 14:04 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm @ 2016-05-31 23:37 ` David Malcolm 2016-06-06 15:17 ` Joseph Myers 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2016-05-31 23:37 UTC (permalink / raw) To: gcc-patches 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 "" } */ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 4/4] C: add fixit hint to misspelled field names 2016-05-31 23:37 ` David Malcolm @ 2016-06-06 15:17 ` Joseph Myers 2016-06-10 17:17 ` [PATCH] C: fixits for misspelled named initializers David Malcolm 0 siblings, 1 reply; 14+ messages in thread From: Joseph Myers @ 2016-06-06 15:17 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches 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 joseph@codesourcery.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH] C: fixits for misspelled named initializers 2016-06-06 15:17 ` Joseph Myers @ 2016-06-10 17:17 ` David Malcolm 2016-06-10 21:54 ` Joseph Myers 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2016-06-10 17:17 UTC (permalink / raw) To: gcc-patches; +Cc: Joseph Myers, David Malcolm On Mon, 2016-06-06 at 15:17 +0000, Joseph Myers wrote: > 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)? This patch adds fixits for named initializers, both old-style, and C99-style. I noticed that the existing error message: "unknown field %qE specified in initializer" didn't contain the type name, so I changed it to: "%qT has no member named %qE" to specify this (and for consistency with other messages). Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. OK for trunk? gcc/c/ChangeLog: * c-parser.c (c_parser_initelt): Provide location of name for new location_t param of set_init_label. * c-tree.h (set_init_label): Add location_t param. * c-typeck.c (set_init_index): Add "fieldname_loc" location_t param and use it when issuing error messages about unrecognized field names. Attempt to provide a fixit hint if appropriate, otherwise update the error message to provide the type name. gcc/testsuite/ChangeLog: * gcc.dg/c99-init-2.c (c): Update expected error message. * gcc.dg/init-bad-8.c (foo): Likewise. * gcc.dg/spellcheck-fields-3.c: New test case. --- gcc/c/c-parser.c | 2 + gcc/c/c-tree.h | 2 +- gcc/c/c-typeck.c | 21 +++++++++- gcc/testsuite/gcc.dg/c99-init-2.c | 2 +- gcc/testsuite/gcc.dg/init-bad-8.c | 2 +- gcc/testsuite/gcc.dg/spellcheck-fields-3.c | 66 ++++++++++++++++++++++++++++++ 6 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/spellcheck-fields-3.c diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c index 2fef1ac..5974ebb 100644 --- a/gcc/c/c-parser.c +++ b/gcc/c/c-parser.c @@ -4397,6 +4397,7 @@ c_parser_initelt (c_parser *parser, struct obstack * braced_init_obstack) /* Old-style structure member designator. */ set_init_label (c_parser_peek_token (parser)->location, c_parser_peek_token (parser)->value, + c_parser_peek_token (parser)->location, braced_init_obstack); /* Use the colon as the error location. */ pedwarn (c_parser_peek_2nd_token (parser)->location, OPT_Wpedantic, @@ -4426,6 +4427,7 @@ c_parser_initelt (c_parser *parser, struct obstack * braced_init_obstack) if (c_parser_next_token_is (parser, CPP_NAME)) { set_init_label (des_loc, c_parser_peek_token (parser)->value, + c_parser_peek_token (parser)->location, braced_init_obstack); c_parser_consume_token (parser); } diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h index b4374e3..8f10a13 100644 --- a/gcc/c/c-tree.h +++ b/gcc/c/c-tree.h @@ -639,7 +639,7 @@ extern void finish_implicit_inits (location_t, struct obstack *); extern void push_init_level (location_t, int, struct obstack *); extern struct c_expr pop_init_level (location_t, int, struct obstack *); extern void set_init_index (location_t, tree, tree, struct obstack *); -extern void set_init_label (location_t, tree, struct obstack *); +extern void set_init_label (location_t, tree, location_t, struct obstack *); extern void process_init_element (location_t, struct c_expr, bool, struct obstack *); extern tree build_compound_literal (location_t, tree, tree, bool); diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c index cd8e9e5..8dea62b 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -8200,7 +8200,7 @@ set_init_index (location_t loc, tree first, tree last, /* Within a struct initializer, specify the next field to be initialized. */ void -set_init_label (location_t loc, tree fieldname, +set_init_label (location_t loc, tree fieldname, location_t fieldname_loc, struct obstack *braced_init_obstack) { tree field; @@ -8219,7 +8219,24 @@ set_init_label (location_t loc, tree fieldname, field = lookup_field (constructor_type, fieldname); if (field == 0) - error_at (loc, "unknown field %qE specified in initializer", fieldname); + { + tree guessed_id = lookup_field_fuzzy (constructor_type, fieldname); + if (guessed_id) + { + rich_location rich_loc (line_table, fieldname_loc); + source_range component_range = + get_range_from_loc (line_table, fieldname_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?", + constructor_type, fieldname, guessed_id); + } + else + error_at (fieldname_loc, "%qT has no member named %qE", + constructor_type, fieldname); + } else do { diff --git a/gcc/testsuite/gcc.dg/c99-init-2.c b/gcc/testsuite/gcc.dg/c99-init-2.c index d3a331f..c07005b 100644 --- a/gcc/testsuite/gcc.dg/c99-init-2.c +++ b/gcc/testsuite/gcc.dg/c99-init-2.c @@ -9,7 +9,7 @@ typedef struct { } A; A a = { [2] = 1 }; /* { dg-error "(array index in non-array)|(near initialization)" } */ int b[] = { .B = 1 }; /* { dg-error "(field name not in record)|(near initialization)" } */ -A c[] = { [0].D = 1 }; /* { dg-error "unknown field" } */ +A c[] = { [0].D = 1 }; /* { dg-error "15: has no member named .D." } */ int d; int e = { d++ }; /* { dg-error "(is not constant)|(near initialization)" } */ A f[2] = { [0].C[0] = 1, [2] = { 2, { 1, 2 } } };/* { dg-error "(array index in initializer exceeds array bounds)|(near initialization)" } */ diff --git a/gcc/testsuite/gcc.dg/init-bad-8.c b/gcc/testsuite/gcc.dg/init-bad-8.c index b321323..f7b0f7f 100644 --- a/gcc/testsuite/gcc.dg/init-bad-8.c +++ b/gcc/testsuite/gcc.dg/init-bad-8.c @@ -6,5 +6,5 @@ struct S { int i, j, k; }; void foo (void) { - struct S s = { .i = 1, .j = 2, .l = 4}; /* { dg-error "34:unknown field .l. specified in initializer" } */ + struct S s = { .i = 1, .j = 2, .l = 4}; /* { dg-error "35: .struct S. has no member named .l." } */ } diff --git a/gcc/testsuite/gcc.dg/spellcheck-fields-3.c b/gcc/testsuite/gcc.dg/spellcheck-fields-3.c new file mode 100644 index 0000000..003a0b5 --- /dev/null +++ b/gcc/testsuite/gcc.dg/spellcheck-fields-3.c @@ -0,0 +1,66 @@ +/* { dg-do compile } */ +/* { dg-options "-fdiagnostics-show-caret -std=c99" } */ + +/* Tests of incorrect name initializers. + Verify that we get underlines and, where appropriate, fixit hints. */ + +struct foo +{ + int foo; + int bar; +}; + +union u +{ + int color; + int shape; +}; + +/* Old-style named initializers. */ + +struct foo old_style_f = { + foa: 1, /* { dg-error ".struct foo. has no member named .foa.; did you mean .foo." } */ +/* { dg-begin-multiline-output "" } + foa: 1, + ^~~ + foo + { dg-end-multiline-output "" } */ + + this_does_not_match: 3 /* { dg-error ".struct foo. has no member named .this_does_not_match." } */ + +/* { dg-begin-multiline-output "" } + this_does_not_match: 3 + ^~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ +}; + +union u old_style_u = { colour: 3 }; /* { dg-error ".union u. has no member named .colour.; did you mean .color.?" } */ +/* { dg-begin-multiline-output "" } + union u old_style_u = { colour: 3 }; + ^~~~~~ + color + { dg-end-multiline-output "" } */ + +/* C99-style named initializers. */ + +struct foo c99_style_f = { + .foa = 1, /* { dg-error ".struct foo. has no member named .foa.; did you mean .foo." } */ +/* { dg-begin-multiline-output "" } + .foa = 1, + ^~~ + foo + { dg-end-multiline-output "" } */ + + .this_does_not_match = 3 /* { dg-error ".struct foo. has no member named .this_does_not_match." } */ +/* { dg-begin-multiline-output "" } + .this_does_not_match = 3 + ^~~~~~~~~~~~~~~~~~~ + { dg-end-multiline-output "" } */ +}; + +union u c99_style_u = { .colour=3 }; /* { dg-error ".union u. has no member named .colour.; did you mean .color.?" } */ +/* { dg-begin-multiline-output "" } + union u c99_style_u = { .colour=3 }; + ^~~~~~ + color + { dg-end-multiline-output "" } */ -- 1.8.5.3 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH] C: fixits for misspelled named initializers 2016-06-10 17:17 ` [PATCH] C: fixits for misspelled named initializers David Malcolm @ 2016-06-10 21:54 ` Joseph Myers 0 siblings, 0 replies; 14+ messages in thread From: Joseph Myers @ 2016-06-10 21:54 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches On Fri, 10 Jun 2016, David Malcolm wrote: > On Mon, 2016-06-06 at 15:17 +0000, Joseph Myers wrote: > > 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)? > > This patch adds fixits for named initializers, both old-style, and > C99-style. > > I noticed that the existing error message: > "unknown field %qE specified in initializer" > didn't contain the type name, so I changed it to: > "%qT has no member named %qE" > to specify this (and for consistency with other messages). > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu. > > OK for trunk? OK. -- Joseph S. Myers joseph@codesourcery.com ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization 2016-04-28 14:04 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm ` (2 preceding siblings ...) 2016-04-28 14:04 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm @ 2016-04-28 14:31 ` Trevor Saunders 2016-04-29 21:29 ` Jason Merrill 3 siblings, 1 reply; 14+ messages in thread From: Trevor Saunders @ 2016-04-28 14:31 UTC (permalink / raw) To: David Malcolm; +Cc: gcc-patches On Thu, Apr 28, 2016 at 10:28:15AM -0400, David Malcolm wrote: > This is a resend of a patch kit I sent in stage 3; the original post > was here: > https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01933.html > > I've rebased the patches against yesterday's trunk and retested them. > > They add various fix-it hints to existing diagnostics (PR 62314 is a > catch-all for adding fix-its). > > The first patch in the kit adds a fix-it insertion hint for missing > "template <> " in explicit specializations, and improves the > reported range of the type name by capturing the full range, rather > than just one token within it. > > I note that clang (http://clang.llvm.org/diagnostics.html) suggests > inserting > template<> > whereas our diagnostic talks about > template <> > hence I have the fixit suggest inserting that. Should we change our > wording instead, and lose the space? Selfishly I'd prefer to lose the space on the grounds all the other projects I work on don't put one there and gcc is inconsistant about it. That said assuming there are projects that put a space there it seems unfortunate we need to pick one which will definitely be suboptimal for some people. Trev ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization 2016-04-28 14:31 ` [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization Trevor Saunders @ 2016-04-29 21:29 ` Jason Merrill 0 siblings, 0 replies; 14+ messages in thread From: Jason Merrill @ 2016-04-29 21:29 UTC (permalink / raw) To: Trevor Saunders, David Malcolm; +Cc: gcc-patches On 04/28/2016 10:30 AM, Trevor Saunders wrote: > On Thu, Apr 28, 2016 at 10:28:15AM -0400, David Malcolm wrote: >> This is a resend of a patch kit I sent in stage 3; the original post >> was here: >> https://gcc.gnu.org/ml/gcc-patches/2015-12/msg01933.html >> >> I've rebased the patches against yesterday's trunk and retested them. >> >> They add various fix-it hints to existing diagnostics (PR 62314 is a >> catch-all for adding fix-its). >> >> The first patch in the kit adds a fix-it insertion hint for missing >> "template <> " in explicit specializations, and improves the >> reported range of the type name by capturing the full range, rather >> than just one token within it. >> >> I note that clang (http://clang.llvm.org/diagnostics.html) suggests >> inserting >> template<> >> whereas our diagnostic talks about >> template <> >> hence I have the fixit suggest inserting that. Should we change our >> wording instead, and lose the space? > > Selfishly I'd prefer to lose the space on the grounds all the other > projects I work on don't put one there and gcc is inconsistant about it. The C++ standard is also inconsistent, sadly. The C++ patches are all OK. Jason ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization @ 2015-12-18 21:47 David Malcolm 2015-12-18 21:47 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm 0 siblings, 1 reply; 14+ messages in thread From: David Malcolm @ 2015-12-18 21:47 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm The following patch adds a fix-it insertion hint for missing "template <> " in explicit specializations. This is more of an enhancement than a bug fix (PR 62314 is more of a catch-all for adding fix-its), but it's low-risk and a user-visible improvement, and this specific example is called out as a benefit of clang over gcc in: http://clang.llvm.org/diagnostics.html I note that clang suggests inserting template<> whereas our diagnostic talks about template <> hence I have the fixit suggest inserting that. Should we change our wording instead, and lose the space? Successfully bootstrapped®rtested on x86_64-pc-linux-gnu; adds 9 new PASS results to g++.sum. OK for trunk in stage 3? gcc/cp/ChangeLog: PR c++/62314 * parser.c (cp_parser_class_head): Capture the start location; use it to emit a fix-it insertion hint when complaining about missing "template <> " in explicit specializations. gcc/testsuite/ChangeLog: PR c++/62314 * g++.dg/pr62314.C: New test case. --- gcc/cp/parser.c | 18 ++++++++++++++++-- gcc/testsuite/g++.dg/pr62314.C | 17 +++++++++++++++++ 2 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr62314.C diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c index a420cf1..2a688b2 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -21502,6 +21502,8 @@ cp_parser_class_head (cp_parser* parser, if (class_key == none_type) return error_mark_node; + location_t class_head_start_location = input_location; + /* Parse the attributes. */ attributes = cp_parser_attributes_opt (parser); @@ -21718,8 +21720,20 @@ cp_parser_class_head (cp_parser* parser, && parser->num_template_parameter_lists == 0 && template_id_p) { - error_at (type_start_token->location, - "an explicit specialization must be preceded by %<template <>%>"); + /* Build a location of this form: + struct typename <ARGS> + ^~~~~~~~~~~~~~~~~~~~~~ + with caret==start at the start token, and + finishing at the end of the type. */ + location_t reported_loc + = make_location (class_head_start_location, + class_head_start_location, + get_finish (type_start_token->location)); + rich_location richloc (line_table, reported_loc); + richloc.add_fixit_insert (class_head_start_location, "template <> "); + error_at_rich_loc + (&richloc, + "an explicit specialization must be preceded by %<template <>%>"); invalid_explicit_specialization_p = true; /* Take the same action that would have been taken by cp_parser_explicit_specialization. */ diff --git a/gcc/testsuite/g++.dg/pr62314.C b/gcc/testsuite/g++.dg/pr62314.C new file mode 100644 index 0000000..ebe75ec --- /dev/null +++ b/gcc/testsuite/g++.dg/pr62314.C @@ -0,0 +1,17 @@ +// { dg-options "-fdiagnostics-show-caret" } + +template <typename T> +struct iterator_traits {}; + +struct file_iterator; + +struct iterator_traits<file_iterator> { // { dg-error "explicit specialization must be preceded by .template" } +}; + +/* Verify that we emit a fixit hint for this case. */ + +/* { dg-begin-multiline-output "" } + struct iterator_traits<file_iterator> + ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + template <> + { dg-end-multiline-output "" } */ -- 1.8.5.3 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 4/4] C: add fixit hint to misspelled field names 2015-12-18 21:47 David Malcolm @ 2015-12-18 21:47 ` David Malcolm 0 siblings, 0 replies; 14+ messages in thread From: David Malcolm @ 2015-12-18 21:47 UTC (permalink / raw) To: gcc-patches; +Cc: David Malcolm 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®rtested 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 +8181,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) @@ -8195,7 +8202,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"); @@ -8211,7 +8222,7 @@ c_parser_postfix_expression_after_primary (c_parser *parser, build_indirect_ref (op_loc, expr.value, RO_ARROW), - ident); + ident, comp_loc); set_c_expr_source_range (&expr, start, finish); expr.original_code = ERROR_MARK; if (TREE_CODE (expr.value) != COMPONENT_REF) @@ -10457,9 +10468,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 00e72b1..4ca7a31 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 9d6c604..b4bda5b 100644 --- a/gcc/c/c-typeck.c +++ b/gcc/c/c-typeck.c @@ -2306,10 +2306,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); @@ -2341,8 +2343,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 e09093c..a2f1c45 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 "" } */ -- 1.8.5.3 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2016-07-04 10:15 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-04-28 14:04 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm 2016-04-28 14:03 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm 2016-05-02 10:40 ` Bernd Schmidt 2016-07-01 17:40 ` David Malcolm 2016-07-04 10:15 ` Bernd Schmidt 2016-04-28 14:03 ` [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names David Malcolm 2016-04-28 14:04 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm 2016-05-31 23:37 ` David Malcolm 2016-06-06 15:17 ` Joseph Myers 2016-06-10 17:17 ` [PATCH] C: fixits for misspelled named initializers David Malcolm 2016-06-10 21:54 ` Joseph Myers 2016-04-28 14:31 ` [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization Trevor Saunders 2016-04-29 21:29 ` Jason Merrill -- strict thread matches above, loose matches on Subject: below -- 2015-12-18 21:47 David Malcolm 2015-12-18 21:47 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).