diff mbox

[RFC/PATCH] Fix-it hints

Message ID CAESRpQCLTvV5K=D7fBpmvZJGE+iavpRBGpcsnVxHh=EhLFjsPQ@mail.gmail.com
State New
Headers show

Commit Message

Manuel López-Ibáñez Aug. 31, 2014, 9:57 p.m. UTC 
This patch implements fix-it hints. See https://gcc.gnu.org/PR62314

When the caret line is active (which is the default), this adds an
additional source-line indicating how to fix the code:

gcc/testsuite/g++.dg/template/crash83.C:5:21: error: an explicit
specialization must be preceded by 'template <>'
 template<typename = class A<0>: > struct B {}; // { dg-error
"explicit specialization|expected" }
                     ^
                     template<>

When the caret line is disabled with -fno-diagnostics-show-caret, the
fix-it hint is printed as:

gcc/testsuite/g++.dg/template/crash83.C:5:21: error: an explicit
specialization must be preceded by 'template <>'
gcc/testsuite/g++.dg/template/crash83.C:5:21: fixit: template<>

The latter form may allow an IDE (such as emacs) to automatically apply the fix.

Currently, fix-it hints are limited to insertions at one single
location, whereas Clang allows insertions, deletions, and replacements
at arbitrary location ranges.

Opinions? Is the proposed interface/implementation acceptable?

Any other diagnostics that could use a fix-it hint? In principle, we
should only give them when we are sure that the proposed fix will fix
the error or silence a warning.  For example, the C++ parser often
says 'x' expected before 'y' but adding x before y rarely fixes
anything.

gcc/ChangeLog:

2014-08-31  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR c++/62314
    * diagnostic.def (DK_FIXIT): New.
    * diagnostic.c (adjust_column): New.
    (adjust_line): Factor out adjust_column.
    (get_source_line_and_column): New.
    (diagnostic_show_locus): Call get_source_line_and_column.
    (diagnostic_action_after_output): Ignore DK_FIXIT.
    (fixit_hint): New.
    * diagnostic-core.h (fixit_hint): Declare.

gcc/cp/ChangeLog:

2014-08-31  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR c++/62314
    * parser.c (cp_parser_diagnose_invalid_type_name): Call fixit_hint.
    (cp_parser_expression_statement): Likewise.
    (cp_parser_class_head): Likewise. Update location.


gcc/testsuite/ChangeLog:

2014-08-31  Manuel López-Ibáñez  <manu@gcc.gnu.org>

    PR c++/62314
    * g++.old-deja/g++.oliva/typename1.C: Handle fixit hint.
    * g++.old-deja/g++.oliva/typename2.C: Likewise.
    * g++.old-deja/g++.other/typename1.C: Likewise.
    * g++.old-deja/g++.pt/typename6.C: Likewise.
    * g++.old-deja/g++.pt/typename3.C: Likewise.
    * g++.old-deja/g++.pt/typename4.C: Likewise.
    * g++.dg/parse/error36.C: Likewise.
    * g++.dg/parse/typedef2.C: Likewise.
    * g++.dg/template/error6.C: Likewise.
    * g++.dg/template/dependent-name5.C: Likewise.
    * g++.dg/template/crash83.C: Likewise.
    * g++.dg/template/typename3.C: Likewise.

Comments

Dodji Seketeli Sept. 25, 2014, 9:34 a.m. UTC | #1 
Hello Manuel,

Sorry for taking so long to reply to this.

FWIW, I like the direction of this.  I find fix-it hints cool in
general.  So thank you for working on this.

Manuel López-Ibáñez <lopezibanez@gmail.com> a écrit:

> This patch implements fix-it hints. See https://gcc.gnu.org/PR62314
>
> When the caret line is active (which is the default), this adds an
> additional source-line indicating how to fix the code:
>
> gcc/testsuite/g++.dg/template/crash83.C:5:21: error: an explicit
> specialization must be preceded by 'template <>'
>  template<typename = class A<0>: > struct B {}; // { dg-error
> "explicit specialization|expected" }
>                      ^
>                      template<>

It looks like your mail user agent wrapped a line above, making it hard
to read.  I suspect it should have been:

  template<typename = class A<0> > struct B {}; // { dg-error "explicit specialization|expected" }
                      ^
                      template<>

> When the caret line is disabled with -fno-diagnostics-show-caret, the
> fix-it hint is printed as:
>
> gcc/testsuite/g++.dg/template/crash83.C:5:21: error: an explicit
> specialization must be preceded by 'template <>'
> gcc/testsuite/g++.dg/template/crash83.C:5:21: fixit: template<>
>
> The latter form may allow an IDE (such as emacs) to automatically
> apply the fix.

Nice.  Is the "fixit:" prefix used by other compilers too?  Or are there
variations from compiler to compiler?

> Currently, fix-it hints are limited to insertions at one single
> location, whereas Clang allows insertions, deletions, and replacements
> at arbitrary location ranges.

Do you have example of each of these kinds of fix-it hints? (deletions,
replacement at location ranges).  I think it'd be nice to have an idea
of what needs to be done, even if we are not doing it "in extenso" right
now.

> Opinions? Is the proposed interface/implementation acceptable?

Please read my comments below.

> Any other diagnostics that could use a fix-it hint? In principle, we
> should only give them when we are sure that the proposed fix will fix
> the error or silence a warning.  For example, the C++ parser often
> says 'x' expected before 'y' but adding x before y rarely fixes
> anything.

I am thinking that maybe the diagnostic about the missing ";" after a
struct/class declaration might be a candidate for this fix-it hint
feature.

It's emitted by cp_parser_class_specifier_1() at:

	if (CLASSTYPE_DECLARED_CLASS (type))
	  error_at (loc, "expected %<;%> after class definition");
	else if (TREE_CODE (type) == RECORD_TYPE)
	  error_at (loc, "expected %<;%> after struct definition");
	else if (TREE_CODE (type) == UNION_TYPE)
	  error_at (loc, "expected %<;%> after union definition");
	else
	  gcc_unreachable ();


[...]

> +
> +static int
> +adjust_column (int line_width, int max_width, int column)

Missing comments for this function.

[...]

> +static const char *
> +get_source_line_and_column (location_t loc, int *line_width, int *column)
> +{

Likewise.

[...]


>  /* Print the physical source line corresponding to the location of
>     this diagnostic, and a caret indicating the precise column.  */
>  void
>  diagnostic_show_locus (diagnostic_context * context,
>  		       const diagnostic_info *diagnostic)
>  {

[...]

>    context->last_location = diagnostic->location;
> -  s = expand_location_to_spelling_point (diagnostic->location);
> -  line = location_get_source_line (s, &line_width);
> -  if (line == NULL || s.column > line_width)
> +  line = get_source_line_and_column (diagnostic->location,
> +				     &line_width, &column);
> +  if (line == NULL)
>      return;
>  
>    max_width = context->caret_max_width;
> -  line = adjust_line (line, line_width, max_width, &(s.column));
> +  line = adjust_line (line, line_width, max_width, &column);

Apparently, each time we call get_source_line_and_column, we also call
adjust_line on it.  So maybe we want to have a
get_adjusted_source_line_and_column (or something like that) that does
it all?

[...]

> @@ -325,13 +345,13 @@ diagnostic_show_locus (diagnostic_contex
>    pp_newline (context->printer);
>    caret_cs = colorize_start (pp_show_color (context->printer), "caret");
>    caret_ce = colorize_stop (pp_show_color (context->printer));
>  
>    /* pp_printf does not implement %*c.  */
> -  size_t len = s.column + 3 + strlen (caret_cs) + strlen (caret_ce);
> +  size_t len = column + 3 + strlen (caret_cs) + strlen (caret_ce);
>    buffer = XALLOCAVEC (char, len);
> -  snprintf (buffer, len, "%s %*c%s", caret_cs, s.column, context->caret_char,
> +  snprintf (buffer, len, "%s %*c%s", caret_cs, column, context->caret_char,
>  	    caret_ce);

Maybe you should factorize out the printing of a colored line starting
at given a column, rather than copy-pasting this in fixit_hint() later?

[...]

>    diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_NOTE);
>    report_diagnostic (&diagnostic);
>    va_end (ap);
>  }
>  
> +/* A fix-it hint at LOCATION.  Use this recommended textual
> +   replacements after another diagnostic message.  */
> +void
> +fixit_hint (location_t location, const char *msg, ...)

I would call this "emit_fixit_hint" to make its intend clearer.  Also,
I'd be more extensive in the comment of this function.  Maybe give the
introductory example you gave for this patch?

Also, just curious, do you have an idea about how you would extend this
to support deletion and replacement fix-it hints?  For instance, from a
user (of the diagnostics primitives) code standpoint, would these be
done through additional functions like emit_deletion_fixit() and
emit_replacement_fixit() with possible different parameters?  Or would
you rather add additional parameters to emit_fixing_hint() to support
that?

> +{
> +  diagnostic_info diagnostic;
> +  va_list ap;
> +
> +  va_start (ap, msg);
> +  diagnostic_set_info (&diagnostic, msg, &ap, location, DK_FIXIT);
> +  if (!global_dc->show_caret)
> +    {
> +      report_diagnostic (&diagnostic);
> +    }

Useless curly brackets here.

Thank you for looking into this.

Cheers,
Manuel López-Ibáñez Sept. 27, 2014, 4:19 p.m. UTC | #2 
On 25 September 2014 11:34, Dodji Seketeli <dodji@redhat.com> wrote:
>> When the caret line is disabled with -fno-diagnostics-show-caret, the
>> fix-it hint is printed as:
>>
>> gcc/testsuite/g++.dg/template/crash83.C:5:21: error: an explicit
>> specialization must be preceded by 'template <>'
>> gcc/testsuite/g++.dg/template/crash83.C:5:21: fixit: template<>
>>
>> The latter form may allow an IDE (such as emacs) to automatically
>> apply the fix.
>
> Nice.  Is the "fixit:" prefix used by other compilers too?  Or are there
> variations from compiler to compiler?

Clang seems to do:

fix-it:"t.cpp":{7:25-7:29}:"Gamma"

http://clang.llvm.org/docs/UsersManual.html#cmdoption-fdiagnostics-parseable-fixits

where the location is a half-open range (insertions are marked as
{7:25-7:25}). However, this is not the standard GNU format for ranges,
and last time I tried to update that (to support multiple ranges) RMS
was not in favour of adding the Clang flavour. Quoting the filename
also does not seem to be what GNU mandates. No idea why they use {}
around the location range and not sure if they support just 7:25,
which seems nicer.

Also, I don't know who are the consumers of Clang's format that GCC
will be interested in supporting.

No idea about other compilers that support fix-it hints.

Comments?


>
>> Currently, fix-it hints are limited to insertions at one single
>> location, whereas Clang allows insertions, deletions, and replacements
>> at arbitrary location ranges.
>
> Do you have example of each of these kinds of fix-it hints? (deletions,
> replacement at location ranges).  I think it'd be nice to have an idea
> of what needs to be done, even if we are not doing it "in extenso" right
> now.

I think in the case of deletions, they just underline the text to be deleted:

typename3.C:3:7: warning: duplicate 'const' declaration specifier
[-Wduplicate-decl-specifier]
const const long long long int x;
      ^~~~~~
fix-it:"typename3.C":{3:7-3:13}:""

In the case of replacements, there does not seem to be any visible
difference with an insertion:

typename3.C:4:33: note: change this ',' to a ';' to call 'f'
const const long long long int x,
                                ^
                                ;
fix-it:"typename3.C":{4:33-4:34}:";"

Cheers,

Manuel.
diff mbox

Patch

Index: gcc/diagnostic.def
===================================================================
--- gcc/diagnostic.def	(revision 214756)
+++ gcc/diagnostic.def	(working copy)
@@ -35,10 +35,11 @@  DEFINE_DIAGNOSTIC_KIND (DK_ICE, "interna
 DEFINE_DIAGNOSTIC_KIND (DK_ERROR, "error: ", "error")
 DEFINE_DIAGNOSTIC_KIND (DK_SORRY, "sorry, unimplemented: ", "error")
 DEFINE_DIAGNOSTIC_KIND (DK_WARNING, "warning: ", "warning")
 DEFINE_DIAGNOSTIC_KIND (DK_ANACHRONISM, "anachronism: ", "warning")
 DEFINE_DIAGNOSTIC_KIND (DK_NOTE, "note: ", "note")
+DEFINE_DIAGNOSTIC_KIND (DK_FIXIT, "fixit: ", "note")
 DEFINE_DIAGNOSTIC_KIND (DK_DEBUG, "debug: ", "note")
 /* These two would be re-classified as DK_WARNING or DK_ERROR, so the
 prefix does not matter.  */
 DEFINE_DIAGNOSTIC_KIND (DK_PEDWARN, "pedwarn: ", NULL)
 DEFINE_DIAGNOSTIC_KIND (DK_PERMERROR, "permerror: ", NULL)
Index: gcc/diagnostic.c
===================================================================
--- gcc/diagnostic.c	(revision 214756)
+++ gcc/diagnostic.c	(working copy)
@@ -254,61 +254,81 @@  diagnostic_build_prefix (diagnostic_cont
 			     s.column, locus_ce, text_cs, text, text_ce)
      : build_message_string ("%s%s:%d:%s %s%s%s", locus_cs, s.file, s.line, locus_ce,
 			     text_cs, text, text_ce));
 }
 
+
+static int
+adjust_column (int line_width, int max_width, int column)
+
+{
+  int right_margin = 10;
+  gcc_checking_assert (line_width >= column);
+  if (line_width >= max_width)
+    {
+      right_margin = MIN (line_width - column, right_margin);
+      right_margin = max_width - right_margin;
+      if (column > right_margin)
+	return right_margin;
+    }
+  return column;
+}
+
 /* If LINE is longer than MAX_WIDTH, and COLUMN is not smaller than
    MAX_WIDTH by some margin, then adjust the start of the line such
    that the COLUMN is smaller than MAX_WIDTH minus the margin.  The
    margin is either 10 characters or the difference between the column
    and the length of the line, whatever is smaller.  The length of
    LINE is given by LINE_WIDTH.  */
 static const char *
 adjust_line (const char *line, int line_width,
 	     int max_width, int *column_p)
 {
-  int right_margin = 10;
-  int column = *column_p;
+  int old_column = *column_p;
 
-  gcc_checking_assert (line_width >= column);
-  right_margin = MIN (line_width - column, right_margin);
-  right_margin = max_width - right_margin;
-  if (line_width >= max_width && column > right_margin)
-    {
-      line += column - right_margin;
-      *column_p = right_margin;
-    }
+  *column_p = adjust_column (line_width, max_width, old_column);
+  line += old_column - *column_p;
   return line;
 }
 
+static const char *
+get_source_line_and_column (location_t loc, int *line_width, int *column)
+{
+  expanded_location s = expand_location_to_spelling_point (loc);
+  const char * line = location_get_source_line (s, line_width);
+  *column = s.column;
+  if (s.column > *line_width)
+    return NULL;
+
+  return line;
+}
 /* Print the physical source line corresponding to the location of
    this diagnostic, and a caret indicating the precise column.  */
 void
 diagnostic_show_locus (diagnostic_context * context,
 		       const diagnostic_info *diagnostic)
 {
   const char *line;
-  int line_width;
+  int line_width, column;
   char *buffer;
-  expanded_location s;
   int max_width;
   const char *saved_prefix;
   const char *caret_cs, *caret_ce;
 
   if (!context->show_caret
       || diagnostic->location <= BUILTINS_LOCATION
       || diagnostic->location == context->last_location)
     return;
 
   context->last_location = diagnostic->location;
-  s = expand_location_to_spelling_point (diagnostic->location);
-  line = location_get_source_line (s, &line_width);
-  if (line == NULL || s.column > line_width)
+  line = get_source_line_and_column (diagnostic->location,
+				     &line_width, &column);
+  if (line == NULL)
     return;
 
   max_width = context->caret_max_width;
-  line = adjust_line (line, line_width, max_width, &(s.column));
+  line = adjust_line (line, line_width, max_width, &column);
 
   pp_newline (context->printer);
   saved_prefix = pp_get_prefix (context->printer);
   pp_set_prefix (context->printer, NULL);
   pp_space (context->printer);
@@ -325,13 +345,13 @@  diagnostic_show_locus (diagnostic_contex
   pp_newline (context->printer);
   caret_cs = colorize_start (pp_show_color (context->printer), "caret");
   caret_ce = colorize_stop (pp_show_color (context->printer));
 
   /* pp_printf does not implement %*c.  */
-  size_t len = s.column + 3 + strlen (caret_cs) + strlen (caret_ce);
+  size_t len = column + 3 + strlen (caret_cs) + strlen (caret_ce);
   buffer = XALLOCAVEC (char, len);
-  snprintf (buffer, len, "%s %*c%s", caret_cs, s.column, context->caret_char,
+  snprintf (buffer, len, "%s %*c%s", caret_cs, column, context->caret_char,
 	    caret_ce);
   pp_string (context->printer, buffer);
   pp_set_prefix (context->printer, saved_prefix);
   pp_needs_newline (context->printer) = true;
 }
@@ -439,10 +459,11 @@  diagnostic_action_after_output (diagnost
     {
     case DK_DEBUG:
     case DK_NOTE:
     case DK_ANACHRONISM:
     case DK_WARNING:
+    case DK_FIXIT:
       break;
 
     case DK_ERROR:
     case DK_SORRY:
       if (context->abort_on_error)
@@ -940,10 +961,55 @@  inform (location_t location, const char
   diagnostic_set_info (&diagnostic, gmsgid, &ap, location, DK_NOTE);
   report_diagnostic (&diagnostic);
   va_end (ap);
 }
 
+/* A fix-it hint at LOCATION.  Use this recommended textual
+   replacements after another diagnostic message.  */
+void
+fixit_hint (location_t location, const char *msg, ...)
+{
+  diagnostic_info diagnostic;
+  va_list ap;
+
+  va_start (ap, msg);
+  diagnostic_set_info (&diagnostic, msg, &ap, location, DK_FIXIT);
+  if (!global_dc->show_caret)
+    {
+      report_diagnostic (&diagnostic);
+    }
+  else 
+    {
+      diagnostic_context * context = global_dc;
+      const char *line;
+      int line_width, column;
+      line = get_source_line_and_column (location, &line_width, &column);
+      if (line == NULL)
+	{
+	  va_end (ap);
+	  return;
+	}
+      column = adjust_column (line_width, context->caret_max_width, column);
+
+      const char *saved_prefix = pp_get_prefix (context->printer);
+      pp_set_prefix (context->printer, NULL);
+
+      const char *caret_cs, *caret_ce;
+      caret_cs = colorize_start (pp_show_color (context->printer), "caret");
+      caret_ce = colorize_stop (pp_show_color (context->printer));
+
+      /* pp_printf does not implement %*c.  */
+      size_t len = strlen (caret_cs) + column + 3 + strlen(msg) + strlen (caret_ce);
+      char * buffer = XALLOCAVEC (char, len);
+      snprintf (buffer, len, "%s%*c%s%s", caret_cs, column, ' ', msg, caret_ce);
+      pp_string (context->printer, buffer);
+      pp_newline_and_flush (context->printer);
+      pp_set_prefix (context->printer, saved_prefix);
+    }
+  va_end (ap);
+}
+
 /* An informative note at LOCATION.  Use this for additional details on an
    error message.  */
 void
 inform_n (location_t location, int n, const char *singular_gmsgid,
           const char *plural_gmsgid, ...)
Index: gcc/diagnostic-core.h
===================================================================
--- gcc/diagnostic-core.h	(revision 214756)
+++ gcc/diagnostic-core.h	(working copy)
@@ -70,10 +70,11 @@  extern void fatal_error (const char *, .
 extern bool pedwarn (location_t, int, const char *, ...)
      ATTRIBUTE_GCC_DIAG(3,4);
 extern bool permerror (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern void sorry (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern void inform (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
+extern void fixit_hint (location_t, const char *, ...) ATTRIBUTE_GCC_DIAG(2,3);
 extern void inform_n (location_t, int, const char *, const char *, ...)
     ATTRIBUTE_GCC_DIAG(3,5) ATTRIBUTE_GCC_DIAG(4,5);
 extern void verbatim (const char *, ...) ATTRIBUTE_GCC_DIAG(1,2);
 extern bool emit_diagnostic (diagnostic_t, location_t, int,
 			     const char *, ...) ATTRIBUTE_GCC_DIAG(4,5);
Index: gcc/testsuite/g++.old-deja/g++.oliva/typename1.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.oliva/typename1.C	(revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.oliva/typename1.C	(working copy)
@@ -10,7 +10,8 @@  template <class T> struct foo;
 template <class T> struct bar {
   typedef int foo;
 };
 
 template <class T> struct baz {
-  typedef bar<T>::foo foo; // { dg-error "" } missing typename
+  typedef bar<T>::foo foo; // { dg-error "typename" "missing typename" }
+  // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 15 }
 };
Index: gcc/testsuite/g++.old-deja/g++.oliva/typename2.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.oliva/typename2.C	(revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.oliva/typename2.C	(working copy)
@@ -21,8 +21,9 @@  struct foo;
 template <class T> struct bar {
   typedef int foo;
 };
 
 template <class T> struct baz {
-  typedef bar<T>::foo foo; // { dg-error "" } implicit typename
+  typedef bar<T>::foo foo; // { dg-error "typename" "implicit typename" }
+  // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 26 }
   void m(foo); 
 };
Index: gcc/testsuite/g++.old-deja/g++.other/typename1.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.other/typename1.C	(revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.other/typename1.C	(working copy)
@@ -12,6 +12,7 @@  public:
 
 template<class T>
 void f()
 {
   Vector<T>::iterator i = 0; // { dg-error "typename" "typename" } missing typename
-} // { dg-error "expected" "expected" { target *-*-* } 16 }
+  // { dg-message "3:fixit: typename" "fixit" { target *-*-*} 16 }
+} // { dg-bogus "expected" "expected" { xfail *-*-* } 16 }
Index: gcc/testsuite/g++.old-deja/g++.pt/typename6.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.pt/typename6.C	(revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.pt/typename6.C	(working copy)
@@ -14,7 +14,8 @@  struct B : public A<U>
   // { dg-message "note" "note" { target *-*-* } 13 }
 };
 
 template <class U>
 A<U>::A_Type B<U>::Func()       // { dg-error "typename" } function
+// { dg-message "1:fixit: typename" "fixit" { target *-*-*} 18 }
 {				
 }
Index: gcc/testsuite/g++.old-deja/g++.pt/typename3.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.pt/typename3.C	(revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.pt/typename3.C	(working copy)
@@ -16,6 +16,7 @@  struct B : public A<U>
 };
 
 
 template <class U>
 B<U>::A_Type B<U>::Func() { // { dg-error "typename" } implicit typename
+ // { dg-message "1:fixit: typename" "fixit" { target *-*-*} 20 }
 }
Index: gcc/testsuite/g++.old-deja/g++.pt/typename4.C
===================================================================
--- gcc/testsuite/g++.old-deja/g++.pt/typename4.C	(revision 214756)
+++ gcc/testsuite/g++.old-deja/g++.pt/typename4.C	(working copy)
@@ -21,6 +21,7 @@  struct C : public B<U>
 };
 
 
 template <class U>
 C<U>::A_Type C<U>::Func() { // { dg-error "typename" } implicit typename
+// { dg-message "1:fixit: typename" "fixit" { target *-*-*} 25 }
 }
Index: gcc/testsuite/g++.dg/parse/error36.C
===================================================================
--- gcc/testsuite/g++.dg/parse/error36.C	(revision 214756)
+++ gcc/testsuite/g++.dg/parse/error36.C	(working copy)
@@ -9,23 +9,28 @@  template <class T> struct A
 
 template <class T>
 void f(T t)
 {
   typedef A<T>::foo type;	// { dg-error "typename" }
+  // { dg-message "11:fixit: typename" "fixit" { target *-*-* } 13 }
   A<T>::bar b;			// { dg-error "typename" "typename" }
-} // { dg-error "expected ';'" "expected" { target *-*-* } 14 }
+  // { dg-message "3:fixit: typename" "fixit" { target *-*-* } 15 }
+} // { dg-bogus "expected ';'" "expected" { xfail *-*-* } 15 }
 
 // PR c++/36353
 template <class T> struct B
 {
   void f()
   {
     A<T>::baz z;		// { dg-error "typename" "typename" }
-  } // { dg-error "expected ';'" "expected" { target *-*-* } 22 }
+  // { dg-message "5:fixit: typename" "fixit" { target *-*-* } 24 }
+  } // { dg-bogus "expected ';'" "expected" { xfail *-*-* } 24 }
 };
 
 // PR c++/40738
 template <class T>
 void g(const A<T>::type &t);	// { dg-error "typename" "typename" }
+// { dg-message "14:fixit: typename" "fixit" { target *-*-* } 31 }
 
 // PR c++/18451
 template <class T> A<T>::B A<T>::b; // { dg-error "typename" }
+// { dg-message "20:fixit: typename" "fixit" { target *-*-* } 35 }
Index: gcc/testsuite/g++.dg/parse/typedef2.C
===================================================================
--- gcc/testsuite/g++.dg/parse/typedef2.C	(revision 214756)
+++ gcc/testsuite/g++.dg/parse/typedef2.C	(working copy)
@@ -1,3 +1,4 @@ 
 template <typename T> struct B { typedef typename T::X X; };
-template <typename T> struct A { typedef B<T>::X::Y Z; }; // { dg-error "" }
- 
+template <typename T> struct A { typedef B<T>::X::Y Z; }; // { dg-error "typename" }
+// { dg-message "42:fixit: typename" "fixit" { target *-*-* } 2 }
+
Index: gcc/testsuite/g++.dg/template/error6.C
===================================================================
--- gcc/testsuite/g++.dg/template/error6.C	(revision 214756)
+++ gcc/testsuite/g++.dg/template/error6.C	(working copy)
@@ -2,10 +2,11 @@  template<int n>
 struct tento {
   enum {value = 10*tento<n-1>::value};
 };
 
 struct tento<0> { // { dg-error "" }
+  // { dg-message "1:fixit: template<>" "fixit" { target *-*-*} 6 }
    enum {value=1};
 };
 
 int main() {
   if (tento<4>::value != 10000) return -1;
Index: gcc/testsuite/g++.dg/template/dependent-name5.C
===================================================================
--- gcc/testsuite/g++.dg/template/dependent-name5.C	(revision 214756)
+++ gcc/testsuite/g++.dg/template/dependent-name5.C	(working copy)
@@ -15,17 +15,19 @@  struct A
   struct N {};
 
   typedef Bar          type1;
   typedef A::Bar       type2;
   typedef A<T>::Bar    type3;
-  typedef A<T*>::Bar    type4;  // { dg-error "" }
+  typedef A<T*>::Bar    type4;  // { dg-error "typename" }
+  // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 20 }
   typedef typename A<T*>::Bar type5;
 
   typedef N<int>       type6;
   typedef A::N<int>    type7;
   typedef A<T>::N<int> type8;
-  typedef A<T*>::template N<int> type9;  // { dg-error "" }
+  typedef A<T*>::template N<int> type9;  // { dg-error "typename" }
+  // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 27 }
   typedef typename A<T*>::template N<int> type10;
 
   typedef D Bar2;
   struct N2 { typedef int K; };
 
@@ -34,11 +36,12 @@  struct A
   typedef A::Bar2 type11;
   typedef type11::K k3;
 
   typedef A::N2 type12;
   typedef typename type12::K k2;
-  typedef type12::K k1;  // { dg-error "" }
+  typedef type12::K k1;  // { dg-error "typename" }
+  // { dg-message "11:fixit: typename" "fixit" { target *-*-*} 41 }
 
   // Check that A::Bar2 is not considered dependent even if we use
   // the typename keyword.
   typedef typename A::Bar2 type13;
   typedef type13::K k4;
Index: gcc/testsuite/g++.dg/template/crash83.C
===================================================================
--- gcc/testsuite/g++.dg/template/crash83.C	(revision 214756)
+++ gcc/testsuite/g++.dg/template/crash83.C	(working copy)
@@ -1,5 +1,6 @@ 
 // PR c++/37650
 
 template<int> struct A {};
 
 template<typename = class A<0>: > struct B {}; // { dg-error "explicit specialization|expected" }
+// { dg-message "21:fixit: template<>" "fixit" { target *-*-* } 5 }
Index: gcc/testsuite/g++.dg/template/typename3.C
===================================================================
--- gcc/testsuite/g++.dg/template/typename3.C	(revision 214756)
+++ gcc/testsuite/g++.dg/template/typename3.C	(working copy)
@@ -2,6 +2,7 @@ 
 // crash test - PR 7266
 
 template <class A>
 struct B {
  typedef A::C::D E;  // { dg-error "" }
+  // { dg-message "10:fixit: typename" "fixit" { target *-*-*} 6 }
 };
Index: gcc/cp/parser.c
===================================================================
--- gcc/cp/parser.c	(revision 214756)
+++ gcc/cp/parser.c	(working copy)
@@ -2992,13 +2992,16 @@  cp_parser_diagnose_invalid_type_name (cp
 	    error_at (location, "and %qT has no template constructors",
 		      parser->scope);
 	}
       else if (TYPE_P (parser->scope)
 	       && dependent_scope_p (parser->scope))
-	error_at (location, "need %<typename%> before %<%T::%E%> because "
-		  "%qT is a dependent scope",
-		  parser->scope, id, parser->scope);
+	{
+	  error_at (location, "%<typename%> is needed before %<%T::%E%> because "
+		    "%qT is a dependent scope",
+		    parser->scope, id, parser->scope);
+	  fixit_hint (location, "typename ");
+	}
       else if (TYPE_P (parser->scope))
 	{
 	  if (cp_lexer_next_token_is (parser->lexer, CPP_LESS))
 	    error_at (location_of (id),
 		      "%qE in %q#T does not name a template type",
@@ -9813,13 +9816,16 @@  cp_parser_expression_statement (cp_parse
   /* Give a helpful message for "A<T>::type t;" and the like.  */
   if (cp_lexer_next_token_is_not (parser->lexer, CPP_SEMICOLON)
       && !cp_parser_uncommitted_to_tentative_parse_p (parser))
     {
       if (TREE_CODE (statement) == SCOPE_REF)
-	error_at (token->location, "need %<typename%> before %qE because "
-		  "%qT is a dependent scope",
-		  statement, TREE_OPERAND (statement, 0));
+	{
+	  error_at (token->location, "%<typename%> is needed before %qE because "
+		    "%qT is a dependent scope",
+		    statement, TREE_OPERAND (statement, 0));
+	  fixit_hint (token->location, "typename ");
+	}
       else if (is_overloaded_fn (statement)
 	       && DECL_CONSTRUCTOR_P (get_first_fn (statement)))
 	{
 	  /* A::A a; */
 	  tree fn = get_first_fn (statement);
@@ -19820,10 +19826,12 @@  cp_parser_class_head (cp_parser* parser,
   /* Look for the class-key.  */
   class_key = cp_parser_class_key (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);
 
   /* If the next token is `::', that is invalid -- but sometimes
      people do try to write:
@@ -20036,12 +20044,14 @@  cp_parser_class_head (cp_parser* parser,
      it is not, try to recover gracefully.  */
   if (at_namespace_scope_p ()
       && parser->num_template_parameter_lists == 0
       && template_id_p)
     {
-      error_at (type_start_token->location,
+      error_at (class_head_start_location,
 		"an explicit specialization must be preceded by %<template <>%>");
+      fixit_hint (class_head_start_location, "template<> ");
+
       invalid_explicit_specialization_p = true;
       /* Take the same action that would have been taken by
 	 cp_parser_explicit_specialization.  */
       ++parser->num_template_parameter_lists;
       begin_specialization ();