[PATCH] Fix PR c++/16160

[PATCH] Fix PR c++/16160

[Patrick Palka]

This patch fixes the above PR where it was reported that the C++
frontend does not reject the malformed class declaration

    struct X<5>;

Instead of rejecting it, the FE treats this declaration as if it were a
forward declaration of a template specialization, i.e. as if it were
written

    template<> struct X<5>;

First off, the FE should reject the declaration because it is malformed
(not 100% sure, though).  Second, since the user probably intended to
have written an explicit template instantiation (as in the PR), the FE
should suggest adding "template" before such a declaration, that is the
declaration

    struct X<5>; // error + suggest adding "template"

This patch does both these things along with adding error messages +
suggestions for

    struct X<5> { }; // error + suggest adding "template <>"

and

    template struct X<5> { }; // error + suggest replacing with "template <>"

Bootstrap and regtesting in progress.  Does this patch look OK for trunk?

gcc/cp/ChangeLog:

	PR c++/16160
	* parser.c (cp_parser_class_head): Identify and reject malformed
	template-id declarations and definitions.
---
 gcc/cp/parser.c                          | 53 +++++++++++++++++++++++---------
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C |  2 +-
 gcc/testsuite/g++.dg/ext/attrib9.C       |  2 +-
 gcc/testsuite/g++.dg/template/crash54.C  |  2 +-
 gcc/testsuite/g++.dg/template/error55.C  | 11 +++++++
 5 files changed, 53 insertions(+), 17 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/error55.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 3290dfa..f6dc004 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -20264,6 +20264,34 @@ cp_parser_class_head (cp_parser* parser,
     }
   virt_specifiers = cp_parser_virt_specifier_seq_opt (parser);
 
+  /* Make sure a top-level template-id declaration or definition is preceded
+     by "template" or "template <>".  */
+  if (template_id_p
+      && at_namespace_scope_p ()
+      && parser->num_template_parameter_lists == 0
+      && !processing_explicit_instantiation)
+    {
+      if (cp_parser_next_token_starts_class_definition_p (parser))
+	{
+          error_at (type_start_token->location,
+		    "an explicit specialization must be preceded by "
+		    "%<template <>%>");
+	  invalid_explicit_specialization_p = true;
+	  /* Try to recover gracefully by taking the same action that would
+	     have been taken by cp_parser_explicit_specialization.  */
+	  ++parser->num_template_parameter_lists;
+	  begin_specialization ();
+	}
+      else if (cp_parser_declares_only_class_p (parser))
+	{
+          error_at (type_start_token->location,
+		    "an explicit instantiation must be preceded by "
+		    "%<template%>");
+	  type = error_mark_node;
+	  goto out;
+	}
+    }
+
   /* If it's not a `:' or a `{' then we can't really be looking at a
      class-head, since a class-head only appears as part of a
      class-specifier.  We have to detect this situation before calling
@@ -20275,6 +20303,16 @@ cp_parser_class_head (cp_parser* parser,
       goto out;
     }
 
+  if (processing_explicit_instantiation)
+    {
+      error_at (type_start_token->location,
+		"an explicit instantiation may not have a definition");
+      inform (type_start_token->location,
+	      "use %<template <>%> to define an explicit specialization");
+      type = error_mark_node;
+      goto out;
+    }
+
   /* At this point, we're going ahead with the class-specifier, even
      if some other problem occurs.  */
   cp_parser_commit_to_tentative_parse (parser);
@@ -20346,20 +20384,7 @@ cp_parser_class_head (cp_parser* parser,
 	  num_templates = 0;
 	}
     }
-  /* An explicit-specialization must be preceded by "template <>".  If
-     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,
-		"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.  */
-      ++parser->num_template_parameter_lists;
-      begin_specialization ();
-    }
+
   /* There must be no "return" statements between this point and the
      end of this function; set "type "to the correct return value and
      use "goto done;" to return.  */
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C
index 3dc51ee..4957ba1 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C
@@ -9,4 +9,4 @@ enum [[gnu::unused]] e;	// { dg-warning "already defined" }
 struct [[gnu::unused]] B *p;	//  { dg-warning "attributes" }
 
 template <class T> struct A { };
-struct [[gnu::unused]] A<int>;	//  { dg-warning "attributes" }
+struct [[gnu::unused]] A<int> y;	//  { dg-warning "attributes" }
diff --git a/gcc/testsuite/g++.dg/ext/attrib9.C b/gcc/testsuite/g++.dg/ext/attrib9.C
index 6672f75..e8e158c 100644
--- a/gcc/testsuite/g++.dg/ext/attrib9.C
+++ b/gcc/testsuite/g++.dg/ext/attrib9.C
@@ -7,4 +7,4 @@ enum __attribute__((unused)) e;	// { dg-warning "already defined" }
 struct __attribute((unused)) B *p;	//  { dg-warning "attributes" }
 
 template <class T> struct A { };
-struct __attribute((unused)) A<int>;	//  { dg-warning "attributes" }
+struct __attribute((unused)) A<int> y;	//  { dg-warning "attributes" }
diff --git a/gcc/testsuite/g++.dg/template/crash54.C b/gcc/testsuite/g++.dg/template/crash54.C
index 26b4875..b1dbec0 100644
--- a/gcc/testsuite/g++.dg/template/crash54.C
+++ b/gcc/testsuite/g++.dg/template/crash54.C
@@ -2,4 +2,4 @@
 
 template<int> struct A;
 
-struct __attribute__((unused)) A<0<; // { dg-error "template argument|unqualified-id" }
+struct __attribute__((unused)) A<0<; // { dg-error "template argument|explicit instantiation" }
diff --git a/gcc/testsuite/g++.dg/template/error55.C b/gcc/testsuite/g++.dg/template/error55.C
new file mode 100644
index 0000000..e40b3a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/error55.C
@@ -0,0 +1,11 @@
+// PR c++/16160
+
+template <int N> struct X { };
+template <int N> struct Y { };
+template <int N> struct Z { };
+
+struct X<2>; // { dg-error "explicit instantiation" }
+
+struct Y<2> { }; // { dg-error "explicit specialization" }
+
+template struct Z<2> { }; // { dg-error "may not have a definition" }
-- 
2.3.0.rc0

Re: [PATCH] Fix PR c++/16160

[Jason Merrill]

On 01/13/2015 10:54 PM, Patrick Palka wrote:
> +	  type = error_mark_node;
> +	  goto out;

Why exit early in the explicit instantiation cases?  Doesn't it work to 
give the error and continue?

Jason

Re: [PATCH] Fix PR c++/16160

[Patrick Palka]

On Wed, Jan 14, 2015 at 8:05 AM, Jason Merrill <jason@redhat.com> wrote:
> On 01/13/2015 10:54 PM, Patrick Palka wrote:
>>
>> +         type = error_mark_node;
>> +         goto out;
>
>
> Why exit early in the explicit instantiation cases?  Doesn't it work to give
> the error and continue?
>
> Jason
>

Yes it does.  I changed it to an early exit in the last minute for no
good reason.  But an earlier version of the patch that continued
instead of exiting worked correctly.

Re: [PATCH] Fix PR c++/16160

[Patrick Palka]

On Wed, Jan 14, 2015 at 8:26 AM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Wed, Jan 14, 2015 at 8:05 AM, Jason Merrill <jason@redhat.com> wrote:
>> On 01/13/2015 10:54 PM, Patrick Palka wrote:
>>>
>>> +         type = error_mark_node;
>>> +         goto out;
>>
>>
>> Why exit early in the explicit instantiation cases?  Doesn't it work to give
>> the error and continue?
>>
>> Jason
>>
>
> Yes it does.  I changed it to an early exit in the last minute for no
> good reason.  But an earlier version of the patch that continued
> instead of exiting worked correctly.

Actually I recall having issues with not exiting early in the "an
explicit instantiation may not have a definition" case.  But in the
"an explicit instantiation must be preceded by template" case one
could safely continue (even though it goes on to be processed as a
forward declaration of a template specialization instead of a mistyped
explicit instantiation...)

[PATCH] Fix PR c++/16160

[Patrick Palka]

Here is version 2 of the patch which only adjusts a couple of testcases
in g++.old-deja/ that I missed earlier.  I am unsure if the extra
dg-error in overload.C is correct.

No code changes yet in gcc/cp/ versus the original patch.  Successfully
bootstrapped and regtested on x86_64-unknown-linux-gnu.

I would advise against continuing rather than erroring out in the "an
explicit instantiation may not have a definition" case because at that
point the frontend thinks that the same declaration is simultaneously an
explicit instantiation and a template specialization, which sounds
potentially problematic (i.e. further processing may trigger ICEs or
infinite loops or something).

As for the "an explicit instantiation must be preceded by template"
case, if we continue past this error, the declaration in question will
be processed as a forward declaration to a template specialization and
not as an explicit instantiation.  So for instance the mentioned
template will not be instantiated anyway.  So there is not much
potential for further diagnostic reporting.  Doesn't sound too
problematic, but doesn't sound very useful either.

-- >8 --

This patch fixes the above PR where it was reported that the C++
frontend does not reject the malformed class declaration

    struct X<5>;

Instead of rejecting it, the FE treats this declaration as if it were a
forward declaration of a template specialization, i.e. as if it were
written

    template<> struct X<5>;

First off, the FE should reject the declaration because it is malformed
(not 100% sure, though).  Second, since the user probably intended to
have written an explicit template instantiation (as in the PR), the FE
should suggest adding "template" before such a declaration, that is the
declaration

    struct X<5>; // error + suggest adding "template"

This patch does both these things along with adding error messages +
suggestions for

    struct X<5> { }; // error + suggest adding "template <>"

and

    template struct X<5> { }; // error + suggest replacing with "template <>"

gcc/cp/ChangeLog:

	PR c++/16160
	* parser.c (cp_parser_class_head): Identify and reject malformed
	template-id declarations and definitions.

gcc/testsuite/ChangeLog:

	PR c++/16160
	* g++.dg/template/error55.C: New test.
	* g++.dg/cpp0x/gen-attrs-9.C: Adjust.
	* g++.dg/ext/attrib9.C: Adjust.
	* g++.dg/template/crash54.C: Adjust.
	* g++.old-deja/g++.jason/overload.C: Adjust.
	* g++.old-deja/g++.pt/spec24.C: Adjust.
---
 gcc/cp/parser.c                                 | 53 ++++++++++++++++++-------
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C        |  2 +-
 gcc/testsuite/g++.dg/ext/attrib9.C              |  2 +-
 gcc/testsuite/g++.dg/template/crash54.C         |  2 +-
 gcc/testsuite/g++.dg/template/error55.C         | 11 +++++
 gcc/testsuite/g++.old-deja/g++.jason/overload.C |  6 +--
 gcc/testsuite/g++.old-deja/g++.pt/spec24.C      |  3 +-
 7 files changed, 58 insertions(+), 21 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/error55.C

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 3290dfa..f6dc004 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -20264,6 +20264,34 @@ cp_parser_class_head (cp_parser* parser,
     }
   virt_specifiers = cp_parser_virt_specifier_seq_opt (parser);
 
+  /* Make sure a top-level template-id declaration or definition is preceded
+     by "template" or "template <>".  */
+  if (template_id_p
+      && at_namespace_scope_p ()
+      && parser->num_template_parameter_lists == 0
+      && !processing_explicit_instantiation)
+    {
+      if (cp_parser_next_token_starts_class_definition_p (parser))
+	{
+          error_at (type_start_token->location,
+		    "an explicit specialization must be preceded by "
+		    "%<template <>%>");
+	  invalid_explicit_specialization_p = true;
+	  /* Try to recover gracefully by taking the same action that would
+	     have been taken by cp_parser_explicit_specialization.  */
+	  ++parser->num_template_parameter_lists;
+	  begin_specialization ();
+	}
+      else if (cp_parser_declares_only_class_p (parser))
+	{
+          error_at (type_start_token->location,
+		    "an explicit instantiation must be preceded by "
+		    "%<template%>");
+	  type = error_mark_node;
+	  goto out;
+	}
+    }
+
   /* If it's not a `:' or a `{' then we can't really be looking at a
      class-head, since a class-head only appears as part of a
      class-specifier.  We have to detect this situation before calling
@@ -20275,6 +20303,16 @@ cp_parser_class_head (cp_parser* parser,
       goto out;
     }
 
+  if (processing_explicit_instantiation)
+    {
+      error_at (type_start_token->location,
+		"an explicit instantiation may not have a definition");
+      inform (type_start_token->location,
+	      "use %<template <>%> to define an explicit specialization");
+      type = error_mark_node;
+      goto out;
+    }
+
   /* At this point, we're going ahead with the class-specifier, even
      if some other problem occurs.  */
   cp_parser_commit_to_tentative_parse (parser);
@@ -20346,20 +20384,7 @@ cp_parser_class_head (cp_parser* parser,
 	  num_templates = 0;
 	}
     }
-  /* An explicit-specialization must be preceded by "template <>".  If
-     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,
-		"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.  */
-      ++parser->num_template_parameter_lists;
-      begin_specialization ();
-    }
+
   /* There must be no "return" statements between this point and the
      end of this function; set "type "to the correct return value and
      use "goto done;" to return.  */
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C
index 3dc51ee..4957ba1 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-9.C
@@ -9,4 +9,4 @@ enum [[gnu::unused]] e;	// { dg-warning "already defined" }
 struct [[gnu::unused]] B *p;	//  { dg-warning "attributes" }
 
 template <class T> struct A { };
-struct [[gnu::unused]] A<int>;	//  { dg-warning "attributes" }
+struct [[gnu::unused]] A<int> y;	//  { dg-warning "attributes" }
diff --git a/gcc/testsuite/g++.dg/ext/attrib9.C b/gcc/testsuite/g++.dg/ext/attrib9.C
index 6672f75..e8e158c 100644
--- a/gcc/testsuite/g++.dg/ext/attrib9.C
+++ b/gcc/testsuite/g++.dg/ext/attrib9.C
@@ -7,4 +7,4 @@ enum __attribute__((unused)) e;	// { dg-warning "already defined" }
 struct __attribute((unused)) B *p;	//  { dg-warning "attributes" }
 
 template <class T> struct A { };
-struct __attribute((unused)) A<int>;	//  { dg-warning "attributes" }
+struct __attribute((unused)) A<int> y;	//  { dg-warning "attributes" }
diff --git a/gcc/testsuite/g++.dg/template/crash54.C b/gcc/testsuite/g++.dg/template/crash54.C
index 26b4875..2e8e836 100644
--- a/gcc/testsuite/g++.dg/template/crash54.C
+++ b/gcc/testsuite/g++.dg/template/crash54.C
@@ -2,4 +2,4 @@
 
 template<int> struct A;
 
-struct __attribute__((unused)) A<0<; // { dg-error "template argument|unqualified-id" }
+template<> struct __attribute__((unused)) A<0<; // { dg-error "template argument|unqualified-id" }
diff --git a/gcc/testsuite/g++.dg/template/error55.C b/gcc/testsuite/g++.dg/template/error55.C
new file mode 100644
index 0000000..e40b3a1
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/error55.C
@@ -0,0 +1,11 @@
+// PR c++/16160
+
+template <int N> struct X { };
+template <int N> struct Y { };
+template <int N> struct Z { };
+
+struct X<2>; // { dg-error "explicit instantiation" }
+
+struct Y<2> { }; // { dg-error "explicit specialization" }
+
+template struct Z<2> { }; // { dg-error "may not have a definition" }
diff --git a/gcc/testsuite/g++.old-deja/g++.jason/overload.C b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
index 6a747ff..2da9c79 100644
--- a/gcc/testsuite/g++.old-deja/g++.jason/overload.C
+++ b/gcc/testsuite/g++.old-deja/g++.jason/overload.C
@@ -11,10 +11,10 @@ void operator+ (int, bar&);
 template <class T> class foo
 {
 public:
-  friend void operator+ <> (int, T&);
+  friend void operator+ <> (int, T&); // { dg-error "int&" }
 };
 
 class baz;
 
-class foo<int>;
-class foo<baz>;
+template class foo<int>;
+template class foo<baz>;
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/spec24.C b/gcc/testsuite/g++.old-deja/g++.pt/spec24.C
index 37b76cb..7909a35 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/spec24.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/spec24.C
@@ -1,5 +1,6 @@
 // { dg-do assemble  }
 
 template <class T> class A;
+
 // template <>
-class A<int>; // { dg-error "" "" { xfail *-*-* } } missing template header - 
+class A<int>; // { dg-error "explicit instantiation" }
-- 
2.3.0.rc0

Re: [PATCH] Fix PR c++/16160

[Jason Merrill]

On 01/14/2015 11:28 AM, Patrick Palka wrote:
> Second, since the user probably intended to
> have written an explicit template instantiation (as in the PR), the FE
> should suggest adding "template" before such a declaration, that is the
> declaration
>
>      struct X<5>; // error + suggest adding "template"

Actually, I think in pre-standard days this declared a specialization, 
before template<> was required.  So I think we want to treat it as a 
specialization in this case as well.

Jason

Re: [PATCH] Fix PR c++/16160

[Patrick Palka]

On Wed, Jan 14, 2015 at 4:28 PM, Jason Merrill <jason@redhat.com> wrote:
> On 01/14/2015 11:28 AM, Patrick Palka wrote:
>>
>> Second, since the user probably intended to
>> have written an explicit template instantiation (as in the PR), the FE
>> should suggest adding "template" before such a declaration, that is the
>> declaration
>>
>>      struct X<5>; // error + suggest adding "template"
>
>
> Actually, I think in pre-standard days this declared a specialization,
> before template<> was required.  So I think we want to treat it as a
> specialization in this case as well.

Did this define a specialization too:

   struct X<5> { };

or was template<> always required here?

>
> Jason
>

Re: [PATCH] Fix PR c++/16160

[Jason Merrill]

On 01/14/2015 05:04 PM, Patrick Palka wrote:
> Did this define a specialization too:
>
>     struct X<5> { };

Yes.  There's an example in the ARM that says

A class can be defined as the definition of a template class.  For example,

   template<class T> class stream { /* ... */ };
   class stream<char> { /* ... */ };

Here, the class declaration will be used as the definition of streams of 
characters (stream<char>).  Other streams will be handled by template 
functions generated from the class template.

Jason