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

[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

[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.

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

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

[Bernd Schmidt]

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

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

[David Malcolm]

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

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

[Bernd Schmidt]

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

[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

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

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

[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

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

[David Malcolm]

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 "" } */

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

[Joseph Myers]

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

[PATCH] C: fixits for misspelled named initializers

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

Re: [PATCH] C: fixits for misspelled named initializers

[Joseph Myers]

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&regrtested on x86_64-pc-linux-gnu.
> 
> OK for trunk?

OK.

-- 
Joseph S. Myers
joseph@codesourcery.com

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

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

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