* [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 3/4] PR c++/62314: C++: add fixit hint to misspelled member names David Malcolm ` (2 more replies) 0 siblings, 3 replies; 7+ 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] 7+ messages in thread
* [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names 2015-12-18 21:47 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm @ 2015-12-18 21:47 ` David Malcolm 2015-12-18 21:47 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm 2015-12-18 21:47 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm 2 siblings, 0 replies; 7+ messages in thread From: David Malcolm @ 2015-12-18 21:47 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 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; a version of this was posted in the September patch kit FWIW, for the C frontend at least: https://gcc.gnu.org/ml/gcc-patches/2015-09/msg00747.html 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 * 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 39c1af2..ac0f4ca 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -2799,9 +2799,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] 7+ messages in thread
* [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" 2015-12-18 21:47 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm 2015-12-18 21:47 ` [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names David Malcolm @ 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 2 siblings, 0 replies; 7+ messages in thread From: David Malcolm @ 2015-12-18 21:47 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. Arguably not 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. Successfully bootstrapped®rtested on x86_64-pc-linux-gnu; adds 15 new PASS results to g++.sum. OK for trunk in stage 3? [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 2a688b2..c7b0aec 100644 --- a/gcc/cp/parser.c +++ b/gcc/cp/parser.c @@ -21287,17 +21287,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] 7+ messages in thread
* [PATCH 4/4] C: add fixit hint to misspelled field names 2015-12-18 21:47 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm 2015-12-18 21:47 ` [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names David Malcolm 2015-12-18 21:47 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm @ 2015-12-18 21:47 ` David Malcolm 2 siblings, 0 replies; 7+ 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] 7+ messages in thread
* [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:31 ` Trevor Saunders 0 siblings, 1 reply; 7+ 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] 7+ 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 @ 2016-04-28 14:31 ` Trevor Saunders 2016-04-29 21:29 ` Jason Merrill 0 siblings, 1 reply; 7+ 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] 7+ messages in thread
* Re: [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization 2016-04-28 14:31 ` Trevor Saunders @ 2016-04-29 21:29 ` Jason Merrill 0 siblings, 0 replies; 7+ 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] 7+ messages in thread
end of thread, other threads:[~2016-04-29 21:29 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-18 21:47 [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization David Malcolm 2015-12-18 21:47 ` [PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names David Malcolm 2015-12-18 21:47 ` [PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition" David Malcolm 2015-12-18 21:47 ` [PATCH 4/4] C: add fixit hint to misspelled field names David Malcolm 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:31 ` Trevor Saunders 2016-04-29 21:29 ` Jason Merrill
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).