diff mbox

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

Message ID 1461853698-43954-1-git-send-email-dmalcolm@redhat.com
State New
Headers show

Commit Message

David Malcolm April 28, 2016, 2:28 p.m. UTC
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

Comments

Trevor Saunders April 28, 2016, 2:30 p.m. UTC | #1
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
Jason Merrill April 29, 2016, 9:29 p.m. UTC | #2
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
diff mbox

Patch

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