[C++ PATCH] Nested non-type template parameter packs using outer template parameter packs (PR c++/35022)

[C++ PATCH] Nested non-type template parameter packs using outer template parameter packs (PR c++/35022)

[Doug Gregor]

[-- Attachment #1: Type: text/plain, Size: 2178 bytes --]

This patch fixes PR c++/35022, a regression involving non-type
template parameter packs whose type involves template parameter packs
from an outer level of templates. The basic problem shown in PR
c++/35022 is that we weren't properly dealing with
tsubst_pack_expansion when it returns a pack expansion, as it does
when we're substituting into a nested template. This patch fixes this
problem in two contexts: when coercing template arguments to a
non-type template parameter pack  (the ICE from the PR) and also when
dealing with the "sizeof...(X)" form of SIZEOF_EXPR.
Also, the actual PR was an ice-on-invalid-code, where adding an
ellipsis makes the code ill-formed. Unfortunately, that illuminated an
error in the parsing logic, where we weren't properly parsing an
unnamed non-type template parameter pack. The parser tweaks in this
patch fix that problem.

The astute reviewer will note that there are 3 xfails in this new test
case. These are examples that I believe should be well-formed, but (1)
it's not entirely clear that the C++0x working paper agrees with me,
(2) the results of this test case will change anyway if N2488 goes
through in two weeks, and (3) I don't have a fix, but the probable fix
would affect too much C++98 code for me to propose at this stage of
GCC development.

Tested i686-pc-linux-gnu, fixes a regression; okay for 4.3?

  - Doug

2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>

	PR c++/35022
	* pt.c (coerce_template_parameter_pack): Cope with
	tsubst_pack_expansion returning a pack expansion (as occurs when
	substituting into a nested template).
	(tsubst_copy) <case SIZEOF_EXPR>: Cope with tsubst_pack_expansion
	returning a pack expansion, or a TREE_VEC ending in a pack
	expansion, both of which can occur when substituting into a nested
	template.
	(tsubst_copy_and_build) <case SIZEOF_EXPR>: When we're
	instantiating the sizeof...(X) form, make tsubst_copy do the
	work.
	* parser.c (cp_parser_template_parameter): Deal with unnamed
	non-type template parameter packs identified by pack expansions in
	the parameter type.
	
2008-02-15  Douglas Gregor  <doug.gregor@gmail.com>

	PR c++/35022
	* g++.dg/cpp0x/vt-35022.C: New.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: variadic-ttp.patch --]
[-- Type: text/x-patch; name=variadic-ttp.patch, Size: 6982 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 132340)
+++ cp/pt.c	(working copy)
@@ -5045,8 +5045,19 @@ coerce_template_parameter_pack (tree par
       if (packed_types == error_mark_node)
         return error_mark_node;
 
+      /* If the type of the non-type template parameter pack includes
+	 a template parameter pack from an outer template, we can end
+	 up with a pack expansion as the result of
+	 tsubst_pack_expansion. In this case, just pretend that there
+	 are no "packed types".  That way, we'll just compare each of
+	 the template arguments against this same template parameter
+	 type.  */
+      if (PACK_EXPANSION_P (packed_types))
+	packed_types = NULL_TREE;
+
       /* Check that we have the right number of arguments.  */
       if (arg_idx < nargs
+	  && packed_types
           && !PACK_EXPANSION_P (TREE_VEC_ELT (inner_args, arg_idx))
           && nargs - arg_idx != TREE_VEC_LENGTH (packed_types))
         {
@@ -5063,6 +5074,7 @@ coerce_template_parameter_pack (tree par
          template parameter pack are, in fact, valid for non-type
          template parameters.  */
       if (arg_idx < nargs 
+	  && packed_types
           && PACK_EXPANSION_P (TREE_VEC_ELT (inner_args, arg_idx)))
         {
           int j, len = TREE_VEC_LENGTH (packed_types);
@@ -9836,9 +9848,29 @@ tsubst_copy (tree t, tree args, tsubst_f
           /* We only want to compute the number of arguments.  */
           tree expanded = tsubst_pack_expansion (TREE_OPERAND (t, 0), args,
                                                 complain, in_decl);
+	  int len;
+
+	  if (TREE_CODE (expanded) == TREE_VEC)
+	    len = TREE_VEC_LENGTH (expanded);
+
 	  if (expanded == error_mark_node)
 	    return error_mark_node;
-          return build_int_cst (size_type_node, TREE_VEC_LENGTH (expanded));
+	  else if (PACK_EXPANSION_P (expanded)
+		   || (TREE_CODE (expanded) == TREE_VEC
+		       && len > 0
+		       && PACK_EXPANSION_P (TREE_VEC_ELT (expanded, len-1))))
+	    {
+	      if (TREE_CODE (expanded) == TREE_VEC)
+		expanded = TREE_VEC_ELT (expanded, len - 1);
+
+	      if (TYPE_P (expanded))
+		return cxx_sizeof_or_alignof_type (expanded, SIZEOF_EXPR, 
+						   true);
+	      else
+		return cxx_sizeof_or_alignof_expr (expanded, SIZEOF_EXPR);
+	    }
+	  else
+	    return build_int_cst (size_type_node, len);
         }
       /* Fall through */
 
@@ -10834,14 +10866,7 @@ tsubst_copy_and_build (tree t,
 
     case SIZEOF_EXPR:
       if (PACK_EXPANSION_P (TREE_OPERAND (t, 0)))
-        {
-          /* We only want to compute the number of arguments.  */
-          tree expanded = tsubst_pack_expansion (TREE_OPERAND (t, 0), args,
-                                                complain, in_decl);
-	  if (expanded == error_mark_node)
-	    return error_mark_node;
-          return build_int_cst (size_type_node, TREE_VEC_LENGTH (expanded));
-        }
+	return tsubst_copy (t, args, complain, in_decl);
       /* Fall through */
       
     case ALIGNOF_EXPR:
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 132340)
+++ cp/parser.c	(working copy)
@@ -9401,29 +9401,41 @@ cp_parser_template_parameter (cp_parser*
       maybe_warn_variadic_templates ();
       
       *is_parameter_pack = true;
+    }
+  /* We might end up with a pack expansion as the type of the non-type
+     template parameter, in which case this is a non-type template
+     parameter pack.  */
+  else if (parameter_declarator
+	   && parameter_declarator->decl_specifiers.type
+	   && PACK_EXPANSION_P (parameter_declarator->decl_specifiers.type))
+    {
+      *is_parameter_pack = true;
+      parameter_declarator->decl_specifiers.type = 
+	PACK_EXPANSION_PATTERN (parameter_declarator->decl_specifiers.type);
+    }
 
+  if (*is_parameter_pack && cp_lexer_next_token_is (parser->lexer, CPP_EQ))
+    {
       /* Parameter packs cannot have default arguments.  However, a
 	 user may try to do so, so we'll parse them and give an
 	 appropriate diagnostic here.  */
-      if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
-	{
-	  /* Consume the `='.  */
-	  cp_lexer_consume_token (parser->lexer);
 
-	  /* Find the name of the parameter pack.  */     
-	  id_declarator = parameter_declarator->declarator;
-	  while (id_declarator && id_declarator->kind != cdk_id)
-	    id_declarator = id_declarator->declarator;
-	  
-	  if (id_declarator && id_declarator->kind == cdk_id)
-	    error ("template parameter pack %qD cannot have a default argument",
-		   id_declarator->u.id.unqualified_name);
-	  else
-	    error ("template parameter pack cannot have a default argument");
-
-          /* Parse the default argument, but throw away the result.  */
-          cp_parser_default_argument (parser, /*template_parm_p=*/true);
-	}
+      /* Consume the `='.  */
+      cp_lexer_consume_token (parser->lexer);
+      
+      /* Find the name of the parameter pack.  */     
+      id_declarator = parameter_declarator->declarator;
+      while (id_declarator && id_declarator->kind != cdk_id)
+	id_declarator = id_declarator->declarator;
+      
+      if (id_declarator && id_declarator->kind == cdk_id)
+	error ("template parameter pack %qD cannot have a default argument",
+	       id_declarator->u.id.unqualified_name);
+      else
+	error ("template parameter pack cannot have a default argument");
+      
+      /* Parse the default argument, but throw away the result.  */
+      cp_parser_default_argument (parser, /*template_parm_p=*/true);
     }
 
   parm = grokdeclarator (parameter_declarator->declarator,
Index: testsuite/g++.dg/cpp0x/vt-35022.C
===================================================================
--- testsuite/g++.dg/cpp0x/vt-35022.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/vt-35022.C	(revision 0)
@@ -0,0 +1,52 @@
+// { dg-options "-std=c++0x" }
+template<typename... T, template<T...p> class X> void f1(X<0>);
+
+template<int> class X1 { };
+template<int...> class X2 { };
+
+void g1()
+{
+  X1<0> x1;
+  f1<int>(x1); // { dg-bogus "no matching" "" { xfail *-*-* } 10 }
+
+  X2<0> x2;
+  f1<int>(x2); // { dg-error "no matching" }
+}
+
+template<typename... T, template<T> class X> void f2(X<0>); // { dg-error "not expanded|T" }
+
+void g2()
+{
+  X1<0> x1;
+  f2<int>(x1); // { dg-bogus "no matching"  "" { xfail *-*-* } 21 }
+
+  X2<0> x2;
+  f2<int>(x2); // { dg-error "no matching" }
+}
+
+template<typename... T, template<T...> class X> void f3(X<0>);
+
+void g3()
+{
+  X1<0> x1;
+  f3<int>(x1); // { dg-bogus "no matching"  "" { xfail *-*-* } 32 }
+
+  X2<0> x2;
+  f3<int>(x2); // { dg-error "no matching" }
+}
+
+
+template<int N> struct Int2Type { };
+
+template<typename... T>
+struct Outer {
+  template<typename... U>
+  void foo(Int2Type<sizeof...(T)>, Int2Type<sizeof...(U)>);
+};
+
+
+Outer<short, int, long> outer;
+
+void g4() {
+  outer.foo<float, double>(Int2Type<3>(), Int2Type<2>());
+}

Re: [C++ PATCH] Nested non-type template parameter packs using outer template parameter packs (PR c++/35022)

[Jason Merrill]

OK.

Jason

Re: [C++ PATCH] Nested non-type template parameter packs using outer template parameter packs (PR c++/35022)

[Doug Gregor]

[-- Attachment #1: Type: text/plain, Size: 1337 bytes --]

Unfortunately, the part of this patch that fixed PR 35022 collided
with the implementation of N2555 (extending variadic template template
parameters). On further inspection, that part of this patch (which
concerns coerce_template_parameter_pack) was fixing the symptom and
not the cause of 35022.

Since this patch actually fixed three separate problems, I've
committed the fixes for sizeof... and parsing of unnamed non-type
template parameter packs to mainline along with the related test
cases. I'll take another shot at fixing 35022 separately. The
committed patch is below; it's just a subset of the already-approved
patch, which removes the changes to coerce_template_parameter_pack.

  - Doug

2008-03-27  Douglas Gregor  <doug.gregor@gmail.com>

	* pt.c (tsubst_copy) <case SIZEOF_EXPR>: Cope with
	tsubst_pack_expansion returning a pack expansion, or a TREE_VEC
	ending in a pack expansion, both of which can occur when
	substituting into a nested template.
	(tsubst_copy_and_build) <case SIZEOF_EXPR>: When we're
	instantiating the sizeof...(X) form, make tsubst_copy do the work.
	* parser.c (cp_parser_template_parameter): Deal with unnamed
	non-type template parameter packs identified by pack expansions in
	the parameter type.

2008-03-27  Douglas Gregor  <doug.gregor@gmail.com>

       * g++.dg/cpp0x/variadic91.C: New.

[-- Attachment #2: vt-fixes.patch --]
[-- Type: application/octet-stream, Size: 5153 bytes --]

Index: cp/pt.c
===================================================================
--- cp/pt.c	(revision 133641)
+++ cp/pt.c	(working copy)
@@ -9916,9 +9916,30 @@ tsubst_copy (tree t, tree args, tsubst_f
           /* We only want to compute the number of arguments.  */
           tree expanded = tsubst_pack_expansion (TREE_OPERAND (t, 0), args,
                                                 complain, in_decl);
+	  int len;
+
+	  if (TREE_CODE (expanded) == TREE_VEC)
+	    len = TREE_VEC_LENGTH (expanded);
+
 	  if (expanded == error_mark_node)
 	    return error_mark_node;
-          return build_int_cst (size_type_node, TREE_VEC_LENGTH (expanded));
+	  else if (PACK_EXPANSION_P (expanded)
+		   || (TREE_CODE (expanded) == TREE_VEC
+		       && len > 0
+		       && PACK_EXPANSION_P (TREE_VEC_ELT (expanded, len-1))))
+	    {
+	      if (TREE_CODE (expanded) == TREE_VEC)
+		expanded = TREE_VEC_ELT (expanded, len - 1);
+
+	      if (TYPE_P (expanded))
+		return cxx_sizeof_or_alignof_type (expanded, SIZEOF_EXPR, 
+						   complain & tf_error);
+	      else
+		return cxx_sizeof_or_alignof_expr (expanded, SIZEOF_EXPR,
+                                                   complain & tf_error);
+	    }
+	  else
+	    return build_int_cst (size_type_node, len);
         }
       /* Fall through */
 
@@ -10918,14 +10939,7 @@ tsubst_copy_and_build (tree t,
 
     case SIZEOF_EXPR:
       if (PACK_EXPANSION_P (TREE_OPERAND (t, 0)))
-        {
-          /* We only want to compute the number of arguments.  */
-          tree expanded = tsubst_pack_expansion (TREE_OPERAND (t, 0), args,
-                                                complain, in_decl);
-	  if (expanded == error_mark_node)
-	    return error_mark_node;
-          return build_int_cst (size_type_node, TREE_VEC_LENGTH (expanded));
-        }
+	return tsubst_copy (t, args, complain, in_decl);
       /* Fall through */
       
     case ALIGNOF_EXPR:
Index: cp/parser.c
===================================================================
--- cp/parser.c	(revision 133641)
+++ cp/parser.c	(working copy)
@@ -9435,29 +9435,41 @@ cp_parser_template_parameter (cp_parser*
       maybe_warn_variadic_templates ();
       
       *is_parameter_pack = true;
+    }
+  /* We might end up with a pack expansion as the type of the non-type
+     template parameter, in which case this is a non-type template
+     parameter pack.  */
+  else if (parameter_declarator
+	   && parameter_declarator->decl_specifiers.type
+	   && PACK_EXPANSION_P (parameter_declarator->decl_specifiers.type))
+    {
+      *is_parameter_pack = true;
+      parameter_declarator->decl_specifiers.type = 
+	PACK_EXPANSION_PATTERN (parameter_declarator->decl_specifiers.type);
+    }
 
+  if (*is_parameter_pack && cp_lexer_next_token_is (parser->lexer, CPP_EQ))
+    {
       /* Parameter packs cannot have default arguments.  However, a
 	 user may try to do so, so we'll parse them and give an
 	 appropriate diagnostic here.  */
-      if (cp_lexer_next_token_is (parser->lexer, CPP_EQ))
-	{
-	  /* Consume the `='.  */
-	  cp_lexer_consume_token (parser->lexer);
 
-	  /* Find the name of the parameter pack.  */     
-	  id_declarator = parameter_declarator->declarator;
-	  while (id_declarator && id_declarator->kind != cdk_id)
-	    id_declarator = id_declarator->declarator;
-	  
-	  if (id_declarator && id_declarator->kind == cdk_id)
-	    error ("template parameter pack %qD cannot have a default argument",
-		   id_declarator->u.id.unqualified_name);
-	  else
-	    error ("template parameter pack cannot have a default argument");
-
-          /* Parse the default argument, but throw away the result.  */
-          cp_parser_default_argument (parser, /*template_parm_p=*/true);
-	}
+      /* Consume the `='.  */
+      cp_lexer_consume_token (parser->lexer);
+      
+      /* Find the name of the parameter pack.  */     
+      id_declarator = parameter_declarator->declarator;
+      while (id_declarator && id_declarator->kind != cdk_id)
+	id_declarator = id_declarator->declarator;
+      
+      if (id_declarator && id_declarator->kind == cdk_id)
+	error ("template parameter pack %qD cannot have a default argument",
+	       id_declarator->u.id.unqualified_name);
+      else
+	error ("template parameter pack cannot have a default argument");
+      
+      /* Parse the default argument, but throw away the result.  */
+      cp_parser_default_argument (parser, /*template_parm_p=*/true);
     }
 
   parm = grokdeclarator (parameter_declarator->declarator,
Index: testsuite/g++.dg/cpp0x/variadic91.C
===================================================================
--- testsuite/g++.dg/cpp0x/variadic91.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/variadic91.C	(revision 0)
@@ -0,0 +1,17 @@
+// { dg-options "-std=c++0x" }
+template<int N> struct Int2Type { };
+
+template<typename... T>
+struct Outer {
+  template<typename... U>
+  void foo(Int2Type<sizeof...(T)>, Int2Type<sizeof...(U)>);
+};
+
+
+Outer<short, int, long> outer;
+
+void g4() {
+  outer.foo<float, double>(Int2Type<3>(), Int2Type<2>());
+}
+
+template<typename... T, template<T...> class X> void f1();