diff mbox

[C++] PR 18747

Message ID 50412D7E.7010203@redhat.com
State New
Headers show

Commit Message

Jason Merrill Aug. 31, 2012, 9:32 p.m. UTC 
Since you're traveling, I poked at this myself some more.  The issue 
here is that there are too many template headers for the declaration, so 
we want to figure out what the right number is and give an appropriate 
message.

Tested x86_64-pc-linux-gnu, applying to trunk.

Comments

Paolo Carlini Sept. 5, 2012, 4:41 p.m. UTC | #1 
Hi,

On 08/31/2012 11:32 PM, Jason Merrill wrote:
> Since you're traveling, I poked at this myself some more.  The issue 
> here is that there are too many template headers for the declaration, 
> so we want to figure out what the right number is and give an 
> appropriate message.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.

Thanks for looking into this. Now I wonder if we made progress on a 
couple of long standing PRs where we weren't strict enough by one with 
the number of 'template <>'. Let me check...

Paolo.
Paolo Carlini Sept. 5, 2012, 4:52 p.m. UTC | #2 
On 09/05/2012 06:41 PM, Paolo Carlini wrote:
> Thanks for looking into this. Now I wonder if we made progress on a 
> couple of long standing PRs where we weren't strict enough by one with 
> the number of 'template <>'. Let me check...
Nope, apparently c++/24314 is still there. But maybe it's easier to fix 
now (I suspect however that the problem has to do with the logic of 
num_template_headers_for_class, which I think you didn't change)

Paolo.
Paolo Carlini Sept. 5, 2012, 6:17 p.m. UTC | #3 
On 09/05/2012 06:52 PM, Paolo Carlini wrote:
> On 09/05/2012 06:41 PM, Paolo Carlini wrote:
>> Thanks for looking into this. Now I wonder if we made progress on a 
>> couple of long standing PRs where we weren't strict enough by one 
>> with the number of 'template <>'. Let me check...
> Nope, apparently c++/24314 is still there. But maybe it's easier to 
> fix now (I suspect however that the problem has to do with the logic 
> of num_template_headers_for_class, which I think you didn't change)
In fact, something seems weird earlier, in 
cp_parser_check_template_parameters. It has:

   /* If there are the same number of template classes and parameter
      lists, that's OK.  */
   if (parser->num_template_parameter_lists == num_templates)
     return true;
   /* If there are more, but only one more, then we are referring to a
      member template.  That's OK too.  */
   if (parser->num_template_parameter_lists == num_templates + 1)
     return true;

but note that for:

template <class T>
struct A
{
    int select() { return 0; }
};

we have parser->num_template_parameter_lists == 1 and num_templates == 
0. Thus it seems that the case 'num_templates + 1' isn't (just) about 
member templates...

Paolo.
Jason Merrill Sept. 6, 2012, 12:03 a.m. UTC | #4 
On 09/05/2012 02:17 PM, Paolo Carlini wrote:
> In fact, something seems weird earlier, in
> cp_parser_check_template_parameters. It has:
>
>    /* If there are the same number of template classes and parameter
>       lists, that's OK.  */
>    if (parser->num_template_parameter_lists == num_templates)
>      return true;
>    /* If there are more, but only one more, then we are referring to a
>       member template.  That's OK too.  */
>    if (parser->num_template_parameter_lists == num_templates + 1)
>      return true;

Right.

> but note that for:
>
> template <class T>
> struct A
> {
>     int select() { return 0; }
> };
>
> we have parser->num_template_parameter_lists == 1 and num_templates ==
> 0. Thus it seems that the case 'num_templates + 1' isn't (just) about
> member templates...

That's odd, num_templates should be 1.  And I notice that 
cp_parser_check_declarator_template_parameters has another copy of the 
num_template_headers_for_class logic; they should be merged.

I think the problem with 24314 is that we try to decide how many 
template headers we want before we determine what declaration we're 
looking at.  When we have a redefinition or specialization, we know 
exactly how many headers we want, and we should check accordingly rather 
than say N or N+1.

Jason
diff mbox

Patch

commit 7c8fead9f721b04227c6790a3dbe256e93144d46
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Aug 31 16:27:37 2012 -0400

    	PR c++/18747
    	* pt.c (check_template_variable): New.
    	(num_template_headers_for_class): Split out...
    	* decl.c (grokdeclarator): ...from here.
    	(start_decl): Remove redundant diagnostic.
    	* cp-tree.h: Declare them
    	* parser.c (cp_parser_single_declaration): Call check_template_variable.
    .
    
    git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/trunk@190830 138bc75d-0d04-0410-961f-82ee72b054a4

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 1b085bd..bd57b92 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5316,6 +5316,8 @@  extern void end_specialization			(void);
 extern void begin_explicit_instantiation	(void);
 extern void end_explicit_instantiation		(void);
 extern tree check_explicit_specialization	(tree, tree, int, int);
+extern int num_template_headers_for_class	(tree);
+extern void check_template_variable		(tree);
 extern tree make_auto				(void);
 extern tree do_auto_deduction			(tree, tree, tree);
 extern tree type_uses_auto			(tree);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index c909dea..8b94e26 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -4461,11 +4461,6 @@  start_decl (const cp_declarator *declarator,
 			       context, DECL_NAME (decl));
 		  DECL_CONTEXT (decl) = DECL_CONTEXT (field);
 		}
-	      if (processing_specialization
-		  && template_class_depth (context) == 0
-		  && CLASSTYPE_TEMPLATE_SPECIALIZATION (context))
-		error ("template header not allowed in member definition "
-		       "of explicitly specialized class");
 	      /* Static data member are tricky; an in-class initialization
 		 still doesn't provide a definition, so the in-class
 		 declaration will have DECL_EXTERNAL set, but will have an
@@ -9564,36 +9559,9 @@  grokdeclarator (const cp_declarator *declarator,
       && declarator->u.id.qualifying_scope
       && MAYBE_CLASS_TYPE_P (declarator->u.id.qualifying_scope))
     {
-      tree t;
-
       ctype = declarator->u.id.qualifying_scope;
       ctype = TYPE_MAIN_VARIANT (ctype);
-      t = ctype;
-      while (t != NULL_TREE && CLASS_TYPE_P (t))
-	{
-	  /* You're supposed to have one `template <...>' for every
-	     template class, but you don't need one for a full
-	     specialization.  For example:
-
-	       template <class T> struct S{};
-	       template <> struct S<int> { void f(); };
-	       void S<int>::f () {}
-
-	     is correct; there shouldn't be a `template <>' for the
-	     definition of `S<int>::f'.  */
-	  if (CLASSTYPE_TEMPLATE_SPECIALIZATION (t)
-	      && !any_dependent_template_arguments_p (CLASSTYPE_TI_ARGS (t)))
-	    /* T is an explicit (not partial) specialization.  All
-	       containing classes must therefore also be explicitly
-	       specialized.  */
-	    break;
-	  if ((CLASSTYPE_USE_TEMPLATE (t) || CLASSTYPE_IS_TEMPLATE (t))
-	      && PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (t)))
-	    template_count += 1;
-
-	  t = TYPE_MAIN_DECL (t);
-	  t = DECL_CONTEXT (t);
-	}
+      template_count = num_template_headers_for_class (ctype);
 
       if (ctype == current_class_type)
 	{
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 091a967..60ba380 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -21310,8 +21310,8 @@  cp_parser_perform_template_parameter_access_checks (VEC (deferred_access_check,g
 }
 
 /* Parse a `decl-specifier-seq [opt] init-declarator [opt] ;' or
-   `function-definition' sequence.  MEMBER_P is true, this declaration
-   appears in a class scope.
+   `function-definition' sequence that follows a template header.
+   If MEMBER_P is true, this declaration appears in a class scope.
 
    Returns the DECL for the declared entity.  If FRIEND_P is non-NULL,
    *FRIEND_P is set to TRUE iff the declaration is a friend.  */
@@ -21431,6 +21431,9 @@  cp_parser_single_declaration (cp_parser* parser,
 		  "explicit template specialization cannot have a storage class");
         decl = error_mark_node;
       }
+
+    if (decl && TREE_CODE (decl) == VAR_DECL)
+      check_template_variable (decl);
     }
 
   /* Look for a trailing `;' after the declaration.  */
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 5ce6e8a..4a39427 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -2208,6 +2208,66 @@  copy_default_args_to_explicit_spec (tree decl)
   TREE_TYPE (decl) = new_type;
 }
 
+/* Return the number of template headers we expect to see for a definition
+   or specialization of CTYPE or one of its non-template members.  */
+
+int
+num_template_headers_for_class (tree ctype)
+{
+  int template_count = 0;
+  tree t = ctype;
+  while (t != NULL_TREE && CLASS_TYPE_P (t))
+    {
+      /* You're supposed to have one `template <...>' for every
+	 template class, but you don't need one for a full
+	 specialization.  For example:
+
+	 template <class T> struct S{};
+	 template <> struct S<int> { void f(); };
+	 void S<int>::f () {}
+
+	 is correct; there shouldn't be a `template <>' for the
+	 definition of `S<int>::f'.  */
+      if (CLASSTYPE_TEMPLATE_SPECIALIZATION (t)
+	  && !any_dependent_template_arguments_p (CLASSTYPE_TI_ARGS (t)))
+	/* T is an explicit (not partial) specialization.  All
+	   containing classes must therefore also be explicitly
+	   specialized.  */
+	break;
+      if ((CLASSTYPE_USE_TEMPLATE (t) || CLASSTYPE_IS_TEMPLATE (t))
+	  && PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (t)))
+	template_count += 1;
+
+      t = TYPE_MAIN_DECL (t);
+      t = DECL_CONTEXT (t);
+    }
+
+  return template_count;
+}
+
+/* Do a simple sanity check on the template headers that precede the
+   variable declaration DECL.  */
+
+void
+check_template_variable (tree decl)
+{
+  tree ctx = CP_DECL_CONTEXT (decl);
+  int wanted = num_template_headers_for_class (ctx);
+  if (!TYPE_P (ctx) || !CLASSTYPE_TEMPLATE_INFO (ctx))
+    permerror (DECL_SOURCE_LOCATION (decl),
+	       "%qD is not a static data member of a class template", decl);
+  else if (template_header_count > wanted)
+    {
+      pedwarn (DECL_SOURCE_LOCATION (decl), 0,
+	       "too many template headers for %D (should be %d)",
+	       decl, wanted);
+      if (CLASSTYPE_TEMPLATE_SPECIALIZATION (ctx))
+	inform (DECL_SOURCE_LOCATION (decl),
+		"members of an explicitly specialized class are defined "
+		"without a template header");
+    }
+}
+
 /* Check to see if the function just declared, as indicated in
    DECLARATOR, and in DECL, is a specialization of a function
    template.  We may also discover that the declaration is an explicit
diff --git a/gcc/testsuite/g++.dg/parse/error50.C b/gcc/testsuite/g++.dg/parse/error50.C
new file mode 100644
index 0000000..dbd8958
--- /dev/null
+++ b/gcc/testsuite/g++.dg/parse/error50.C
@@ -0,0 +1,18 @@ 
+// PR c++/18747
+
+template<> int i;   // { dg-error "template" }
+
+struct A
+{
+  static int i;
+};
+
+template<> int A::i;     // { dg-error "template" }
+
+template <class T>
+struct B
+{
+  static T i;
+};
+
+template<> template <> int B<int>::i; // { dg-error "should be 1" }