[PATCH 4/4] C: add fixit hint to misspelled field names

[PATCH 4/4] C: add fixit hint to misspelled field names

[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&regrtested 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

[PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization

[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&regrtested 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

[PATCH 3/4] PR c++/62314: C++: add fixit hint to misspelled member names

[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&regrtested 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

[PATCH 2/4] PR c++/62314: add fixit hint for "expected ';' after class definition"

[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&regrtested 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

Re: [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization

[Jason Merrill]

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

Re: [PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization

[Trevor Saunders]

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

[PATCH 1/4] PR c++/62314: add fixit hint for missing "template <> " in explicit specialization

[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