[PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

Hello,

Consider this example:

     1	template<class...I> struct List {};
     2	template<int T> struct Z {static const int value = T;};
     3	template<int...T> using LZ = List<Z<T>...>;
     4
     5	template<class...U>
     6	struct F
     7	{
     8	  using N = LZ<U::value...>; //#1 This should amount to List<Z<U::value>...>
     9	}
    10
    11	F<Z<1>, Z<2> >::N A; //#2

which G++ fails to compile, with this error message:

test-PR53609-3.cc: In instantiation of 'struct F<Z<1>, Z<2> >':
test-PR53609-3.cc:11:15:   required from here
test-PR53609-3.cc:3:43: error: wrong number of template arguments (2, should be 1)
 template<int...T> using LZ = List<Z<T>...>;
                                           ^
test-PR53609-3.cc:2:24: error: provided for 'template<int T> struct Z'
 template<int T> struct Z {static const int value = T;};

I think this is because in #1, when we substitute the argument pack
{U::value...} into the pack expansion Z<T>..., tsubst_pack_expansion
yields Z<U::value...>, instead of Z<U::value>..., so the instantiation
of LZ amounts to List<Z<U::value...> >, instead of
List<Z<U::value>...>.

The idea of this patch is to make tsubst_pack_expansion support
substituting an argument pack (into a pack expansion) where one of the
arguments (let's call it the Ith argument) is itself a pack expansion
P.  In that case, the Ith element resulting from the substituting
should be a pack expansion P'.

The pattern of P' is then the pattern of P into which the pattern of
the Ith argument of the argument pack has been substituted.

Tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* pt.c (real_argument_pack_element_p)
	(any_non_real_argument_pack_element_p)
	(arg_pack_select_for_pack_expansion)
	(set_arg_pack_select_index_for_pack_expansion): New static
	functions.
	(has_bare_parameter_packs): Factorized out of ...
	(check_for_bare_parameter_packs): ... here.
	(tsubst_pack_expansion): Support substituting an argument pack
	that contains a pack expansion.

gcc/testsuite/

	* g++.dg/cpp0x/variadic139.C: New test.
	* g++.dg/cpp0x/variadic140.C: Likewise.
	* g++.dg/cpp0x/variadic141.C: Likewise.
---
 gcc/cp/pt.c                              |  151 ++++++++++++++++++++++++------
 gcc/testsuite/g++.dg/cpp0x/variadic139.C |   14 +++
 gcc/testsuite/g++.dg/cpp0x/variadic140.C |   22 +++++
 gcc/testsuite/g++.dg/cpp0x/variadic141.C |   22 +++++
 4 files changed, 182 insertions(+), 27 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 16952bf..bcfe83f 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3310,6 +3310,29 @@ make_pack_expansion (tree arg)
   return result;
 }
 
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+  tree parameter_packs = NULL_TREE;
+  struct find_parameter_pack_data ppd;
+
+  if (!processing_template_decl || !t || t == error_mark_node)
+    return NULL_TREE;
+
+  if (TREE_CODE (t) == TYPE_DECL)
+    t = TREE_TYPE (t);
+
+  ppd.parameter_packs = &parameter_packs;
+  ppd.visited = pointer_set_create ();
+  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
+  pointer_set_destroy (ppd.visited);
+
+  return parameter_packs;
+}
+
 /* Checks T for any "bare" parameter packs, which have not yet been
    expanded, and issues an error if any are found. This operation can
    only be done on full expressions or types (e.g., an expression
@@ -3327,19 +3350,7 @@ make_pack_expansion (tree arg)
 bool 
 check_for_bare_parameter_packs (tree t)
 {
-  tree parameter_packs = NULL_TREE;
-  struct find_parameter_pack_data ppd;
-
-  if (!processing_template_decl || !t || t == error_mark_node)
-    return false;
-
-  if (TREE_CODE (t) == TYPE_DECL)
-    t = TREE_TYPE (t);
-
-  ppd.parameter_packs = &parameter_packs;
-  ppd.visited = pointer_set_create ();
-  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
-  pointer_set_destroy (ppd.visited);
+  tree parameter_packs = has_bare_parameter_packs (t);
 
   if (parameter_packs) 
     {
@@ -9065,6 +9076,86 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is
+   *NOT* a pack expansion.  */
+
+static bool
+real_argument_pack_element_p (tree arg_pack, int i)
+{
+  return !PACK_EXPANSION_P (TREE_VEC_ELT
+			    (ARGUMENT_PACK_ARGS (arg_pack), i));
+}
+
+/* Return true iff ARG_PACK is an argument pack that contains a pack
+   expansion.  */
+
+static bool
+any_non_real_argument_pack_element_p (tree arg_pack)
+{
+  if (arg_pack == NULL_TREE
+      || arg_pack == error_mark_node
+      || !ARGUMENT_PACK_P (arg_pack))
+    return false;
+
+  for (int i = 0; i < TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)); ++i)
+    if (!real_argument_pack_element_p (arg_pack, i))
+      return true;
+  return false;
+}
+
+/* Creates an ARGUMENT_PACK_SELECT tree node, for the purpose of
+   substituting an argument pack into a pack expansion.  This is a
+   subroutine of tsubst_pack_expansion.   */
+
+static tree
+arg_pack_select_for_pack_expansion (tree arg_pack)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  if (!any_non_real_argument_pack_element_p (arg_pack))
+    ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  else
+    {
+      tree tmp_arg_pack = TYPE_P (arg_pack)
+	? cxx_make_type (TREE_CODE (arg_pack))
+	: make_node (TREE_CODE (arg_pack));
+      tree tmp_vec =
+	make_tree_vec (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)));
+      SET_ARGUMENT_PACK_ARGS (tmp_arg_pack, tmp_vec);
+      ARGUMENT_PACK_SELECT_FROM_PACK (aps) = tmp_arg_pack;
+    }
+
+   return aps;
+}
+
+/* Setup APS, which is an instance of an ARGUMENT_PACK_SELECT tree, so
+   that it selects the Ith argument out of the argument pack
+   ARG_PACK.  If the Ith argument is a pack expansion, then just
+   select its pattern.  Otherwise, select the whole argument.  This
+   is a subroutine of tsubst_pack_expansion.  */
+
+static void
+set_arg_pack_select_index_for_pack_expansion (tree aps,
+					      int i,
+					      tree arg_pack)
+{
+  if (any_non_real_argument_pack_element_p (arg_pack))
+    {
+      tree args_vec =
+	ARGUMENT_PACK_ARGS (ARGUMENT_PACK_SELECT_FROM_PACK (aps));
+      if (real_argument_pack_element_p (arg_pack, i))
+	TREE_VEC_ELT (args_vec, i) =
+	  TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), i);
+      else
+	TREE_VEC_ELT (args_vec, i) =
+	  PACK_EXPANSION_PATTERN (TREE_VEC_ELT
+				  (ARGUMENT_PACK_ARGS (arg_pack),
+				   i));
+    }
+
+  ARGUMENT_PACK_SELECT_INDEX (aps) = i;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9284,20 +9375,21 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       for (pack = packs; pack; pack = TREE_CHAIN (pack))
         {
           tree parm = TREE_PURPOSE (pack);
-	  tree arg;
+	  tree arg_pack = TREE_VALUE (pack);
+	  tree aps; 		/* instance of ARGUMENT_PACK_SELECT
+				   tree.  */
 
 	  /* Select the Ith argument from the pack.  */
           if (TREE_CODE (parm) == PARM_DECL)
             {
 	      if (i == 0)
 		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
+		  aps = arg_pack_select_for_pack_expansion (arg_pack);
 		  mark_used (parm);
-		  register_local_specialization (arg, parm);
+		  register_local_specialization (aps, parm);
 		}
 	      else
-		arg = retrieve_local_specialization (parm);
+		aps = retrieve_local_specialization (parm);
             }
           else
             {
@@ -9306,25 +9398,30 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 
 	      if (i == 0)
 		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
+		  aps = arg_pack_select_for_pack_expansion (arg_pack);
 		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
+		  TMPL_ARG (args, level, idx) = aps;
 		}
 	      else
 		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
+		aps = TMPL_ARG (args, level, idx);
             }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
+	  set_arg_pack_select_index_for_pack_expansion (aps, i,
+							arg_pack);
         }
 
       /* Substitute into the PATTERN with the altered arguments.  */
       if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
+	t = tsubst_expr (pattern, args, complain, in_decl,
+			 /*integral_constant_expression_p=*/false);
       else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
+	t = tsubst (pattern, args, complain, in_decl);
+
+      /*  If the Ith argument pack element is a pack expansion, then
+	  the Ith element resulting from the substituting is going to
+	  be a pack expansion as well.  */
+      TREE_VEC_ELT (result, i) =
+	(has_bare_parameter_packs (t)) ? make_pack_expansion (t) : t;
 
       if (TREE_VEC_ELT (result, i) == error_mark_node)
 	{
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..89f7b32
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};                           
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
-- 
		Dodji

[PING] [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

I am friendly pinging the patch below ...


Dodji Seketeli <dodji@redhat.com> a écrit:

> Hello,
>
> Consider this example:
>
>      1	template<class...I> struct List {};
>      2	template<int T> struct Z {static const int value = T;};
>      3	template<int...T> using LZ = List<Z<T>...>;
>      4
>      5	template<class...U>
>      6	struct F
>      7	{
>      8	  using N = LZ<U::value...>; //#1 This should amount to List<Z<U::value>...>
>      9	}
>     10
>     11	F<Z<1>, Z<2> >::N A; //#2
>
> which G++ fails to compile, with this error message:
>
> test-PR53609-3.cc: In instantiation of 'struct F<Z<1>, Z<2> >':
> test-PR53609-3.cc:11:15:   required from here
> test-PR53609-3.cc:3:43: error: wrong number of template arguments (2, should be 1)
>  template<int...T> using LZ = List<Z<T>...>;
>                                            ^
> test-PR53609-3.cc:2:24: error: provided for 'template<int T> struct Z'
>  template<int T> struct Z {static const int value = T;};
>
> I think this is because in #1, when we substitute the argument pack
> {U::value...} into the pack expansion Z<T>..., tsubst_pack_expansion
> yields Z<U::value...>, instead of Z<U::value>..., so the instantiation
> of LZ amounts to List<Z<U::value...> >, instead of
> List<Z<U::value>...>.
>
> The idea of this patch is to make tsubst_pack_expansion support
> substituting an argument pack (into a pack expansion) where one of the
> arguments (let's call it the Ith argument) is itself a pack expansion
> P.  In that case, the Ith element resulting from the substituting
> should be a pack expansion P'.
>
> The pattern of P' is then the pattern of P into which the pattern of
> the Ith argument of the argument pack has been substituted.
>
> Tested on x86_64-unknown-linux-gnu against trunk.
>
> gcc/cp/
>
> 	* pt.c (real_argument_pack_element_p)
> 	(any_non_real_argument_pack_element_p)
> 	(arg_pack_select_for_pack_expansion)
> 	(set_arg_pack_select_index_for_pack_expansion): New static
> 	functions.
> 	(has_bare_parameter_packs): Factorized out of ...
> 	(check_for_bare_parameter_packs): ... here.
> 	(tsubst_pack_expansion): Support substituting an argument pack
> 	that contains a pack expansion.
>
> gcc/testsuite/
>
> 	* g++.dg/cpp0x/variadic139.C: New test.
> 	* g++.dg/cpp0x/variadic140.C: Likewise.
> 	* g++.dg/cpp0x/variadic141.C: Likewise.
> ---
>  gcc/cp/pt.c                              |  151 ++++++++++++++++++++++++------
>  gcc/testsuite/g++.dg/cpp0x/variadic139.C |   14 +++
>  gcc/testsuite/g++.dg/cpp0x/variadic140.C |   22 +++++
>  gcc/testsuite/g++.dg/cpp0x/variadic141.C |   22 +++++
>  4 files changed, 182 insertions(+), 27 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C
>
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 16952bf..bcfe83f 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -3310,6 +3310,29 @@ make_pack_expansion (tree arg)
>    return result;
>  }
>  
> +/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
> +   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
> +
> +static tree
> +has_bare_parameter_packs (tree t)
> +{
> +  tree parameter_packs = NULL_TREE;
> +  struct find_parameter_pack_data ppd;
> +
> +  if (!processing_template_decl || !t || t == error_mark_node)
> +    return NULL_TREE;
> +
> +  if (TREE_CODE (t) == TYPE_DECL)
> +    t = TREE_TYPE (t);
> +
> +  ppd.parameter_packs = &parameter_packs;
> +  ppd.visited = pointer_set_create ();
> +  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
> +  pointer_set_destroy (ppd.visited);
> +
> +  return parameter_packs;
> +}
> +
>  /* Checks T for any "bare" parameter packs, which have not yet been
>     expanded, and issues an error if any are found. This operation can
>     only be done on full expressions or types (e.g., an expression
> @@ -3327,19 +3350,7 @@ make_pack_expansion (tree arg)
>  bool 
>  check_for_bare_parameter_packs (tree t)
>  {
> -  tree parameter_packs = NULL_TREE;
> -  struct find_parameter_pack_data ppd;
> -
> -  if (!processing_template_decl || !t || t == error_mark_node)
> -    return false;
> -
> -  if (TREE_CODE (t) == TYPE_DECL)
> -    t = TREE_TYPE (t);
> -
> -  ppd.parameter_packs = &parameter_packs;
> -  ppd.visited = pointer_set_create ();
> -  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
> -  pointer_set_destroy (ppd.visited);
> +  tree parameter_packs = has_bare_parameter_packs (t);
>  
>    if (parameter_packs) 
>      {
> @@ -9065,6 +9076,86 @@ make_fnparm_pack (tree spec_parm)
>    return extract_fnparm_pack (NULL_TREE, &spec_parm);
>  }
>  
> +/* Return true iff the Ith element of the argument pack ARG_PACK is
> +   *NOT* a pack expansion.  */
> +
> +static bool
> +real_argument_pack_element_p (tree arg_pack, int i)
> +{
> +  return !PACK_EXPANSION_P (TREE_VEC_ELT
> +			    (ARGUMENT_PACK_ARGS (arg_pack), i));
> +}
> +
> +/* Return true iff ARG_PACK is an argument pack that contains a pack
> +   expansion.  */
> +
> +static bool
> +any_non_real_argument_pack_element_p (tree arg_pack)
> +{
> +  if (arg_pack == NULL_TREE
> +      || arg_pack == error_mark_node
> +      || !ARGUMENT_PACK_P (arg_pack))
> +    return false;
> +
> +  for (int i = 0; i < TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)); ++i)
> +    if (!real_argument_pack_element_p (arg_pack, i))
> +      return true;
> +  return false;
> +}
> +
> +/* Creates an ARGUMENT_PACK_SELECT tree node, for the purpose of
> +   substituting an argument pack into a pack expansion.  This is a
> +   subroutine of tsubst_pack_expansion.   */
> +
> +static tree
> +arg_pack_select_for_pack_expansion (tree arg_pack)
> +{
> +  tree aps = make_node (ARGUMENT_PACK_SELECT);
> +
> +  if (!any_non_real_argument_pack_element_p (arg_pack))
> +    ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
> +  else
> +    {
> +      tree tmp_arg_pack = TYPE_P (arg_pack)
> +	? cxx_make_type (TREE_CODE (arg_pack))
> +	: make_node (TREE_CODE (arg_pack));
> +      tree tmp_vec =
> +	make_tree_vec (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)));
> +      SET_ARGUMENT_PACK_ARGS (tmp_arg_pack, tmp_vec);
> +      ARGUMENT_PACK_SELECT_FROM_PACK (aps) = tmp_arg_pack;
> +    }
> +
> +   return aps;
> +}
> +
> +/* Setup APS, which is an instance of an ARGUMENT_PACK_SELECT tree, so
> +   that it selects the Ith argument out of the argument pack
> +   ARG_PACK.  If the Ith argument is a pack expansion, then just
> +   select its pattern.  Otherwise, select the whole argument.  This
> +   is a subroutine of tsubst_pack_expansion.  */
> +
> +static void
> +set_arg_pack_select_index_for_pack_expansion (tree aps,
> +					      int i,
> +					      tree arg_pack)
> +{
> +  if (any_non_real_argument_pack_element_p (arg_pack))
> +    {
> +      tree args_vec =
> +	ARGUMENT_PACK_ARGS (ARGUMENT_PACK_SELECT_FROM_PACK (aps));
> +      if (real_argument_pack_element_p (arg_pack, i))
> +	TREE_VEC_ELT (args_vec, i) =
> +	  TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), i);
> +      else
> +	TREE_VEC_ELT (args_vec, i) =
> +	  PACK_EXPANSION_PATTERN (TREE_VEC_ELT
> +				  (ARGUMENT_PACK_ARGS (arg_pack),
> +				   i));
> +    }
> +
> +  ARGUMENT_PACK_SELECT_INDEX (aps) = i;
> +}
> +
>  /* Substitute ARGS into T, which is an pack expansion
>     (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
>     TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
> @@ -9284,20 +9375,21 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
>        for (pack = packs; pack; pack = TREE_CHAIN (pack))
>          {
>            tree parm = TREE_PURPOSE (pack);
> -	  tree arg;
> +	  tree arg_pack = TREE_VALUE (pack);
> +	  tree aps; 		/* instance of ARGUMENT_PACK_SELECT
> +				   tree.  */
>  
>  	  /* Select the Ith argument from the pack.  */
>            if (TREE_CODE (parm) == PARM_DECL)
>              {
>  	      if (i == 0)
>  		{
> -		  arg = make_node (ARGUMENT_PACK_SELECT);
> -		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
> +		  aps = arg_pack_select_for_pack_expansion (arg_pack);
>  		  mark_used (parm);
> -		  register_local_specialization (arg, parm);
> +		  register_local_specialization (aps, parm);
>  		}
>  	      else
> -		arg = retrieve_local_specialization (parm);
> +		aps = retrieve_local_specialization (parm);
>              }
>            else
>              {
> @@ -9306,25 +9398,30 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
>  
>  	      if (i == 0)
>  		{
> -		  arg = make_node (ARGUMENT_PACK_SELECT);
> -		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
> +		  aps = arg_pack_select_for_pack_expansion (arg_pack);
>  		  /* Update the corresponding argument.  */
> -		  TMPL_ARG (args, level, idx) = arg;
> +		  TMPL_ARG (args, level, idx) = aps;
>  		}
>  	      else
>  		/* Re-use the ARGUMENT_PACK_SELECT.  */
> -		arg = TMPL_ARG (args, level, idx);
> +		aps = TMPL_ARG (args, level, idx);
>              }
> -	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
> +	  set_arg_pack_select_index_for_pack_expansion (aps, i,
> +							arg_pack);
>          }
>  
>        /* Substitute into the PATTERN with the altered arguments.  */
>        if (!TYPE_P (pattern))
> -        TREE_VEC_ELT (result, i) = 
> -          tsubst_expr (pattern, args, complain, in_decl,
> -                       /*integral_constant_expression_p=*/false);
> +	t = tsubst_expr (pattern, args, complain, in_decl,
> +			 /*integral_constant_expression_p=*/false);
>        else
> -        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
> +	t = tsubst (pattern, args, complain, in_decl);
> +
> +      /*  If the Ith argument pack element is a pack expansion, then
> +	  the Ith element resulting from the substituting is going to
> +	  be a pack expansion as well.  */
> +      TREE_VEC_ELT (result, i) =
> +	(has_bare_parameter_packs (t)) ? make_pack_expansion (t) : t;
>  
>        if (TREE_VEC_ELT (result, i) == error_mark_node)
>  	{
> diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
> new file mode 100644
> index 0000000..89f7b32
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
> @@ -0,0 +1,14 @@
> +// Origin: PR c++/53609
> +// { dg-do compile { target c++11 } }
> +
> +template<class...I> struct List {};
> +template<int T> struct Z {static const int value = T;};
> +template<int...T> using LZ = List<Z<T>...>;
> +
> +template<class...U>
> +struct F
> +{
> +  using N = LZ<U::value...>;
> +};                           
> +
> +F<Z<1>, Z<2> >::N A;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
> new file mode 100644
> index 0000000..17ca9e5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
> @@ -0,0 +1,22 @@
> +// Origin: PR c++/53609
> +// { dg-do compile { target c++11 } }
> +
> +template<class...I> struct List{ static const bool is_ok = false;};
> +template<int T> struct Z
> +{
> +  static const int value = T;
> +  static const int value_square = T * T;
> +};
> +
> +template<template<int> class U>
> +struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
> +
> +template<int...T> using LZ = List<Z<T>...>;
> +
> +template<class...T>
> +struct F
> +{
> +  using N = LZ<T::value..., T::value_square...>;
> +};
> +
> +static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
> diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
> new file mode 100644
> index 0000000..6b893a7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
> @@ -0,0 +1,22 @@
> +// Origin: PR c++/53609
> +// { dg-do compile { target c++11 } }
> +
> +template<class...I> struct List{ static const bool is_ok = false;};
> +template<int T> struct Z
> +{
> +  static const int value = T;
> +  static const int value_square = T * T;
> +};
> +
> +template<template<int> class U>
> +struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
> +
> +template<int...T> using LZ = List<Z<T>...>;
> +
> +template<class...T>
> +struct F
> +{
> +  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
> +};
> +
> +static_assert (F<Z<2>, Z<3>>::N::is_ok, "");

-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

It seems like your new code is a generalization of the old code for 
handling substitution of a pack for itself (arg_from_parm_pack and such) 
and the code for handling other packs with a single pack expansion 
argument, and should replace those rather than adding on.

The solution that if at a certain index all the packs have expansion 
arguments then the substitution produces a pack expansion seems right to 
me, but if one pack has an expansion and another pack has a normal 
argument, we can't do the substitution and need to fall back on the 
PACK_EXPANSION_EXTRA_ARGS mechanism.

> +set_arg_pack_select_index_for_pack_expansion (tree aps,
> +					      int i,
> +					      tree arg_pack)
> +{
> +  if (any_non_real_argument_pack_element_p (arg_pack))

I don't think we care if *any* element is an expansion (and please talk 
about expansions rather than "non-real elements").  What we care about 
is whether the i'th element is an expansion.  And we need to compare all 
the pack elements, so I think this needs to be handled in the main 
function rather than encapsulated here.

> +	TREE_VEC_ELT (args_vec, i) =
> +	  TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), i);

Aren't the LHS and RHS the same location here?

Jason

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> It seems like your new code is a generalization of the old code for
> handling substitution of a pack for itself (arg_from_parm_pack and
> such) and the code for handling other packs with a single pack
> expansion argument, and should replace those rather than adding on.

I guess that can of worms is now open.  :-)

I tried to think about how to do this "properly".  What I came up with
in the patch below is to properly organize tsubst_pack_expansion in
two parts that are already screaming to emerge, IMHO.  The first part
maps each parameter pack with its matching argument pack, and does
only that.  The second part walks that map and builds the list of Ei
where each Ei is the result of substituting elements at index I in the
argument packs into the pattern of the expansion passed to
tsubst_pack_expansion.

It's only the latter part knows how to deal with special cases like:
  - some parameter packs having no matching argument pack
  - parameter packs having matching argument packs element that are
  expansions
  - etc

Is that what you had in mind?

> The solution that if at a certain index all the packs have expansion
> arguments then the substitution produces a pack expansion seems right
> to me, but if one pack has an expansion and another pack has a normal
> argument, we can't do the substitution and need to fall back on the
> PACK_EXPANSION_EXTRA_ARGS mechanism.

Right, this is hopefully what this updated patch implements.

> > +set_arg_pack_select_index_for_pack_expansion (tree aps,
> > +					      int i,
> > +					      tree arg_pack)
> > +{
> > +  if (any_non_real_argument_pack_element_p (arg_pack))
> 
> I don't think we care if *any* element is an expansion (and please
> talk about expansions rather than "non-real elements").  What we care
> about is whether the i'th element is an expansion.  And we need to
> compare all the pack elements, so I think this needs to be handled in
> the main function rather than encapsulated here.

Done.

> 
> > +	TREE_VEC_ELT (args_vec, i) =
> > +	  TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), i);
> 
> Aren't the LHS and RHS the same location here?

Yes, this is hopefully dropped now.

Bootstrapped and tested against trunk on x86_64-unknown-linux-gnu.

gcc/cp/

	* pt.c (argument_pack_element_is_expansion_p)
	(arg_pack_select_for_pack_expansion)
	(set_arg_pack_select_index_for_pack_expansion, scan_parm_packs)
	(gen_elem_of_pack_expansion_instantiation): New static functions.
	(has_bare_parameter_packs): Factorized out of ...
	(check_for_bare_parameter_packs): ... here.
	(tsubst_pack_expansion): Now this function is clearly organized in
	two parts: a part that maps each parameter pack with its matching
	argument pack, and a part that walks that map to build the result
	of the substituting each element of the argument packs into the
	parameter packs.  Use gen_elem_of_pack_expansion_instantiation for
	the latter part.

gcc/testsuite/

	* g++.dg/cpp0x/variadic139.C: New test.
	* g++.dg/cpp0x/variadic140.C: Likewise.
	* g++.dg/cpp0x/variadic141.C: Likewise.
---
 gcc/cp/pt.c                              | 525 ++++++++++++++++++++++---------
 gcc/testsuite/g++.dg/cpp0x/variadic139.C |  14 +
 gcc/testsuite/g++.dg/cpp0x/variadic140.C |  22 ++
 gcc/testsuite/g++.dg/cpp0x/variadic141.C |  22 ++
 4 files changed, 441 insertions(+), 142 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ecb013e..b8f4294 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3308,6 +3308,29 @@ make_pack_expansion (tree arg)
   return result;
 }
 
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+  tree parameter_packs = NULL_TREE;
+  struct find_parameter_pack_data ppd;
+
+  if (!processing_template_decl || !t || t == error_mark_node)
+    return NULL_TREE;
+
+  if (TREE_CODE (t) == TYPE_DECL)
+    t = TREE_TYPE (t);
+
+  ppd.parameter_packs = &parameter_packs;
+  ppd.visited = pointer_set_create ();
+  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
+  pointer_set_destroy (ppd.visited);
+
+  return parameter_packs;
+}
+
 /* Checks T for any "bare" parameter packs, which have not yet been
    expanded, and issues an error if any are found. This operation can
    only be done on full expressions or types (e.g., an expression
@@ -3325,19 +3348,7 @@ make_pack_expansion (tree arg)
 bool 
 check_for_bare_parameter_packs (tree t)
 {
-  tree parameter_packs = NULL_TREE;
-  struct find_parameter_pack_data ppd;
-
-  if (!processing_template_decl || !t || t == error_mark_node)
-    return false;
-
-  if (TREE_CODE (t) == TYPE_DECL)
-    t = TREE_TYPE (t);
-
-  ppd.parameter_packs = &parameter_packs;
-  ppd.visited = pointer_set_create ();
-  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
-  pointer_set_destroy (ppd.visited);
+  tree parameter_packs = has_bare_parameter_packs (t);
 
   if (parameter_packs) 
     {
@@ -9095,6 +9106,328 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is a
+   pack expansion.  */
+
+static bool
+argument_pack_element_is_expansion_p (tree arg_pack, int i)
+{
+  tree vec = ARGUMENT_PACK_ARGS (arg_pack);
+  if (i >= TREE_VEC_LENGTH (vec))
+    return false;
+  return PACK_EXPANSION_P (TREE_VEC_ELT (vec, i));
+}
+
+
+/* This is a subroutine of tsubst_pack_expansion.
+
+   It creates an ARGUMENT_PACK_SELECT tree node, for the purpose of
+   substituting an argument pack into a pack expansion.  ARG_PACK is
+   an instance of {TYPE,NON_TYPE}_ARGUMENT_PACK.
+   ALL_PACKS_HAVE_EXPANSION_ARGS must be set to true if every single
+   parameter pack in the expansion passed to tsubst_pack_expansion has
+   an argument that is itself an expansion.  */
+
+static tree
+arg_pack_select_for_pack_expansion (tree arg_pack,
+				    bool all_packs_have_expansion_args)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  if (!all_packs_have_expansion_args)
+    ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  else
+    {
+      tree tmp_arg_pack = TYPE_P (arg_pack)
+	? cxx_make_type (TREE_CODE (arg_pack))
+	: make_node (TREE_CODE (arg_pack));
+      tree tmp_vec =
+	make_tree_vec (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)));
+      SET_ARGUMENT_PACK_ARGS (tmp_arg_pack, tmp_vec);
+      ARGUMENT_PACK_SELECT_FROM_PACK (aps) = tmp_arg_pack;
+    }
+
+   return aps;
+}
+
+/* This is a subroutine of tsubst_pack_expansion.
+
+   Setup APS, which is an instance of an ARGUMENT_PACK_SELECT tree, so
+   that it selects the Ith element out of the argument pack ARG_PACK.
+   If the Ith element is a pack expansion, then just select its
+   pattern.  Otherwise, select the whole element.
+   ALL_PACKS_HAVE_EXPANSION_ARGS must be set to true if every single
+   parameter pack in the expansion passed to tsubst_pack_expansion has
+   an argument that is itself an expansion.  */
+
+static void
+set_arg_pack_select_index_for_pack_expansion (tree aps, int i, tree arg_pack,
+					      bool all_packs_have_expansion_args)
+{
+  if (all_packs_have_expansion_args)
+    {
+      tree args_vec =
+	ARGUMENT_PACK_ARGS (ARGUMENT_PACK_SELECT_FROM_PACK (aps));
+      gcc_assert (argument_pack_element_is_expansion_p (arg_pack, i));
+
+      TREE_VEC_ELT (args_vec, i) =
+	PACK_EXPANSION_PATTERN (TREE_VEC_ELT
+				(ARGUMENT_PACK_ARGS (arg_pack),
+				 i));
+    }
+  ARGUMENT_PACK_SELECT_INDEX (aps) = i;
+}
+
+/*  This is a subroutine of gen_elem_of_pack_expansion_instantiation,
+    which is itself a subroutine of tsubst_pack_expansion.
+
+    For a given INDEX of an argument pack, this function scans the
+    list of parameter packs PARM_PACKS and reports some interesting
+    properties about them.  Note that PARM_PACKS must be an instance of
+    TREE_LIST where TREE_PURPOSE is a parameter pack, and TREE_VALUE
+    is the argument for that pack.
+
+    The properties returned about the scanned parameter packs
+    are the following:
+
+	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
+	  pack has got an argument that is an expansion. 
+	- HAS_NON_EXPANSION_ARG_P: Set to TRUE iff at least one
+	  parameter pack has got
+	  an argument that is not an expansion.
+	- HAS_EMPTY_ARG_P: Set to TRUE iff at least one parameter pack has
+	  no argument.
+	- ARG_FROM_PACK_LEVEL_TO_PRUNE: is non-zero iff the argument of
+	  the parameter pack is the result of calling template_parm_to_arg
+	  on the pack.  In that case, this is set to the parameter level
+	  of that pack.  */
+
+static void
+scan_parm_packs (tree parm_packs,
+		 unsigned index,
+		 bool *has_expansion_arg_p,
+		 bool *has_non_expansion_arg_p,
+		 bool *has_empty_arg_p,
+		 int *arg_from_pack_level_to_prune)
+{
+
+  if (parm_packs == NULL_TREE)
+    {
+      if (has_empty_arg_p)
+	*has_empty_arg_p = true;
+      return ;
+    }
+
+  for (tree parm_pack = parm_packs;
+       parm_pack;
+       parm_pack = TREE_CHAIN (parm_pack))
+    {
+      tree arg = TREE_VALUE (parm_pack);
+      tree parm = TREE_PURPOSE (parm_pack);
+
+      if (arg == NULL_TREE)
+	{
+	  if (has_empty_arg_p)
+	    *has_empty_arg_p = true;
+	  continue;
+	}
+
+      if (arg_from_pack_level_to_prune != NULL
+	  && arg_from_parm_pack_p (arg, parm))
+	{
+	  int level = 0, idx = 0;
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    parm = DECL_INITIAL (parm);
+	  template_parm_level_and_index (parm, &level, &idx);
+	  if (*arg_from_pack_level_to_prune == 0
+	      || *arg_from_pack_level_to_prune > level)
+	    *arg_from_pack_level_to_prune = level;
+	  return;
+	}
+
+      if (argument_pack_element_is_expansion_p (arg, index))
+	{
+	  if (has_expansion_arg_p != NULL)
+	    *has_expansion_arg_p = true;
+	}
+      else
+	{ 
+	  if (has_non_expansion_arg_p != NULL)
+	    *has_non_expansion_arg_p = true;
+	}
+    }
+}
+
+/* [temp.variadic]/6 says that:
+
+       The instantiation of a pack expansion [...]
+       produces a list E1,E2, ..., En, where N is the number of elements
+       in the pack expansion parameters.
+
+   This subroutine of tsubst_pack_expansion produces one of these Ei.
+
+   PATTERN is the pattern of the pack expansion.  PARM_PACKS is a
+   TREE_LIST in which each TREE_PURPOSE is a parameter pack of
+   PATTERN, and each TREE_VALUE is its corresponding argument pack.
+   INDEX is the index 'i' of the element Ei to produce.  ARGS,
+   COMPLAIN, and IN_DECL are the same parameters as for the
+   tsubst_pack_expansion function.
+
+   The function returns the resulting Ei upon successful completion,
+   or error_mark_node.  If the *INSTANTIATION_YIELDS_NO_LIST_P is set
+   to true upon completion, that means the result of the function is
+   not an element of a list, but rather the sole result of the pack
+   expansion substituting that has to be returned as the result of the
+   tsubst_pack_expansion function.
+
+   Note that this function possibly modifies the ARGS parameter, so
+   it's the responsibility of the caller to restaure it.  */
+
+static tree
+gen_elem_of_pack_expansion_instantiation (tree pattern,
+					  tree parm_packs,
+					  unsigned index,
+					  bool *instantiation_yields_no_list_p,
+					  tree args /* This parm
+						       gets modified.  */,
+					  tsubst_flags_t complain,
+					  tree in_decl)
+{
+  /* The boolean below is TRUE if at least one parameter pack doesn't
+     yet have all argument packs to really perform the
+     susbtituting.  */
+  bool use_pack_expansion_extra = false,
+    has_expansion_arg = false,
+    has_non_expansion_arg = false,
+    has_empty_arg = false,
+    /* this is true if, at index I of the argument pack,, all parm
+       packs have a pack expansion argument.  */
+    all_packs_have_expansion_args = false;
+
+  int arg_from_pack_level_to_prune = 0;
+  tree t;
+
+  scan_parm_packs (parm_packs, index,
+		   &has_expansion_arg,
+		   &has_non_expansion_arg,
+		   &has_empty_arg,
+		   &arg_from_pack_level_to_prune);
+
+  if ((has_expansion_arg && has_non_expansion_arg)
+      || (has_empty_arg && (has_expansion_arg || has_non_expansion_arg)))
+    use_pack_expansion_extra = true;
+
+  all_packs_have_expansion_args =
+    has_expansion_arg && !has_non_expansion_arg;
+
+  if (use_pack_expansion_extra)
+    {
+      /* We got some full packs, but we can't substitute them in until we
+	 have values for all the packs.  So remember these until then.  */
+
+      int levels = TMPL_ARGS_DEPTH (args);
+      tree save_args;
+
+      /* The call to add_to_template_args in tsubst_pack_expansion
+	 assumes no overlap between saved args and new args, so prune
+	 away any fake args, i.e. those that satisfied
+	 arg_from_parm_pack_p in scan_parm_packs above.  */
+      if (arg_from_pack_level_to_prune != 0
+	  && levels >= arg_from_pack_level_to_prune)
+	{
+	  gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
+		      && arg_from_pack_level_to_prune > 1);
+	  TREE_VEC_LENGTH (args) = arg_from_pack_level_to_prune - 1;
+	  save_args = copy_node (args);
+	  TREE_VEC_LENGTH (args) = levels;
+	}
+      else
+	save_args = args;
+
+      t = make_pack_expansion (pattern);
+      PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
+
+      *instantiation_yields_no_list_p = true;
+
+      return t;
+    }
+  /* If we have one parameter pack whose matching argument pack is
+     just what template_parm_to_arg returned when passed the
+     parameter pack, or if we only have empty arguments ... */
+  else if (arg_from_pack_level_to_prune || has_empty_arg)
+    {
+      /* ... we just return a pack expansion which pattern is PATTERN
+	 into which ARGS has been substituted.  */
+      *instantiation_yields_no_list_p = true;
+    }
+  else
+    {
+      /* For each parameter pack, change the substitution of the parameter
+	 pack to the ith argument in its argument pack, then expand the
+	 pattern.  */
+      for (tree pack = parm_packs; pack; pack = TREE_CHAIN (pack))
+	{
+	  tree parm = TREE_PURPOSE (pack);
+	  tree arg_pack = TREE_VALUE (pack);
+	  tree aps;			/* instance of ARGUMENT_PACK_SELECT
+					   tree.  */
+
+	  /* Select the Ith argument from the pack.  */
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    {
+	      if (index == 0)
+		{
+		  aps =
+		    arg_pack_select_for_pack_expansion
+		    (arg_pack, all_packs_have_expansion_args);
+
+		  mark_used (parm);
+		  register_local_specialization (aps, parm);
+		}
+	      else
+		aps = retrieve_local_specialization (parm);
+	    }
+	  else
+	    {
+	      int idx, level;
+	      template_parm_level_and_index (parm, &level, &idx);
+
+	      if (index == 0)
+		{
+		  aps =
+		    arg_pack_select_for_pack_expansion
+		    (arg_pack, all_packs_have_expansion_args);
+		  /* Update the corresponding argument.  */
+		  TMPL_ARG (args, level, idx) = aps;
+		}
+	      else
+		/* Re-use the ARGUMENT_PACK_SELECT.  */
+		aps = TMPL_ARG (args, level, idx);
+	    }
+	  set_arg_pack_select_index_for_pack_expansion
+	    (aps, index, arg_pack, all_packs_have_expansion_args);
+	}
+
+      *instantiation_yields_no_list_p = false;
+    }
+
+  /* Substitute into the PATTERN with the (possibly altered)
+     arguments.  */
+  if (!TYPE_P (pattern))
+    t = tsubst_expr (pattern, args, complain, in_decl,
+		     /*integral_constant_expression_p=*/false);
+  else
+    t = tsubst (pattern, args, complain, in_decl);
+
+  /*  If the Ith argument pack element is a pack expansion, then
+      the Ith element resulting from the substituting is going to
+      be a pack expansion as well.  */
+  if (has_bare_parameter_packs (t))
+    t = make_pack_expansion (t);
+
+  return t;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9106,9 +9439,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 {
   tree pattern;
   tree pack, packs = NULL_TREE;
-  bool unsubstituted_packs = false;
-  bool real_packs = false;
-  int missing_level = 0;
   int i, len = -1;
   tree result;
   struct pointer_map_t *saved_local_specializations = NULL;
@@ -9187,14 +9517,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	  return result;
 	}
 
-      if (arg_from_parm_pack_p (arg_pack, parm_pack))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would find
-	   in the implicit typedef of a class inside the class itself.
-	   Consider this parameter "unsubstituted", so that we will
-	   maintain the outer pack expansion.  */
-	arg_pack = NULL_TREE;
-          
       if (arg_pack)
         {
           int my_len = 
@@ -9221,77 +9543,19 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
                        pattern);
               return error_mark_node;
             }
-
-	  if (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-	      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
-						 0)))
-	    /* This isn't a real argument pack yet.  */;
-	  else
-	    real_packs = true;
-
-          /* Keep track of the parameter packs and their corresponding
-             argument packs.  */
-          packs = tree_cons (parm_pack, arg_pack, packs);
-          TREE_TYPE (packs) = orig_arg;
         }
-      else
-	{
-	  /* We can't substitute for this parameter pack.  We use a flag as
-	     well as the missing_level counter because function parameter
-	     packs don't have a level.  */
-	  unsubstituted_packs = true;
-	  if (!missing_level || missing_level > level)
-	    missing_level = level;
-	}
-    }
 
-  /* We cannot expand this expansion expression, because we don't have
-     all of the argument packs we need.  */
-  if (unsubstituted_packs)
-    {
-      if (real_packs)
-	{
-	  /* We got some full packs, but we can't substitute them in until we
-	     have values for all the packs.  So remember these until then.  */
-	  tree save_args;
-
-	  t = make_pack_expansion (pattern);
-
-	  /* The call to add_to_template_args above assumes no overlap
-	     between saved args and new args, so prune away any fake
-	     args, i.e. those that satisfied arg_from_parm_pack_p above.  */
-	  if (missing_level && levels >= missing_level)
-	    {
-	      gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
-			  && missing_level > 1);
-	      TREE_VEC_LENGTH (args) = missing_level - 1;
-	      save_args = copy_node (args);
-	      TREE_VEC_LENGTH (args) = levels;
-	    }
-	  else
-	    save_args = args;
-
-	  PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
-	}
-      else
-	{
-	  /* There were no real arguments, we're just replacing a parameter
-	     pack with another version of itself. Substitute into the
-	     pattern and return a PACK_EXPANSION_*. The caller will need to
-	     deal with that.  */
-	  if (TREE_CODE (t) == EXPR_PACK_EXPANSION)
-	    t = tsubst_expr (pattern, args, complain, in_decl,
-			     /*integral_constant_expression_p=*/false);
-	  else
-	    t = tsubst (pattern, args, complain, in_decl);
-	  t = make_pack_expansion (t);
-	}
-      return t;
+      /* Keep track of the parameter packs and their corresponding
+	 argument packs.  */
+      packs = tree_cons (parm_pack, arg_pack, packs);
+      TREE_TYPE (packs) = orig_arg;
     }
 
-  /* We could not find any argument packs that work.  */
+  /* We could not find any argument packs that work, so we'll just
+     return an unsubstituted pack expansion.  The caller must be
+     prepared to deal with this.  */
   if (len < 0)
-    return error_mark_node;
+    len = 1;
 
   if (need_local_specializations)
     {
@@ -9305,63 +9569,39 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 
   /* For each argument in each argument pack, substitute into the
      pattern.  */
-  result = make_tree_vec (len);
-  for (i = 0; i < len; ++i)
-    {
-      /* For parameter pack, change the substitution of the parameter
-         pack to the ith argument in its argument pack, then expand
-         the pattern.  */
-      for (pack = packs; pack; pack = TREE_CHAIN (pack))
-        {
-          tree parm = TREE_PURPOSE (pack);
-	  tree arg;
-
-	  /* Select the Ith argument from the pack.  */
-          if (TREE_CODE (parm) == PARM_DECL)
-            {
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  mark_used (parm);
-		  register_local_specialization (arg, parm);
-		}
-	      else
-		arg = retrieve_local_specialization (parm);
-            }
-          else
-            {
-              int idx, level;
-              template_parm_level_and_index (parm, &level, &idx);
-
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
-		}
-	      else
-		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
-            }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
-        }
+  if (len > 0)
+    {
+      bool yield_no_list = false;
+      t = gen_elem_of_pack_expansion_instantiation (pattern, packs, 0,
+						    &yield_no_list,
+						    args, complain,
+						    in_decl);
+      if (yield_no_list)
+	{
+	  result = t;
+	  goto out;
+	}
 
-      /* Substitute into the PATTERN with the altered arguments.  */
-      if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
-      else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
+      result = make_tree_vec (len);
+      TREE_VEC_ELT (result, 0) = t;
 
-      if (TREE_VEC_ELT (result, i) == error_mark_node)
+      for (i = 1; i < len; ++i)
 	{
-	  result = error_mark_node;
-	  break;
+	  
+	  t = gen_elem_of_pack_expansion_instantiation (pattern, packs, i,
+							&yield_no_list,
+							args, complain,
+							in_decl);
+	  TREE_VEC_ELT (result, i) = t;
+      	  if (t == error_mark_node)
+	    {
+	      result = error_mark_node;
+	      break;
+	    }
 	}
     }
+  else
+    result = make_tree_vec (0);
 
   /* Update ARGS to restore the substitution from parameter packs to
      their argument packs.  */
@@ -9385,6 +9625,7 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
         }
     }
 
+ out:
   if (need_local_specializations)
     {
       pointer_map_destroy (local_specializations);
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..89f7b32
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};                           
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
-- 
1.7.11.7



-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

On 12/03/2012 08:27 AM, Dodji Seketeli wrote:
> +	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
> +	  pack has got an argument that is an expansion.

The "got" is unnecessary, just "has an argument" is better.

> +   Setup APS, which is an instance of an ARGUMENT_PACK_SELECT tree, so
> +   that it selects the Ith element out of the argument pack ARG_PACK.
> +   If the Ith element is a pack expansion, then just select its
> +   pattern.  Otherwise, select the whole element.

I wonder if, rather than set up a temporary pack at this point, it makes 
sense to look through pack expansions when we use an 
ARGUMENT_PACK_SELECT.  Is there any case where we actually want an 
ARGUMENT_PACK_SELECT to be an expansion?

> +  /* If we have one parameter pack whose matching argument pack is
> +     just what template_parm_to_arg returned when passed the
> +     parameter pack, or if we only have empty arguments ... */
> +  else if (arg_from_pack_level_to_prune || has_empty_arg)
> +    {
> +      /* ... we just return a pack expansion which pattern is PATTERN
> +	 into which ARGS has been substituted.  */
> +      *instantiation_yields_no_list_p = true;
> +    }

I was thinking we wouldn't need to recognize this case specifically, 
that the code following it would work the way we want.  If callers get a 
vector with a single pack expansion element rather than just a pack 
expansion, is that a problem?  Alternately, if len == 1, maybe we should 
always just return the single element.

> +  /* We could not find any argument packs that work, so we'll just
> +     return an unsubstituted pack expansion.  The caller must be
> +     prepared to deal with this.  */
>    if (len < 0)
> -    return error_mark_node;
> +    len = 1;

Why this change?  Why is returning error_mark_node no longer the right 
thing to do?

Jason

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 12/03/2012 08:27 AM, Dodji Seketeli wrote:
> > +	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
> > +	  pack has got an argument that is an expansion.
> 
> The "got" is unnecessary, just "has an argument" is better.

Removed, thanks.

> > +   Setup APS, which is an instance of an ARGUMENT_PACK_SELECT tree, so
> > +   that it selects the Ith element out of the argument pack ARG_PACK.
> > +   If the Ith element is a pack expansion, then just select its
> > +   pattern.  Otherwise, select the whole element.
> 
> I wonder if, rather than set up a temporary pack at this point, it
> makes sense to look through pack expansions when we use an
> ARGUMENT_PACK_SELECT.

Yeah, good point.

> Is there any case where we actually want an ARGUMENT_PACK_SELECT to
> be an expansion?

I did the change to look through pack expansions in tsubst and nothing
in the testsuite seems to be wanting an ARGUMENT_PACK_SELECT to
resolve to an expansion.

> 
> > +  /* If we have one parameter pack whose matching argument pack is
> > +     just what template_parm_to_arg returned when passed the
> > +     parameter pack, or if we only have empty arguments ... */
> > +  else if (arg_from_pack_level_to_prune || has_empty_arg)
> > +    {
> > +      /* ... we just return a pack expansion which pattern is PATTERN
> > +	 into which ARGS has been substituted.  */
> > +      *instantiation_yields_no_list_p = true;
> > +    }
> 
> I was thinking we wouldn't need to recognize this case specifically,
> that the code following it would work the way we want.  If callers get
> a vector with a single pack expansion element rather than just a pack
> expansion, is that a problem?  Alternately, if len == 1, maybe we
> should always just return the single element.

Right, I made the change to do away with the need of the
instantiation_yields_no_list_p variable.

Though I think it would still be appropriate to keep this 'if' just to
avoid going into the 'else' block for cases where we know that looping
over the parameter packs (and do the ARGUMENT_PACK_SELECT dance) is
unnecessary; all we want is to go straight to the point where we
substitute args into the pattern, build a pack expansion and return
it.  Or isn't what you meant?

> 
> > +  /* We could not find any argument packs that work, so we'll just
> > +     return an unsubstituted pack expansion.  The caller must be
> > +     prepared to deal with this.  */
> >    if (len < 0)
> > -    return error_mark_node;
> > +    len = 1;
> 
> Why this change?  Why is returning error_mark_node no longer the right
> thing to do?

My understanding is that in practise, we never hit this point in the
previous code.  The reason why len would still be negative at this
point would be if we didn't find any (good) argument pack.  But in
that case, we would have considered that we have unsubstituted packs
and would have returned an unsubstituted pack expansion before we
reach this point.  So I believe this new code follows the same logic.
Am I missing something?  Also, I just tried returning error_mark_node,
(just to make sure) and it broke quite some tests of the suite.


Tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* pt.c (argument_pack_element_is_expansion_p)
	(make_argument_pack_select, scan_parm_packs)
	(gen_elem_of_pack_expansion_instantiation): New static functions.
	(has_bare_parameter_packs): Factorized out of ...
	(check_for_bare_parameter_packs): ... here.
	(tsubst): When looking through an ARGUMENT_PACK_SELECT tree node,
	look through the possibly resulting pack expansion as well.
	(tsubst_pack_expansion): Now this function is clearly organized in
	two parts: a part that maps each parameter pack with its matching
	argument pack, and a part that walks that map to build the result
	of substituting each element of the argument packs into the
	parameter packs.  Use gen_elem_of_pack_expansion_instantiation for
	the latter part.

gcc/testsuite/

	* g++.dg/cpp0x/variadic139.C: New test.
	* g++.dg/cpp0x/variadic140.C: Likewise.
	* g++.dg/cpp0x/variadic141.C: Likewise.
---
 gcc/cp/pt.c                              | 459 +++++++++++++++++++++----------
 gcc/testsuite/g++.dg/cpp0x/variadic139.C |  14 +
 gcc/testsuite/g++.dg/cpp0x/variadic140.C |  22 ++
 gcc/testsuite/g++.dg/cpp0x/variadic141.C |  22 ++
 4 files changed, 373 insertions(+), 144 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ecb013e..1436721 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3308,6 +3308,29 @@ make_pack_expansion (tree arg)
   return result;
 }
 
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+  tree parameter_packs = NULL_TREE;
+  struct find_parameter_pack_data ppd;
+
+  if (!processing_template_decl || !t || t == error_mark_node)
+    return NULL_TREE;
+
+  if (TREE_CODE (t) == TYPE_DECL)
+    t = TREE_TYPE (t);
+
+  ppd.parameter_packs = &parameter_packs;
+  ppd.visited = pointer_set_create ();
+  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
+  pointer_set_destroy (ppd.visited);
+
+  return parameter_packs;
+}
+
 /* Checks T for any "bare" parameter packs, which have not yet been
    expanded, and issues an error if any are found. This operation can
    only be done on full expressions or types (e.g., an expression
@@ -3325,19 +3348,7 @@ make_pack_expansion (tree arg)
 bool 
 check_for_bare_parameter_packs (tree t)
 {
-  tree parameter_packs = NULL_TREE;
-  struct find_parameter_pack_data ppd;
-
-  if (!processing_template_decl || !t || t == error_mark_node)
-    return false;
-
-  if (TREE_CODE (t) == TYPE_DECL)
-    t = TREE_TYPE (t);
-
-  ppd.parameter_packs = &parameter_packs;
-  ppd.visited = pointer_set_create ();
-  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
-  pointer_set_destroy (ppd.visited);
+  tree parameter_packs = has_bare_parameter_packs (t);
 
   if (parameter_packs) 
     {
@@ -9095,6 +9106,256 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is a
+   pack expansion.  */
+
+static bool
+argument_pack_element_is_expansion_p (tree arg_pack, int i)
+{
+  tree vec = ARGUMENT_PACK_ARGS (arg_pack);
+  if (i >= TREE_VEC_LENGTH (vec))
+    return false;
+  return PACK_EXPANSION_P (TREE_VEC_ELT (vec, i));
+}
+
+
+/* Creates and return an ARGUMENT_PACK_SELECT tree node.  */
+
+static tree
+make_argument_pack_select (tree arg_pack, unsigned index)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+
+  return aps;
+}
+
+/*  This is a subroutine of gen_elem_of_pack_expansion_instantiation,
+    which is itself a subroutine of tsubst_pack_expansion.
+
+    For a given INDEX of an argument pack, this function scans the
+    list of parameter packs PARM_PACKS and reports some interesting
+    properties about them.  Note that PARM_PACKS must be an instance of
+    TREE_LIST where TREE_PURPOSE is a parameter pack, and TREE_VALUE
+    is the argument for that pack.
+
+    The properties returned about the scanned parameter packs
+    are the following:
+
+	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
+	  pack has an argument that is an expansion.
+	- HAS_NON_EXPANSION_ARG_P: Set to TRUE iff at least one
+	  parameter pack has an argument that is not an expansion.
+	- HAS_EMPTY_ARG_P: Set to TRUE iff at least one parameter pack has
+	  no argument.
+	- ARG_FROM_PACK_LEVEL_TO_PRUNE: is non-zero iff the argument of
+	  the parameter pack is the result of calling template_parm_to_arg
+	  on the pack.  In that case, this is set to the parameter level
+	  of that pack.  */
+
+static void
+scan_parm_packs (tree parm_packs,
+		 unsigned index,
+		 bool *has_expansion_arg_p,
+		 bool *has_non_expansion_arg_p,
+		 bool *has_empty_arg_p,
+		 int *arg_from_pack_level_to_prune)
+{
+
+  if (parm_packs == NULL_TREE)
+    {
+      if (has_empty_arg_p)
+	*has_empty_arg_p = true;
+      return ;
+    }
+
+  for (tree parm_pack = parm_packs;
+       parm_pack;
+       parm_pack = TREE_CHAIN (parm_pack))
+    {
+      tree arg = TREE_VALUE (parm_pack);
+      tree parm = TREE_PURPOSE (parm_pack);
+
+      if (arg == NULL_TREE)
+	{
+	  if (has_empty_arg_p)
+	    *has_empty_arg_p = true;
+	  continue;
+	}
+
+      if (arg_from_pack_level_to_prune != NULL
+	  && arg_from_parm_pack_p (arg, parm))
+	{
+	  int level = 0, idx = 0;
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    parm = DECL_INITIAL (parm);
+	  template_parm_level_and_index (parm, &level, &idx);
+	  if (*arg_from_pack_level_to_prune == 0
+	      || *arg_from_pack_level_to_prune > level)
+	    *arg_from_pack_level_to_prune = level;
+	  return;
+	}
+
+      if (argument_pack_element_is_expansion_p (arg, index))
+	{
+	  if (has_expansion_arg_p != NULL)
+	    *has_expansion_arg_p = true;
+	}
+      else
+	{
+	  if (has_non_expansion_arg_p != NULL)
+	    *has_non_expansion_arg_p = true;
+	}
+    }
+}
+
+/* [temp.variadic]/6 says that:
+
+       The instantiation of a pack expansion [...]
+       produces a list E1,E2, ..., En, where N is the number of elements
+       in the pack expansion parameters.
+
+   This subroutine of tsubst_pack_expansion produces one of these Ei.
+
+   PATTERN is the pattern of the pack expansion.  PARM_PACKS is a
+   TREE_LIST in which each TREE_PURPOSE is a parameter pack of
+   PATTERN, and each TREE_VALUE is its corresponding argument pack.
+   INDEX is the index 'i' of the element Ei to produce.  ARGS,
+   COMPLAIN, and IN_DECL are the same parameters as for the
+   tsubst_pack_expansion function.
+
+   The function returns the resulting Ei upon successful completion,
+   or error_mark_node.
+
+   Note that this function possibly modifies the ARGS parameter, so
+   it's the responsibility of the caller to restore it.  */
+
+static tree
+gen_elem_of_pack_expansion_instantiation (tree pattern,
+					  tree parm_packs,
+					  unsigned index,
+					  tree args /* This parm gets
+						       modified.  */,
+					  tsubst_flags_t complain,
+					  tree in_decl)
+{
+  /* The boolean below is TRUE if at least one parameter pack doesn't
+     yet have all argument packs to really perform the
+     susbtituting.  */
+  bool use_pack_expansion_extra = false,
+    has_expansion_arg = false,
+    has_non_expansion_arg = false,
+    has_empty_arg = false;
+  int arg_from_pack_level_to_prune = 0;
+  tree t;
+
+  scan_parm_packs (parm_packs, index,
+		   &has_expansion_arg,
+		   &has_non_expansion_arg,
+		   &has_empty_arg,
+		   &arg_from_pack_level_to_prune);
+
+  if ((has_expansion_arg && has_non_expansion_arg)
+      || (has_empty_arg && (has_expansion_arg || has_non_expansion_arg)))
+    use_pack_expansion_extra = true;
+
+  if (use_pack_expansion_extra)
+    {
+      /* We got some full packs, but we can't substitute them in until we
+	 have values for all the packs.  So remember these until then.  */
+
+      int levels = TMPL_ARGS_DEPTH (args);
+      tree save_args;
+
+      /* The call to add_to_template_args in tsubst_pack_expansion
+	 assumes no overlap between saved args and new args, so prune
+	 away any fake args, i.e. those that satisfied
+	 arg_from_parm_pack_p in scan_parm_packs above.  */
+      if (arg_from_pack_level_to_prune != 0
+	  && levels >= arg_from_pack_level_to_prune)
+	{
+	  gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
+		      && arg_from_pack_level_to_prune > 1);
+	  TREE_VEC_LENGTH (args) = arg_from_pack_level_to_prune - 1;
+	  save_args = copy_node (args);
+	  TREE_VEC_LENGTH (args) = levels;
+	}
+      else
+	save_args = args;
+
+      t = make_pack_expansion (pattern);
+      PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
+
+      return t;
+    }
+  /* If we have one parameter pack whose matching argument pack is
+     just what template_parm_to_arg returned when passed the
+     parameter pack, or if we only have empty arguments ... */
+  else if (arg_from_pack_level_to_prune || has_empty_arg)
+    /* ... we just return a pack expansion which pattern is PATTERN
+       into which ARGS has been substituted.  */;
+  else
+    {
+      /* For each parameter pack, change the substitution of the parameter
+	 pack to the ith argument in its argument pack, then expand the
+	 pattern.  */
+      for (tree pack = parm_packs; pack; pack = TREE_CHAIN (pack))
+	{
+	  tree parm = TREE_PURPOSE (pack);
+	  tree arg_pack = TREE_VALUE (pack);
+	  tree aps;			/* instance of ARGUMENT_PACK_SELECT
+					   tree.  */
+
+	  /* Select the Ith argument from the pack.  */
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    {
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  mark_used (parm);
+		  register_local_specialization (aps, parm);
+		}
+	      else
+		aps = retrieve_local_specialization (parm);
+	    }
+	  else
+	    {
+	      int idx, level;
+	      template_parm_level_and_index (parm, &level, &idx);
+
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  /* Update the corresponding argument.  */
+		  TMPL_ARG (args, level, idx) = aps;
+		}
+	      else
+		/* Re-use the ARGUMENT_PACK_SELECT.  */
+		aps = TMPL_ARG (args, level, idx);
+	    }
+	  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+	}
+    }
+
+  /* Substitute into the PATTERN with the (possibly altered)
+     arguments.  */
+  if (!TYPE_P (pattern))
+    t = tsubst_expr (pattern, args, complain, in_decl,
+		     /*integral_constant_expression_p=*/false);
+  else
+    t = tsubst (pattern, args, complain, in_decl);
+
+  /*  If the Ith argument pack element is a pack expansion, then
+      the Ith element resulting from the substituting is going to
+      be a pack expansion as well.  */
+  if (has_bare_parameter_packs (t))
+    t = make_pack_expansion (t);
+
+  return t;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9106,9 +9367,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 {
   tree pattern;
   tree pack, packs = NULL_TREE;
-  bool unsubstituted_packs = false;
-  bool real_packs = false;
-  int missing_level = 0;
   int i, len = -1;
   tree result;
   struct pointer_map_t *saved_local_specializations = NULL;
@@ -9187,14 +9445,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	  return result;
 	}
 
-      if (arg_from_parm_pack_p (arg_pack, parm_pack))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would find
-	   in the implicit typedef of a class inside the class itself.
-	   Consider this parameter "unsubstituted", so that we will
-	   maintain the outer pack expansion.  */
-	arg_pack = NULL_TREE;
-          
       if (arg_pack)
         {
           int my_len = 
@@ -9221,77 +9471,19 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
                        pattern);
               return error_mark_node;
             }
-
-	  if (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-	      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
-						 0)))
-	    /* This isn't a real argument pack yet.  */;
-	  else
-	    real_packs = true;
-
-          /* Keep track of the parameter packs and their corresponding
-             argument packs.  */
-          packs = tree_cons (parm_pack, arg_pack, packs);
-          TREE_TYPE (packs) = orig_arg;
         }
-      else
-	{
-	  /* We can't substitute for this parameter pack.  We use a flag as
-	     well as the missing_level counter because function parameter
-	     packs don't have a level.  */
-	  unsubstituted_packs = true;
-	  if (!missing_level || missing_level > level)
-	    missing_level = level;
-	}
-    }
-
-  /* We cannot expand this expansion expression, because we don't have
-     all of the argument packs we need.  */
-  if (unsubstituted_packs)
-    {
-      if (real_packs)
-	{
-	  /* We got some full packs, but we can't substitute them in until we
-	     have values for all the packs.  So remember these until then.  */
-	  tree save_args;
-
-	  t = make_pack_expansion (pattern);
 
-	  /* The call to add_to_template_args above assumes no overlap
-	     between saved args and new args, so prune away any fake
-	     args, i.e. those that satisfied arg_from_parm_pack_p above.  */
-	  if (missing_level && levels >= missing_level)
-	    {
-	      gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
-			  && missing_level > 1);
-	      TREE_VEC_LENGTH (args) = missing_level - 1;
-	      save_args = copy_node (args);
-	      TREE_VEC_LENGTH (args) = levels;
-	    }
-	  else
-	    save_args = args;
-
-	  PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
-	}
-      else
-	{
-	  /* There were no real arguments, we're just replacing a parameter
-	     pack with another version of itself. Substitute into the
-	     pattern and return a PACK_EXPANSION_*. The caller will need to
-	     deal with that.  */
-	  if (TREE_CODE (t) == EXPR_PACK_EXPANSION)
-	    t = tsubst_expr (pattern, args, complain, in_decl,
-			     /*integral_constant_expression_p=*/false);
-	  else
-	    t = tsubst (pattern, args, complain, in_decl);
-	  t = make_pack_expansion (t);
-	}
-      return t;
+      /* Keep track of the parameter packs and their corresponding
+	 argument packs.  */
+      packs = tree_cons (parm_pack, arg_pack, packs);
+      TREE_TYPE (packs) = orig_arg;
     }
 
-  /* We could not find any argument packs that work.  */
+  /* We could not find any argument packs that work, so we'll just
+     return an unsubstituted pack expansion.  The caller must be
+     prepared to deal with this.  */
   if (len < 0)
-    return error_mark_node;
+    len = 1;
 
   if (need_local_specializations)
     {
@@ -9306,60 +9498,19 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   /* For each argument in each argument pack, substitute into the
      pattern.  */
   result = make_tree_vec (len);
-  for (i = 0; i < len; ++i)
+  if (len > 0)
     {
-      /* For parameter pack, change the substitution of the parameter
-         pack to the ith argument in its argument pack, then expand
-         the pattern.  */
-      for (pack = packs; pack; pack = TREE_CHAIN (pack))
-        {
-          tree parm = TREE_PURPOSE (pack);
-	  tree arg;
-
-	  /* Select the Ith argument from the pack.  */
-          if (TREE_CODE (parm) == PARM_DECL)
-            {
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  mark_used (parm);
-		  register_local_specialization (arg, parm);
-		}
-	      else
-		arg = retrieve_local_specialization (parm);
-            }
-          else
-            {
-              int idx, level;
-              template_parm_level_and_index (parm, &level, &idx);
-
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
-		}
-	      else
-		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
-            }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
-        }
-
-      /* Substitute into the PATTERN with the altered arguments.  */
-      if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
-      else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
-
-      if (TREE_VEC_ELT (result, i) == error_mark_node)
+      for (i = 0; i < len; ++i)
 	{
-	  result = error_mark_node;
-	  break;
+	  t = gen_elem_of_pack_expansion_instantiation (pattern, packs, i,
+							args, complain,
+							in_decl);
+	  TREE_VEC_ELT (result, i) = t;
+      	  if (t == error_mark_node)
+	    {
+	      result = error_mark_node;
+	      break;
+	    }
 	}
     }
 
@@ -9374,6 +9525,10 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       else
         {
           int idx, level;
+
+	  if (TREE_VALUE (pack) == NULL_TREE)
+	    continue;
+
           template_parm_level_and_index (parm, &level, &idx);
           
           /* Update the corresponding argument.  */
@@ -11102,8 +11257,24 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    arg = TMPL_ARG (args, level, idx);
 
 	    if (arg && TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-	      /* See through ARGUMENT_PACK_SELECT arguments. */
-	      arg = ARGUMENT_PACK_SELECT_ARG (arg);
+	      {
+		/* See through ARGUMENT_PACK_SELECT arguments. */
+		arg = ARGUMENT_PACK_SELECT_ARG (arg);
+		/* If the selected argument is an expansion E, that most
+		   likely means we were called from
+		   gen_elem_of_pack_expansion_instantiation during the
+		   substituting of pack an argument pack (which Ith
+		   element is a pack expansion, where I is
+		   ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
+		   In this case, the Ith element resulting from this
+		   substituting is going to be a pack expansion, which
+		   pattern is the pattern of E.  Let's return the
+		   pattern of E, and
+		   gen_elem_of_pack_expansion_instantiation will
+		   build the resulting pack expansion from it.  */
+		if (PACK_EXPANSION_P (arg))
+		  arg = PACK_EXPANSION_PATTERN (arg);
+	      }
 	  }
 
 	if (arg == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..a1c64f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

On 12/08/2012 05:12 PM, Dodji Seketeli wrote:
>>> +  else if (arg_from_pack_level_to_prune || has_empty_arg)
>>> +    {
>>> +      /* ... we just return a pack expansion which pattern is PATTERN
>>> +	 into which ARGS has been substituted.  */
>>> +      *instantiation_yields_no_list_p = true;
>>> +    }
>>
> Though I think it would still be appropriate to keep this 'if' just to
> avoid going into the 'else' block for cases where we know that looping
> over the parameter packs (and do the ARGUMENT_PACK_SELECT dance) is
> unnecessary; all we want is to go straight to the point where we
> substitute args into the pattern, build a pack expansion and return
> it.  Or isn't what you meant?

I suppose we should keep this for has_empty_arg, but I'd like to do away 
with special handling of the arg_from_parm_pack case if possible, as 
it's a lot of extra complexity.

> My understanding is that in practise, we never hit this point in the
> previous code.  The reason why len would still be negative at this
> point would be if we didn't find any (good) argument pack.  But in
> that case, we would have considered that we have unsubstituted packs
> and would have returned an unsubstituted pack expansion before we
> reach this point.

I don't see that; in the old code, if there are unsubstituted packs we 
tsubst the args into the pattern.

> +      /* We got some full packs, but we can't substitute them in until we
> +	 have values for all the packs.  So remember these until then.  */

How can having this in gen_elem_of_pack_expansion_instantiation could 
work?  We don't want an expansion for each element of the packs that we 
have.

Jason

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 12/08/2012 05:12 PM, Dodji Seketeli wrote:
> >>> +  else if (arg_from_pack_level_to_prune || has_empty_arg)
> >>> +    {
> >>> +      /* ... we just return a pack expansion which pattern is PATTERN
> >>> +	 into which ARGS has been substituted.  */
> >>> +      *instantiation_yields_no_list_p = true;
> >>> +    }
> >>
> > Though I think it would still be appropriate to keep this 'if' just to
> > avoid going into the 'else' block for cases where we know that looping
> > over the parameter packs (and do the ARGUMENT_PACK_SELECT dance) is
> > unnecessary; all we want is to go straight to the point where we
> > substitute args into the pattern, build a pack expansion and return
> > it.  Or isn't what you meant?
> 
> I suppose we should keep this for has_empty_arg, but I'd like to do
> away with special handling of the arg_from_parm_pack case if possible,
> as it's a lot of extra complexity.

OK, done.

> 
> > My understanding is that in practise, we never hit this point in the
> > previous code.  The reason why len would still be negative at this
> > point would be if we didn't find any (good) argument pack.  But in
> > that case, we would have considered that we have unsubstituted packs
> > and would have returned an unsubstituted pack expansion before we
> > reach this point.
> 
> I don't see that; in the old code, if there are unsubstituted packs we
> tsubst the args into the pattern.

Yes, and we build and return a pack expansion with that pattern.
That's what I meant clumsily by "unsubstituted pack expansion".  I
should have probably said "uninstantiated".

So, all this to say that we never return error_mark_node for the case
where len < 0 in practise.  Do you agree?

> 
> > +      /* We got some full packs, but we can't substitute them in until we
> > +	 have values for all the packs.  So remember these until then.  */
> 
> How can having this in gen_elem_of_pack_expansion_instantiation could
> work?  We don't want an expansion for each element of the packs that
> we have.

Oops, it seems I was too hasty in trying to do away with the
instantiation_yields_no_list_p parameter to
gen_elem_of_pack_expansion_instantiation.  That is, whenever we are in
this case we must return just one expansion, as you noted.  So I am
putting back a way for gen_elem_of_pack_expansion_instantiation to
notify its caller that it needs to return the uninstantiated pack
expansion right away.

Tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* pt.c (argument_pack_element_is_expansion_p)
	(make_argument_pack_select, scan_parm_packs)
	(gen_elem_of_pack_expansion_instantiation): New static functions.
	(has_bare_parameter_packs): Factorized out of ...
	(check_for_bare_parameter_packs): ... here.
	(tsubst): When looking through an ARGUMENT_PACK_SELECT tree node,
	look through the possibly resulting pack expansion as well.
	(tsubst_pack_expansion): Now this function is clearly organized in
	two parts: a part that maps each parameter pack with its matching
	argument pack, and a part that walks that map to build the result
	of substituting each element of the argument packs into the
	parameter packs.  Use gen_elem_of_pack_expansion_instantiation for
	the latter part.

gcc/testsuite/

	* g++.dg/cpp0x/variadic139.C: New test.
	* g++.dg/cpp0x/variadic140.C: Likewise.
	* g++.dg/cpp0x/variadic141.C: Likewise.
---
 gcc/cp/pt.c                              | 473 +++++++++++++++++++++----------
 gcc/testsuite/g++.dg/cpp0x/variadic139.C |  14 +
 gcc/testsuite/g++.dg/cpp0x/variadic140.C |  22 ++
 gcc/testsuite/g++.dg/cpp0x/variadic141.C |  22 ++
 4 files changed, 387 insertions(+), 144 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ecb013e..663d46a 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3308,6 +3308,29 @@ make_pack_expansion (tree arg)
   return result;
 }
 
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+  tree parameter_packs = NULL_TREE;
+  struct find_parameter_pack_data ppd;
+
+  if (!processing_template_decl || !t || t == error_mark_node)
+    return NULL_TREE;
+
+  if (TREE_CODE (t) == TYPE_DECL)
+    t = TREE_TYPE (t);
+
+  ppd.parameter_packs = &parameter_packs;
+  ppd.visited = pointer_set_create ();
+  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
+  pointer_set_destroy (ppd.visited);
+
+  return parameter_packs;
+}
+
 /* Checks T for any "bare" parameter packs, which have not yet been
    expanded, and issues an error if any are found. This operation can
    only be done on full expressions or types (e.g., an expression
@@ -3325,19 +3348,7 @@ make_pack_expansion (tree arg)
 bool 
 check_for_bare_parameter_packs (tree t)
 {
-  tree parameter_packs = NULL_TREE;
-  struct find_parameter_pack_data ppd;
-
-  if (!processing_template_decl || !t || t == error_mark_node)
-    return false;
-
-  if (TREE_CODE (t) == TYPE_DECL)
-    t = TREE_TYPE (t);
-
-  ppd.parameter_packs = &parameter_packs;
-  ppd.visited = pointer_set_create ();
-  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
-  pointer_set_destroy (ppd.visited);
+  tree parameter_packs = has_bare_parameter_packs (t);
 
   if (parameter_packs) 
     {
@@ -9095,6 +9106,262 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is a
+   pack expansion.  */
+
+static bool
+argument_pack_element_is_expansion_p (tree arg_pack, int i)
+{
+  tree vec = ARGUMENT_PACK_ARGS (arg_pack);
+  if (i >= TREE_VEC_LENGTH (vec))
+    return false;
+  return PACK_EXPANSION_P (TREE_VEC_ELT (vec, i));
+}
+
+
+/* Creates and return an ARGUMENT_PACK_SELECT tree node.  */
+
+static tree
+make_argument_pack_select (tree arg_pack, unsigned index)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+
+  return aps;
+}
+
+/*  This is a subroutine of gen_elem_of_pack_expansion_instantiation,
+    which is itself a subroutine of tsubst_pack_expansion.
+
+    For a given INDEX of an argument pack, this function scans the
+    list of parameter packs PARM_PACKS and reports some interesting
+    properties about them.  Note that PARM_PACKS must be an instance of
+    TREE_LIST where TREE_PURPOSE is a parameter pack, and TREE_VALUE
+    is the argument for that pack.
+
+    The properties returned about the scanned parameter packs
+    are the following:
+
+	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
+	  pack has an argument that is an expansion.
+	- HAS_NON_EXPANSION_ARG_P: Set to TRUE iff at least one
+	  parameter pack has an argument that is not an expansion.
+	- HAS_EMPTY_ARG_P: Set to TRUE iff at least one parameter pack has
+	  no argument.
+	- ARG_FROM_PACK_LEVEL_TO_PRUNE: is non-zero iff the argument of
+	  the parameter pack is the result of calling template_parm_to_arg
+	  on the pack.  In that case, this is set to the parameter level
+	  of that pack.  */
+
+static void
+scan_parm_packs (tree parm_packs,
+		 unsigned index,
+		 bool *has_expansion_arg_p,
+		 bool *has_non_expansion_arg_p,
+		 bool *has_empty_arg_p,
+		 int *arg_from_pack_level_to_prune)
+{
+
+  if (parm_packs == NULL_TREE)
+    {
+      if (has_empty_arg_p)
+	*has_empty_arg_p = true;
+      return ;
+    }
+
+  for (tree parm_pack = parm_packs;
+       parm_pack;
+       parm_pack = TREE_CHAIN (parm_pack))
+    {
+      tree arg = TREE_VALUE (parm_pack);
+      tree parm = TREE_PURPOSE (parm_pack);
+
+      if (arg == NULL_TREE)
+	{
+	  if (has_empty_arg_p)
+	    *has_empty_arg_p = true;
+	  continue;
+	}
+
+      if (arg_from_pack_level_to_prune != NULL
+	  && arg_from_parm_pack_p (arg, parm))
+	{
+	  int level = 0, idx = 0;
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    parm = DECL_INITIAL (parm);
+	  template_parm_level_and_index (parm, &level, &idx);
+	  if (*arg_from_pack_level_to_prune == 0
+	      || *arg_from_pack_level_to_prune > level)
+	    *arg_from_pack_level_to_prune = level;
+	  return;
+	}
+
+      if (argument_pack_element_is_expansion_p (arg, index))
+	{
+	  if (has_expansion_arg_p != NULL)
+	    *has_expansion_arg_p = true;
+	}
+      else
+	{
+	  if (has_non_expansion_arg_p != NULL)
+	    *has_non_expansion_arg_p = true;
+	}
+    }
+}
+
+/* [temp.variadic]/6 says that:
+
+       The instantiation of a pack expansion [...]
+       produces a list E1,E2, ..., En, where N is the number of elements
+       in the pack expansion parameters.
+
+   This subroutine of tsubst_pack_expansion produces one of these Ei.
+
+   PATTERN is the pattern of the pack expansion.  PARM_PACKS is a
+   TREE_LIST in which each TREE_PURPOSE is a parameter pack of
+   PATTERN, and each TREE_VALUE is its corresponding argument pack.
+   INDEX is the index 'i' of the element Ei to produce.  ARGS,
+   COMPLAIN, and IN_DECL are the same parameters as for the
+   tsubst_pack_expansion function.
+
+   The function returns the resulting Ei upon successful completion,
+   or error_mark_node.  If the *INSTANTIATION_YIELDS_NO_LIST_P is set
+   to true upon completion, that means the result of the function is
+   not an element of a list, but rather the sole result of the pack
+   expansion substituting that has to be returned as the result of the
+   tsubst_pack_expansion function.
+
+   Note that this function possibly modifies the ARGS parameter, so
+   it's the responsibility of the caller to restore it.  */
+
+static tree
+gen_elem_of_pack_expansion_instantiation (tree pattern,
+					  tree parm_packs,
+					  unsigned index,
+					  bool *instantiation_yields_no_list_p,
+					  tree args /* This parm gets
+						       modified.  */,
+					  tsubst_flags_t complain,
+					  tree in_decl)
+{
+  /* The boolean below is TRUE if at least one parameter pack doesn't
+     yet have all argument packs to really perform the
+     susbtituting.  */
+  bool use_pack_expansion_extra = false,
+    has_expansion_arg = false,
+    has_non_expansion_arg = false,
+    has_empty_arg = false;
+  int arg_from_pack_level_to_prune = 0;
+  tree t;
+
+  *instantiation_yields_no_list_p = false;
+
+  scan_parm_packs (parm_packs, index,
+		   &has_expansion_arg,
+		   &has_non_expansion_arg,
+		   &has_empty_arg,
+		   &arg_from_pack_level_to_prune);
+
+  if ((has_expansion_arg && has_non_expansion_arg)
+      || (has_empty_arg && (has_expansion_arg || has_non_expansion_arg)))
+    use_pack_expansion_extra = true;
+
+  if (use_pack_expansion_extra)
+    {
+      /* We got some full packs, but we can't substitute them in until we
+	 have values for all the packs.  So remember these until then.  */
+
+      int levels = TMPL_ARGS_DEPTH (args);
+      tree save_args;
+
+      /* The call to add_to_template_args in tsubst_pack_expansion
+	 assumes no overlap between saved args and new args, so prune
+	 away any fake args, i.e. those that satisfied
+	 arg_from_parm_pack_p in scan_parm_packs above.  */
+      if (arg_from_pack_level_to_prune != 0
+	  && levels >= arg_from_pack_level_to_prune)
+	{
+	  gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
+		      && arg_from_pack_level_to_prune > 1);
+	  TREE_VEC_LENGTH (args) = arg_from_pack_level_to_prune - 1;
+	  save_args = copy_node (args);
+	  TREE_VEC_LENGTH (args) = levels;
+	}
+      else
+	save_args = args;
+
+      t = make_pack_expansion (pattern);
+      PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
+
+      *instantiation_yields_no_list_p = true;
+      return t;
+    }
+  /* If we have empty arguments ... */
+  else if (has_empty_arg)
+    /* ... we just return a pack expansion which pattern is PATTERN
+       into which ARGS has been substituted.  */;
+  else
+    {
+      /* For each parameter pack, change the substitution of the parameter
+	 pack to the ith argument in its argument pack, then expand the
+	 pattern.  */
+      for (tree pack = parm_packs; pack; pack = TREE_CHAIN (pack))
+	{
+	  tree parm = TREE_PURPOSE (pack);
+	  tree arg_pack = TREE_VALUE (pack);
+	  tree aps;			/* instance of ARGUMENT_PACK_SELECT
+					   tree.  */
+
+	  /* Select the Ith argument from the pack.  */
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    {
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  mark_used (parm);
+		  register_local_specialization (aps, parm);
+		}
+	      else
+		aps = retrieve_local_specialization (parm);
+	    }
+	  else
+	    {
+	      int idx, level;
+	      template_parm_level_and_index (parm, &level, &idx);
+
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  /* Update the corresponding argument.  */
+		  TMPL_ARG (args, level, idx) = aps;
+		}
+	      else
+		/* Re-use the ARGUMENT_PACK_SELECT.  */
+		aps = TMPL_ARG (args, level, idx);
+	    }
+	  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+	}
+    }
+
+  /* Substitute into the PATTERN with the (possibly altered)
+     arguments.  */
+  if (!TYPE_P (pattern))
+    t = tsubst_expr (pattern, args, complain, in_decl,
+		     /*integral_constant_expression_p=*/false);
+  else
+    t = tsubst (pattern, args, complain, in_decl);
+
+  /*  If the Ith argument pack element is a pack expansion, then
+      the Ith element resulting from the substituting is going to
+      be a pack expansion as well.  */
+  if (has_bare_parameter_packs (t))
+    t = make_pack_expansion (t);
+
+  return t;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9106,9 +9373,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 {
   tree pattern;
   tree pack, packs = NULL_TREE;
-  bool unsubstituted_packs = false;
-  bool real_packs = false;
-  int missing_level = 0;
   int i, len = -1;
   tree result;
   struct pointer_map_t *saved_local_specializations = NULL;
@@ -9187,14 +9451,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	  return result;
 	}
 
-      if (arg_from_parm_pack_p (arg_pack, parm_pack))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would find
-	   in the implicit typedef of a class inside the class itself.
-	   Consider this parameter "unsubstituted", so that we will
-	   maintain the outer pack expansion.  */
-	arg_pack = NULL_TREE;
-          
       if (arg_pack)
         {
           int my_len = 
@@ -9221,77 +9477,19 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
                        pattern);
               return error_mark_node;
             }
-
-	  if (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-	      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
-						 0)))
-	    /* This isn't a real argument pack yet.  */;
-	  else
-	    real_packs = true;
-
-          /* Keep track of the parameter packs and their corresponding
-             argument packs.  */
-          packs = tree_cons (parm_pack, arg_pack, packs);
-          TREE_TYPE (packs) = orig_arg;
         }
-      else
-	{
-	  /* We can't substitute for this parameter pack.  We use a flag as
-	     well as the missing_level counter because function parameter
-	     packs don't have a level.  */
-	  unsubstituted_packs = true;
-	  if (!missing_level || missing_level > level)
-	    missing_level = level;
-	}
-    }
-
-  /* We cannot expand this expansion expression, because we don't have
-     all of the argument packs we need.  */
-  if (unsubstituted_packs)
-    {
-      if (real_packs)
-	{
-	  /* We got some full packs, but we can't substitute them in until we
-	     have values for all the packs.  So remember these until then.  */
-	  tree save_args;
-
-	  t = make_pack_expansion (pattern);
-
-	  /* The call to add_to_template_args above assumes no overlap
-	     between saved args and new args, so prune away any fake
-	     args, i.e. those that satisfied arg_from_parm_pack_p above.  */
-	  if (missing_level && levels >= missing_level)
-	    {
-	      gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
-			  && missing_level > 1);
-	      TREE_VEC_LENGTH (args) = missing_level - 1;
-	      save_args = copy_node (args);
-	      TREE_VEC_LENGTH (args) = levels;
-	    }
-	  else
-	    save_args = args;
 
-	  PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
-	}
-      else
-	{
-	  /* There were no real arguments, we're just replacing a parameter
-	     pack with another version of itself. Substitute into the
-	     pattern and return a PACK_EXPANSION_*. The caller will need to
-	     deal with that.  */
-	  if (TREE_CODE (t) == EXPR_PACK_EXPANSION)
-	    t = tsubst_expr (pattern, args, complain, in_decl,
-			     /*integral_constant_expression_p=*/false);
-	  else
-	    t = tsubst (pattern, args, complain, in_decl);
-	  t = make_pack_expansion (t);
-	}
-      return t;
+      /* Keep track of the parameter packs and their corresponding
+	 argument packs.  */
+      packs = tree_cons (parm_pack, arg_pack, packs);
+      TREE_TYPE (packs) = orig_arg;
     }
 
-  /* We could not find any argument packs that work.  */
+  /* We could not find any argument packs that work, so we'll just
+     return an unsubstituted pack expansion.  The caller must be
+     prepared to deal with this.  */
   if (len < 0)
-    return error_mark_node;
+    len = 1;
 
   if (need_local_specializations)
     {
@@ -9306,60 +9504,27 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   /* For each argument in each argument pack, substitute into the
      pattern.  */
   result = make_tree_vec (len);
-  for (i = 0; i < len; ++i)
+  if (len > 0)
     {
-      /* For parameter pack, change the substitution of the parameter
-         pack to the ith argument in its argument pack, then expand
-         the pattern.  */
-      for (pack = packs; pack; pack = TREE_CHAIN (pack))
-        {
-          tree parm = TREE_PURPOSE (pack);
-	  tree arg;
-
-	  /* Select the Ith argument from the pack.  */
-          if (TREE_CODE (parm) == PARM_DECL)
-            {
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  mark_used (parm);
-		  register_local_specialization (arg, parm);
-		}
-	      else
-		arg = retrieve_local_specialization (parm);
-            }
-          else
-            {
-              int idx, level;
-              template_parm_level_and_index (parm, &level, &idx);
-
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
-		}
-	      else
-		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
-            }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
-        }
-
-      /* Substitute into the PATTERN with the altered arguments.  */
-      if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
-      else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
-
-      if (TREE_VEC_ELT (result, i) == error_mark_node)
+      for (i = 0; i < len; ++i)
 	{
-	  result = error_mark_node;
-	  break;
+	  bool yield_no_list_p = false;
+	  t = gen_elem_of_pack_expansion_instantiation (pattern, packs, i,
+							&yield_no_list_p,
+							args, complain,
+							in_decl);
+	  if (yield_no_list_p)
+	    {
+	      result = t;
+	      break;
+	    }
+
+	  TREE_VEC_ELT (result, i) = t;
+      	  if (t == error_mark_node)
+	    {
+	      result = error_mark_node;
+	      break;
+	    }
 	}
     }
 
@@ -9374,6 +9539,10 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       else
         {
           int idx, level;
+
+	  if (TREE_VALUE (pack) == NULL_TREE)
+	    continue;
+
           template_parm_level_and_index (parm, &level, &idx);
           
           /* Update the corresponding argument.  */
@@ -11102,8 +11271,24 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    arg = TMPL_ARG (args, level, idx);
 
 	    if (arg && TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-	      /* See through ARGUMENT_PACK_SELECT arguments. */
-	      arg = ARGUMENT_PACK_SELECT_ARG (arg);
+	      {
+		/* See through ARGUMENT_PACK_SELECT arguments. */
+		arg = ARGUMENT_PACK_SELECT_ARG (arg);
+		/* If the selected argument is an expansion E, that most
+		   likely means we were called from
+		   gen_elem_of_pack_expansion_instantiation during the
+		   substituting of pack an argument pack (which Ith
+		   element is a pack expansion, where I is
+		   ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
+		   In this case, the Ith element resulting from this
+		   substituting is going to be a pack expansion, which
+		   pattern is the pattern of E.  Let's return the
+		   pattern of E, and
+		   gen_elem_of_pack_expansion_instantiation will
+		   build the resulting pack expansion from it.  */
+		if (PACK_EXPANSION_P (arg))
+		  arg = PACK_EXPANSION_PATTERN (arg);
+	      }
 	  }
 
 	if (arg == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..a1c64f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
-- 
1.7.11.7



-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

On 12/11/2012 10:55 AM, Dodji Seketeli wrote:
> Oops, it seems I was too hasty in trying to do away with the
> instantiation_yields_no_list_p parameter to
> gen_elem_of_pack_expansion_instantiation.

I still think that the elem function should just deal with the single 
element case; the cases where we can't do a piecewise substitution 
should be handled outside the elem function.

>> I suppose we should keep this for has_empty_arg, but I'd like to do
>> away with special handling of the arg_from_parm_pack case if possible,
>> as it's a lot of extra complexity.
>
> OK, done.

Looks like you only removed the check from that one if; I'd like to do 
away with the arg_from_parm_pack function entirely if we can.

Jason

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 12/11/2012 10:55 AM, Dodji Seketeli wrote:
>> Oops, it seems I was too hasty in trying to do away with the
>> instantiation_yields_no_list_p parameter to
>> gen_elem_of_pack_expansion_instantiation.
>
> I still think that the elem function should just deal with the single
> element case; the cases where we can't do a piecewise substitution
> should be handled outside the elem function.

Like in the patch below?

>
>>> I suppose we should keep this for has_empty_arg, but I'd like to do
>>> away with special handling of the arg_from_parm_pack case if possible,
>>> as it's a lot of extra complexity.
>>
>> OK, done.
>
> Looks like you only removed the check from that one if; I'd like to do
> away with the arg_from_parm_pack function entirely if we can.

OK.  The below should hopefully approach what you have in mind.


Tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* pt.c (argument_pack_element_is_expansion_p)
	(make_argument_pack_select, scan_parm_packs_and_args)
	(gen_elem_of_pack_expansion_instantiation): New static functions.
	(has_bare_parameter_packs): Factorized out of ...
	(check_for_bare_parameter_packs): ... here.
	(tsubst): When looking through an ARGUMENT_PACK_SELECT tree node,
	look through the possibly resulting pack expansion as well.
	(tsubst_pack_expansion): Now this function is clearly organized in
	two parts: a part that maps each parameter pack with its matching
	argument pack, and a part that walks that map to build the result
	of substituting each element of the argument packs into the
	parameter packs.  Use gen_elem_of_pack_expansion_instantiation for
	the latter part.
	(arg_from_parm_pack_p): Remove this for it's not used by
	tsubst_pack_expansion anymore.

gcc/testsuite/

	* g++.dg/cpp0x/variadic139.C: New test.
	* g++.dg/cpp0x/variadic140.C: Likewise.
	* g++.dg/cpp0x/variadic141.C: Likewise.
---
 gcc/cp/pt.c                              | 487 ++++++++++++++++++++-----------
 gcc/testsuite/g++.dg/cpp0x/variadic139.C |  14 +
 gcc/testsuite/g++.dg/cpp0x/variadic140.C |  22 ++
 gcc/testsuite/g++.dg/cpp0x/variadic141.C |  22 ++
 4 files changed, 370 insertions(+), 175 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ecb013e..14914d5 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -201,7 +201,6 @@ static void append_type_to_template_for_access_check_1 (tree, tree, tree,
 static tree listify (tree);
 static tree listify_autos (tree, tree);
 static tree template_parm_to_arg (tree t);
-static bool arg_from_parm_pack_p (tree, tree);
 static tree current_template_args (void);
 static tree tsubst_template_parm (tree, tree, tsubst_flags_t);
 static tree instantiate_alias_template (tree, tree, tsubst_flags_t);
@@ -3308,6 +3307,29 @@ make_pack_expansion (tree arg)
   return result;
 }
 
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+  tree parameter_packs = NULL_TREE;
+  struct find_parameter_pack_data ppd;
+
+  if (!processing_template_decl || !t || t == error_mark_node)
+    return NULL_TREE;
+
+  if (TREE_CODE (t) == TYPE_DECL)
+    t = TREE_TYPE (t);
+
+  ppd.parameter_packs = &parameter_packs;
+  ppd.visited = pointer_set_create ();
+  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
+  pointer_set_destroy (ppd.visited);
+
+  return parameter_packs;
+}
+
 /* Checks T for any "bare" parameter packs, which have not yet been
    expanded, and issues an error if any are found. This operation can
    only be done on full expressions or types (e.g., an expression
@@ -3325,19 +3347,7 @@ make_pack_expansion (tree arg)
 bool 
 check_for_bare_parameter_packs (tree t)
 {
-  tree parameter_packs = NULL_TREE;
-  struct find_parameter_pack_data ppd;
-
-  if (!processing_template_decl || !t || t == error_mark_node)
-    return false;
-
-  if (TREE_CODE (t) == TYPE_DECL)
-    t = TREE_TYPE (t);
-
-  ppd.parameter_packs = &parameter_packs;
-  ppd.visited = pointer_set_create ();
-  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
-  pointer_set_destroy (ppd.visited);
+  tree parameter_packs = has_bare_parameter_packs (t);
 
   if (parameter_packs) 
     {
@@ -3812,42 +3822,6 @@ template_parm_to_arg (tree t)
   return t;
 }
 
-/* This function returns TRUE if PARM_PACK is a template parameter
-   pack and if ARG_PACK is what template_parm_to_arg returned when
-   passed PARM_PACK.  */
-
-static bool
-arg_from_parm_pack_p (tree arg_pack, tree parm_pack)
-{
-  /* For clarity in the comments below let's use the representation
-     argument_pack<elements>' to denote an argument pack and its
-     elements.
-
-     In the 'if' block below, we want to detect cases where
-     ARG_PACK is argument_pack<PARM_PACK...>.  I.e, we want to
-     check if ARG_PACK is an argument pack which sole element is
-     the expansion of PARM_PACK.  That argument pack is typically
-     created by template_parm_to_arg when passed a parameter
-     pack.  */
-
-  if (arg_pack
-      && TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0)))
-    {
-      tree expansion = TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0);
-      tree pattern = PACK_EXPANSION_PATTERN (expansion);
-      if ((TYPE_P (pattern) && same_type_p (pattern, parm_pack))
-	  || (!TYPE_P (pattern) && cp_tree_equal (parm_pack, pattern)))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would
-	   find in the implicit typedef of a class inside the
-	   class itself.  Consider this parameter "unsubstituted",
-	   so that we will maintain the outer pack expansion.  */
-	return true;
-    }
-  return false;
-}
-
 /* Given a set of template parameters, return them as a set of template
    arguments.  The template parameters are represented as a TREE_VEC, in
    the form documented in cp-tree.h for template arguments.  */
@@ -9095,6 +9069,218 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is a
+   pack expansion.  */
+
+static bool
+argument_pack_element_is_expansion_p (tree arg_pack, int i)
+{
+  tree vec = ARGUMENT_PACK_ARGS (arg_pack);
+  if (i >= TREE_VEC_LENGTH (vec))
+    return false;
+  return PACK_EXPANSION_P (TREE_VEC_ELT (vec, i));
+}
+
+
+/* Creates and return an ARGUMENT_PACK_SELECT tree node.  */
+
+static tree
+make_argument_pack_select (tree arg_pack, unsigned index)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+
+  return aps;
+}
+
+/*  This is a subroutine of tsubst_pack_expansion.
+
+    For a given INDEX of an argument pack, this function scans the
+    list of parameter packs PARM_PACKS and reports some interesting
+    properties about them.  Note that PARM_PACKS must be an instance of
+    TREE_LIST where TREE_PURPOSE is a parameter pack, and TREE_VALUE
+    is the argument for that pack.
+
+    The properties returned about the scanned parameter packs
+    are the following:
+
+	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
+	  pack has an argument that is an expansion.
+	- HAS_NON_EXPANSION_ARG_P: Set to TRUE iff at least one
+	  parameter pack has an argument that is not an expansion.
+	- USE_PACK_EXPANSION_EXTRA_P: Set to TRUE iff the
+	  tsubst_pack_expansion cannot build the final substitution
+	  piecewise by calling
+	  gen_elem_of_pack_expansion_instantiation, and so must return
+	  an unsubstituted pack expansion using the
+	  PACK_EXPANSION_EXTRA mecanism.
+	- HAS_EMPTY_ARG_P: Set to TRUE iff at least one parameter pack has
+	  no argument.
+	- ARG_FROM_PACK_LEVEL_TO_PRUNE: is non-zero iff the argument of
+	  the parameter pack is the result of calling template_parm_to_arg
+	  on the pack.  In that case, this is set to the parameter level
+	  of that pack.  */
+
+static void
+scan_parm_packs_and_args (tree parm_packs,
+			  unsigned index,
+			  bool *use_pack_expansion_extra_p,
+			  bool *has_empty_arg_p,
+			  int *arg_from_pack_level_to_prune)
+{
+
+  bool has_expansion_arg = false, has_non_expansion_arg = false;
+
+  if (parm_packs == NULL_TREE)
+    {
+      if (has_empty_arg_p)
+	*has_empty_arg_p = true;
+      return ;
+    }
+
+  for (tree parm_pack = parm_packs;
+       parm_pack;
+       parm_pack = TREE_CHAIN (parm_pack))
+    {
+      tree arg = TREE_VALUE (parm_pack);
+      tree parm = TREE_PURPOSE (parm_pack);
+
+      if (arg == NULL_TREE)
+	{
+	  if (has_empty_arg_p)
+	    *has_empty_arg_p = true;
+	  continue;
+	}
+
+      if (argument_pack_element_is_expansion_p (arg, index))
+	{
+	  has_expansion_arg = true;
+
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    parm = DECL_INITIAL (parm);
+
+	  int level = 0, idx = 0;
+	  template_parm_level_and_index (parm, &level, &idx);
+
+	  if (*arg_from_pack_level_to_prune == 0
+	      || *arg_from_pack_level_to_prune > level)
+	    *arg_from_pack_level_to_prune = level;
+	}
+      else
+	has_non_expansion_arg = true;
+    }
+
+  if (use_pack_expansion_extra_p)
+    /* If one parameter pack has an expansion as argument and another
+       one doesn't or if on pack has an empty argument and another
+       other doesn't, then tsubst_pack_expansion cannot perform the
+       substitution and need to fall back on the PACK_EXPANSION_EXTRA
+       mechanism.  */
+    if ((has_expansion_arg && has_non_expansion_arg)
+	|| (*has_empty_arg_p && (has_expansion_arg || has_non_expansion_arg)))
+      *use_pack_expansion_extra_p = true;
+}
+
+/* [temp.variadic]/6 says that:
+
+       The instantiation of a pack expansion [...]
+       produces a list E1,E2, ..., En, where N is the number of elements
+       in the pack expansion parameters.
+
+   This subroutine of tsubst_pack_expansion produces one of these Ei.
+
+   PATTERN is the pattern of the pack expansion.  PARM_PACKS is a
+   TREE_LIST in which each TREE_PURPOSE is a parameter pack of
+   PATTERN, and each TREE_VALUE is its corresponding argument pack.
+   HAS_EMPTY_ARG should be set to TRUE if any of the parameter packs
+   in PARM_PACKS has an empty argument.  INDEX is the index 'i' of the
+   element Ei to produce.  ARGS, COMPLAIN, and IN_DECL are the same
+   parameters as for the tsubst_pack_expansion function.
+
+   The function returns the resulting Ei upon successful completion,
+   or error_mark_node.
+
+   Note that this function possibly modifies the ARGS parameter, so
+   it's the responsibility of the caller to restore it.  */
+
+static tree
+gen_elem_of_pack_expansion_instantiation (tree pattern,
+					  tree parm_packs,
+					  bool has_empty_args,
+					  unsigned index,
+					  tree args /* This parm gets
+						       modified.  */,
+					  tsubst_flags_t complain,
+					  tree in_decl)
+{
+  tree t;
+
+  /* If we have empty arguments ... */
+  if (has_empty_args)
+    /* ... we just return a pack expansion which pattern is PATTERN
+       into which ARGS has been substituted.  */;
+  else
+    {
+      /* For each parameter pack, change the substitution of the parameter
+	 pack to the ith argument in its argument pack, then expand the
+	 pattern.  */
+      for (tree pack = parm_packs; pack; pack = TREE_CHAIN (pack))
+	{
+	  tree parm = TREE_PURPOSE (pack);
+	  tree arg_pack = TREE_VALUE (pack);
+	  tree aps;			/* instance of ARGUMENT_PACK_SELECT
+					   tree.  */
+
+	  /* Select the Ith argument from the pack.  */
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    {
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  mark_used (parm);
+		  register_local_specialization (aps, parm);
+		}
+	      else
+		aps = retrieve_local_specialization (parm);
+	    }
+	  else
+	    {
+	      int idx, level;
+	      template_parm_level_and_index (parm, &level, &idx);
+
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  /* Update the corresponding argument.  */
+		  TMPL_ARG (args, level, idx) = aps;
+		}
+	      else
+		/* Re-use the ARGUMENT_PACK_SELECT.  */
+		aps = TMPL_ARG (args, level, idx);
+	    }
+	  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+	}
+    }
+
+  /* Substitute into the PATTERN with the (possibly altered)
+     arguments.  */
+  if (!TYPE_P (pattern))
+    t = tsubst_expr (pattern, args, complain, in_decl,
+		     /*integral_constant_expression_p=*/false);
+  else
+    t = tsubst (pattern, args, complain, in_decl);
+
+  /*  If the Ith argument pack element is a pack expansion, then
+      the Ith element resulting from the substituting is going to
+      be a pack expansion as well.  */
+  if (has_bare_parameter_packs (t))
+    t = make_pack_expansion (t);
+
+  return t;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9106,9 +9292,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 {
   tree pattern;
   tree pack, packs = NULL_TREE;
-  bool unsubstituted_packs = false;
-  bool real_packs = false;
-  int missing_level = 0;
   int i, len = -1;
   tree result;
   struct pointer_map_t *saved_local_specializations = NULL;
@@ -9187,14 +9370,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	  return result;
 	}
 
-      if (arg_from_parm_pack_p (arg_pack, parm_pack))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would find
-	   in the implicit typedef of a class inside the class itself.
-	   Consider this parameter "unsubstituted", so that we will
-	   maintain the outer pack expansion.  */
-	arg_pack = NULL_TREE;
-          
       if (arg_pack)
         {
           int my_len = 
@@ -9221,77 +9396,19 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
                        pattern);
               return error_mark_node;
             }
-
-	  if (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-	      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
-						 0)))
-	    /* This isn't a real argument pack yet.  */;
-	  else
-	    real_packs = true;
-
-          /* Keep track of the parameter packs and their corresponding
-             argument packs.  */
-          packs = tree_cons (parm_pack, arg_pack, packs);
-          TREE_TYPE (packs) = orig_arg;
         }
-      else
-	{
-	  /* We can't substitute for this parameter pack.  We use a flag as
-	     well as the missing_level counter because function parameter
-	     packs don't have a level.  */
-	  unsubstituted_packs = true;
-	  if (!missing_level || missing_level > level)
-	    missing_level = level;
-	}
-    }
-
-  /* We cannot expand this expansion expression, because we don't have
-     all of the argument packs we need.  */
-  if (unsubstituted_packs)
-    {
-      if (real_packs)
-	{
-	  /* We got some full packs, but we can't substitute them in until we
-	     have values for all the packs.  So remember these until then.  */
-	  tree save_args;
-
-	  t = make_pack_expansion (pattern);
 
-	  /* The call to add_to_template_args above assumes no overlap
-	     between saved args and new args, so prune away any fake
-	     args, i.e. those that satisfied arg_from_parm_pack_p above.  */
-	  if (missing_level && levels >= missing_level)
-	    {
-	      gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
-			  && missing_level > 1);
-	      TREE_VEC_LENGTH (args) = missing_level - 1;
-	      save_args = copy_node (args);
-	      TREE_VEC_LENGTH (args) = levels;
-	    }
-	  else
-	    save_args = args;
-
-	  PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
-	}
-      else
-	{
-	  /* There were no real arguments, we're just replacing a parameter
-	     pack with another version of itself. Substitute into the
-	     pattern and return a PACK_EXPANSION_*. The caller will need to
-	     deal with that.  */
-	  if (TREE_CODE (t) == EXPR_PACK_EXPANSION)
-	    t = tsubst_expr (pattern, args, complain, in_decl,
-			     /*integral_constant_expression_p=*/false);
-	  else
-	    t = tsubst (pattern, args, complain, in_decl);
-	  t = make_pack_expansion (t);
-	}
-      return t;
+      /* Keep track of the parameter packs and their corresponding
+	 argument packs.  */
+      packs = tree_cons (parm_pack, arg_pack, packs);
+      TREE_TYPE (packs) = orig_arg;
     }
 
-  /* We could not find any argument packs that work.  */
+  /* We could not find any argument packs that work, so we'll just
+     return an unsubstituted pack expansion.  The caller must be
+     prepared to deal with this.  */
   if (len < 0)
-    return error_mark_node;
+    len = 1;
 
   if (need_local_specializations)
     {
@@ -9306,60 +9423,60 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   /* For each argument in each argument pack, substitute into the
      pattern.  */
   result = make_tree_vec (len);
-  for (i = 0; i < len; ++i)
+  if (len > 0)
     {
-      /* For parameter pack, change the substitution of the parameter
-         pack to the ith argument in its argument pack, then expand
-         the pattern.  */
-      for (pack = packs; pack; pack = TREE_CHAIN (pack))
-        {
-          tree parm = TREE_PURPOSE (pack);
-	  tree arg;
+      for (i = 0; i < len; ++i)
+	{
+	  bool use_pack_expansion_extra = false, has_empty_args = false;
+	  int fake_arg_level_to_prune = 0;
 
-	  /* Select the Ith argument from the pack.  */
-          if (TREE_CODE (parm) == PARM_DECL)
-            {
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  mark_used (parm);
-		  register_local_specialization (arg, parm);
-		}
-	      else
-		arg = retrieve_local_specialization (parm);
-            }
-          else
-            {
-              int idx, level;
-              template_parm_level_and_index (parm, &level, &idx);
+	  scan_parm_packs_and_args (packs, i,
+				    &use_pack_expansion_extra,
+				    &has_empty_args,
+				    &fake_arg_level_to_prune);
 
-	      if (i == 0)
+	  if (use_pack_expansion_extra)
+	    {
+	      /* We got some full packs, but we can't substitute them
+		 in until we have values for all the packs.  So
+		 remember these until then.  */
+
+	      int levels = TMPL_ARGS_DEPTH (args);
+	      tree save_args;
+
+	      /* The call to add_to_template_args in
+		 tsubst_pack_expansion assumes no overlap between
+		 saved args and new args, so prune away any fake args,
+		 i.e, those resulting from template_parm_to_arg.  */
+	      if (fake_arg_level_to_prune != 0
+		  && levels >= fake_arg_level_to_prune)
 		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
+		  gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
+			      && fake_arg_level_to_prune > 1);
+		  TREE_VEC_LENGTH (args) = fake_arg_level_to_prune - 1;
+		  save_args = copy_node (args);
+		  TREE_VEC_LENGTH (args) = levels;
 		}
 	      else
-		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
-            }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
-        }
+		save_args = args;
 
-      /* Substitute into the PATTERN with the altered arguments.  */
-      if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
-      else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
+	      t = make_pack_expansion (pattern);
+	      PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
 
-      if (TREE_VEC_ELT (result, i) == error_mark_node)
-	{
-	  result = error_mark_node;
-	  break;
+	      result = t;
+	      break;
+	    }
+
+	  t = gen_elem_of_pack_expansion_instantiation (pattern, packs,
+							has_empty_args, i,
+							args, complain,
+							in_decl);
+	  TREE_VEC_ELT (result, i) = t;
+      	  if (t == error_mark_node)
+	    {
+	      result = error_mark_node;
+	      break;
+	    }
 	}
     }
 
@@ -9374,6 +9491,10 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       else
         {
           int idx, level;
+
+	  if (TREE_VALUE (pack) == NULL_TREE)
+	    continue;
+
           template_parm_level_and_index (parm, &level, &idx);
           
           /* Update the corresponding argument.  */
@@ -11102,8 +11223,24 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    arg = TMPL_ARG (args, level, idx);
 
 	    if (arg && TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-	      /* See through ARGUMENT_PACK_SELECT arguments. */
-	      arg = ARGUMENT_PACK_SELECT_ARG (arg);
+	      {
+		/* See through ARGUMENT_PACK_SELECT arguments. */
+		arg = ARGUMENT_PACK_SELECT_ARG (arg);
+		/* If the selected argument is an expansion E, that most
+		   likely means we were called from
+		   gen_elem_of_pack_expansion_instantiation during the
+		   substituting of pack an argument pack (which Ith
+		   element is a pack expansion, where I is
+		   ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
+		   In this case, the Ith element resulting from this
+		   substituting is going to be a pack expansion, which
+		   pattern is the pattern of E.  Let's return the
+		   pattern of E, and
+		   gen_elem_of_pack_expansion_instantiation will
+		   build the resulting pack expansion from it.  */
+		if (PACK_EXPANSION_P (arg))
+		  arg = PACK_EXPANSION_PATTERN (arg);
+	      }
 	  }
 
 	if (arg == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..a1c64f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

On 12/11/2012 04:09 PM, Dodji Seketeli wrote:
> OK.  The below should hopefully approach what you have in mind.

Thanks, that's closer to what I was thinking.  I'd also like to move the 
scan and PACK_EXPANSION_EXTRA_ARGS code back out of the loop.

Jason

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:


> I'd also like to move the scan and PACK_EXPANSION_EXTRA_ARGS code back
> out of the loop.

Like this?

Tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* pt.c (argument_pack_element_is_expansion_p)
	(make_argument_pack_select, scan_parm_packs_and_args)
	(gen_elem_of_pack_expansion_instantiation): New static functions.
	(has_bare_parameter_packs): Factorized out of ...
	(check_for_bare_parameter_packs): ... here.
	(tsubst): When looking through an ARGUMENT_PACK_SELECT tree node,
	look through the possibly resulting pack expansion as well.
	(tsubst_pack_expansion): Now this function is clearly organized in
	two parts: a part that maps each parameter pack with its matching
	argument pack, and a part that walks that map to build the result
	of substituting each element of the argument packs into the
	parameter packs.  Use gen_elem_of_pack_expansion_instantiation for
	the latter part.
	(arg_from_parm_pack_p): Remove this for it's not used by
	tsubst_pack_expansion anymore.

gcc/testsuite/

	* g++.dg/cpp0x/variadic139.C: New test.
	* g++.dg/cpp0x/variadic140.C: Likewise.
	* g++.dg/cpp0x/variadic141.C: Likewise.
---
 gcc/cp/pt.c                              | 485 ++++++++++++++++++++-----------
 gcc/testsuite/g++.dg/cpp0x/variadic139.C |  14 +
 gcc/testsuite/g++.dg/cpp0x/variadic140.C |  22 ++
 gcc/testsuite/g++.dg/cpp0x/variadic141.C |  22 ++
 4 files changed, 366 insertions(+), 177 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ecb013e..221323c 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -201,7 +201,6 @@ static void append_type_to_template_for_access_check_1 (tree, tree, tree,
 static tree listify (tree);
 static tree listify_autos (tree, tree);
 static tree template_parm_to_arg (tree t);
-static bool arg_from_parm_pack_p (tree, tree);
 static tree current_template_args (void);
 static tree tsubst_template_parm (tree, tree, tsubst_flags_t);
 static tree instantiate_alias_template (tree, tree, tsubst_flags_t);
@@ -3308,6 +3307,29 @@ make_pack_expansion (tree arg)
   return result;
 }
 
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+  tree parameter_packs = NULL_TREE;
+  struct find_parameter_pack_data ppd;
+
+  if (!processing_template_decl || !t || t == error_mark_node)
+    return NULL_TREE;
+
+  if (TREE_CODE (t) == TYPE_DECL)
+    t = TREE_TYPE (t);
+
+  ppd.parameter_packs = &parameter_packs;
+  ppd.visited = pointer_set_create ();
+  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
+  pointer_set_destroy (ppd.visited);
+
+  return parameter_packs;
+}
+
 /* Checks T for any "bare" parameter packs, which have not yet been
    expanded, and issues an error if any are found. This operation can
    only be done on full expressions or types (e.g., an expression
@@ -3325,19 +3347,7 @@ make_pack_expansion (tree arg)
 bool 
 check_for_bare_parameter_packs (tree t)
 {
-  tree parameter_packs = NULL_TREE;
-  struct find_parameter_pack_data ppd;
-
-  if (!processing_template_decl || !t || t == error_mark_node)
-    return false;
-
-  if (TREE_CODE (t) == TYPE_DECL)
-    t = TREE_TYPE (t);
-
-  ppd.parameter_packs = &parameter_packs;
-  ppd.visited = pointer_set_create ();
-  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
-  pointer_set_destroy (ppd.visited);
+  tree parameter_packs = has_bare_parameter_packs (t);
 
   if (parameter_packs) 
     {
@@ -3812,42 +3822,6 @@ template_parm_to_arg (tree t)
   return t;
 }
 
-/* This function returns TRUE if PARM_PACK is a template parameter
-   pack and if ARG_PACK is what template_parm_to_arg returned when
-   passed PARM_PACK.  */
-
-static bool
-arg_from_parm_pack_p (tree arg_pack, tree parm_pack)
-{
-  /* For clarity in the comments below let's use the representation
-     argument_pack<elements>' to denote an argument pack and its
-     elements.
-
-     In the 'if' block below, we want to detect cases where
-     ARG_PACK is argument_pack<PARM_PACK...>.  I.e, we want to
-     check if ARG_PACK is an argument pack which sole element is
-     the expansion of PARM_PACK.  That argument pack is typically
-     created by template_parm_to_arg when passed a parameter
-     pack.  */
-
-  if (arg_pack
-      && TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0)))
-    {
-      tree expansion = TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0);
-      tree pattern = PACK_EXPANSION_PATTERN (expansion);
-      if ((TYPE_P (pattern) && same_type_p (pattern, parm_pack))
-	  || (!TYPE_P (pattern) && cp_tree_equal (parm_pack, pattern)))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would
-	   find in the implicit typedef of a class inside the
-	   class itself.  Consider this parameter "unsubstituted",
-	   so that we will maintain the outer pack expansion.  */
-	return true;
-    }
-  return false;
-}
-
 /* Given a set of template parameters, return them as a set of template
    arguments.  The template parameters are represented as a TREE_VEC, in
    the form documented in cp-tree.h for template arguments.  */
@@ -9095,6 +9069,216 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is a
+   pack expansion.  */
+
+static bool
+argument_pack_element_is_expansion_p (tree arg_pack, int i)
+{
+  tree vec = ARGUMENT_PACK_ARGS (arg_pack);
+  if (i >= TREE_VEC_LENGTH (vec))
+    return false;
+  return PACK_EXPANSION_P (TREE_VEC_ELT (vec, i));
+}
+
+
+/* Creates and return an ARGUMENT_PACK_SELECT tree node.  */
+
+static tree
+make_argument_pack_select (tree arg_pack, unsigned index)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+
+  return aps;
+}
+
+/*  This is a subroutine of tsubst_pack_expansion.
+
+    It scans the list of parameter packs PARM_PACKS and reports some
+    interesting properties about them.  ARG_PACK_LEN is the length of
+    the argument packs.  Note that PARM_PACKS must be an instance of
+    TREE_LIST where TREE_PURPOSE is a parameter pack, and TREE_VALUE
+    is the argument for that pack.
+
+    The properties returned about the scanned parameter packs
+    are the following:
+
+	- HAS_EXPANSION_ARG_P: Set to TRUE iff at least one parameter
+	  pack has an argument that is an expansion.
+	- HAS_NON_EXPANSION_ARG_P: Set to TRUE iff at least one
+	  parameter pack has an argument that is not an expansion.
+	- USE_PACK_EXPANSION_EXTRA_P: Set to TRUE iff the
+	  tsubst_pack_expansion cannot build the final substitution
+	  piecewise by calling
+	  gen_elem_of_pack_expansion_instantiation, and so must return
+	  an unsubstituted pack expansion using the
+	  PACK_EXPANSION_EXTRA mecanism.
+	- HAS_EMPTY_ARG_P: Set to TRUE iff at least one parameter pack has
+	  no argument.
+	- FAKE_ARG_LEVEL_TO_PRUNE: is non-zero iff the argument of
+	  the parameter pack is e.g the result of calling template_parm_to_arg
+	  on the pack.  In that case, this is set to the parameter level
+	  of that pack.  */
+
+static void
+scan_parm_packs_and_args (tree parm_packs,
+			  int arg_pack_len,
+			  bool *use_pack_expansion_extra_p,
+			  bool *has_empty_arg_p,
+			  int *fake_arg_level_to_prune)
+{
+  if (parm_packs == NULL_TREE)
+    {
+      *has_empty_arg_p = true;
+      return ;
+    }
+
+  for (int i = 0 ; i < arg_pack_len; ++i)
+    {
+      bool has_expansion_arg = false, has_non_expansion_arg = false;
+
+      for (tree parm_pack = parm_packs;
+	   parm_pack;
+	   parm_pack = TREE_CHAIN (parm_pack))
+	{
+	  tree arg = TREE_VALUE (parm_pack);
+	  tree parm = TREE_PURPOSE (parm_pack);
+
+	  if (arg == NULL_TREE)
+	    {
+	      *has_empty_arg_p = true;
+	      continue;
+	    }
+
+	  if (argument_pack_element_is_expansion_p (arg, i))
+	    {
+	      has_expansion_arg = true;
+	      if (TREE_CODE (parm) == PARM_DECL)
+		parm = DECL_INITIAL (parm);
+
+	      int level = 0, idx = 0;
+	      template_parm_level_and_index (parm, &level, &idx);
+
+	      if (*fake_arg_level_to_prune == 0
+		  || *fake_arg_level_to_prune > level)
+		*fake_arg_level_to_prune = level;
+	    }
+	  else
+	    has_non_expansion_arg = true;
+	}
+
+      /* If one pack has an empty argument and another other doesn't,
+	 or if for a given index one parameter pack has an expansion
+	 as argument and another one doesn't, then
+	 tsubst_pack_expansion cannot perform the substitution and
+	 need to fall back on the PACK_EXPANSION_EXTRA mechanism.  */
+      if ((has_expansion_arg && has_non_expansion_arg)
+	  || (*has_empty_arg_p && (has_expansion_arg || has_non_expansion_arg)))
+	*use_pack_expansion_extra_p = true;
+    }
+}
+
+/* [temp.variadic]/6 says that:
+
+       The instantiation of a pack expansion [...]
+       produces a list E1,E2, ..., En, where N is the number of elements
+       in the pack expansion parameters.
+
+   This subroutine of tsubst_pack_expansion produces one of these Ei.
+
+   PATTERN is the pattern of the pack expansion.  PARM_PACKS is a
+   TREE_LIST in which each TREE_PURPOSE is a parameter pack of
+   PATTERN, and each TREE_VALUE is its corresponding argument pack.
+   HAS_EMPTY_ARG should be set to TRUE if any of the parameter packs
+   in PARM_PACKS has an empty argument.  INDEX is the index 'i' of the
+   element Ei to produce.  ARGS, COMPLAIN, and IN_DECL are the same
+   parameters as for the tsubst_pack_expansion function.
+
+   The function returns the resulting Ei upon successful completion,
+   or error_mark_node.
+
+   Note that this function possibly modifies the ARGS parameter, so
+   it's the responsibility of the caller to restore it.  */
+
+static tree
+gen_elem_of_pack_expansion_instantiation (tree pattern,
+					  tree parm_packs,
+					  bool has_empty_args,
+					  unsigned index,
+					  tree args /* This parm gets
+						       modified.  */,
+					  tsubst_flags_t complain,
+					  tree in_decl)
+{
+  tree t;
+
+  /* If we have empty arguments ... */
+  if (has_empty_args)
+    /* ... we just return a pack expansion which pattern is PATTERN
+       into which ARGS has been substituted.  */;
+  else
+    {
+      /* For each parameter pack, change the substitution of the parameter
+	 pack to the ith argument in its argument pack, then expand the
+	 pattern.  */
+      for (tree pack = parm_packs; pack; pack = TREE_CHAIN (pack))
+	{
+	  tree parm = TREE_PURPOSE (pack);
+	  tree arg_pack = TREE_VALUE (pack);
+	  tree aps;			/* instance of ARGUMENT_PACK_SELECT
+					   tree.  */
+
+	  /* Select the Ith argument from the pack.  */
+	  if (TREE_CODE (parm) == PARM_DECL)
+	    {
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  mark_used (parm);
+		  register_local_specialization (aps, parm);
+		}
+	      else
+		aps = retrieve_local_specialization (parm);
+	    }
+	  else
+	    {
+	      int idx, level;
+	      template_parm_level_and_index (parm, &level, &idx);
+
+	      if (index == 0)
+		{
+		  aps = make_argument_pack_select (arg_pack, index);
+		  /* Update the corresponding argument.  */
+		  TMPL_ARG (args, level, idx) = aps;
+		}
+	      else
+		/* Re-use the ARGUMENT_PACK_SELECT.  */
+		aps = TMPL_ARG (args, level, idx);
+	    }
+	  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+	}
+    }
+
+  /* Substitute into the PATTERN with the (possibly altered)
+     arguments.  */
+  if (!TYPE_P (pattern))
+    t = tsubst_expr (pattern, args, complain, in_decl,
+		     /*integral_constant_expression_p=*/false);
+  else
+    t = tsubst (pattern, args, complain, in_decl);
+
+  /*  If the Ith argument pack element is a pack expansion, then
+      the Ith element resulting from the substituting is going to
+      be a pack expansion as well.  */
+  if (has_bare_parameter_packs (t))
+    t = make_pack_expansion (t);
+
+  return t;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9106,9 +9290,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 {
   tree pattern;
   tree pack, packs = NULL_TREE;
-  bool unsubstituted_packs = false;
-  bool real_packs = false;
-  int missing_level = 0;
   int i, len = -1;
   tree result;
   struct pointer_map_t *saved_local_specializations = NULL;
@@ -9187,14 +9368,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	  return result;
 	}
 
-      if (arg_from_parm_pack_p (arg_pack, parm_pack))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would find
-	   in the implicit typedef of a class inside the class itself.
-	   Consider this parameter "unsubstituted", so that we will
-	   maintain the outer pack expansion.  */
-	arg_pack = NULL_TREE;
-          
       if (arg_pack)
         {
           int my_len = 
@@ -9221,77 +9394,55 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
                        pattern);
               return error_mark_node;
             }
-
-	  if (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-	      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
-						 0)))
-	    /* This isn't a real argument pack yet.  */;
-	  else
-	    real_packs = true;
-
-          /* Keep track of the parameter packs and their corresponding
-             argument packs.  */
-          packs = tree_cons (parm_pack, arg_pack, packs);
-          TREE_TYPE (packs) = orig_arg;
         }
-      else
-	{
-	  /* We can't substitute for this parameter pack.  We use a flag as
-	     well as the missing_level counter because function parameter
-	     packs don't have a level.  */
-	  unsubstituted_packs = true;
-	  if (!missing_level || missing_level > level)
-	    missing_level = level;
-	}
+
+      /* Keep track of the parameter packs and their corresponding
+	 argument packs.  */
+      packs = tree_cons (parm_pack, arg_pack, packs);
+      TREE_TYPE (packs) = orig_arg;
     }
 
-  /* We cannot expand this expansion expression, because we don't have
-     all of the argument packs we need.  */
-  if (unsubstituted_packs)
-    {
-      if (real_packs)
-	{
-	  /* We got some full packs, but we can't substitute them in until we
-	     have values for all the packs.  So remember these until then.  */
-	  tree save_args;
+  /* We could not find any argument packs that work, so we'll just
+     return an unsubstituted pack expansion.  The caller must be
+     prepared to deal with this.  */
+  if (len < 0)
+    len = 1;
 
-	  t = make_pack_expansion (pattern);
+  bool use_pack_expansion_extra = false, has_empty_args = false;
+  int fake_arg_level_to_prune = 0;
+  scan_parm_packs_and_args (packs, len, &use_pack_expansion_extra,
+			    &has_empty_args, &fake_arg_level_to_prune);
 
-	  /* The call to add_to_template_args above assumes no overlap
-	     between saved args and new args, so prune away any fake
-	     args, i.e. those that satisfied arg_from_parm_pack_p above.  */
-	  if (missing_level && levels >= missing_level)
-	    {
-	      gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
-			  && missing_level > 1);
-	      TREE_VEC_LENGTH (args) = missing_level - 1;
-	      save_args = copy_node (args);
-	      TREE_VEC_LENGTH (args) = levels;
-	    }
-	  else
-	    save_args = args;
+  if (use_pack_expansion_extra)
+    {
+      /* We got some full packs, but we can't substitute them
+	 in until we have values for all the packs.  So
+	 remember these until then.  */
 
-	  PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
-	}
-      else
+      int levels = TMPL_ARGS_DEPTH (args);
+      tree save_args;
+
+      /* The call to add_to_template_args in
+	 tsubst_pack_expansion assumes no overlap between
+	 saved args and new args, so prune away any fake args,
+	 i.e, those resulting from template_parm_to_arg.  */
+      if (fake_arg_level_to_prune != 0
+	  && levels >= fake_arg_level_to_prune)
 	{
-	  /* There were no real arguments, we're just replacing a parameter
-	     pack with another version of itself. Substitute into the
-	     pattern and return a PACK_EXPANSION_*. The caller will need to
-	     deal with that.  */
-	  if (TREE_CODE (t) == EXPR_PACK_EXPANSION)
-	    t = tsubst_expr (pattern, args, complain, in_decl,
-			     /*integral_constant_expression_p=*/false);
-	  else
-	    t = tsubst (pattern, args, complain, in_decl);
-	  t = make_pack_expansion (t);
+	  gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
+		      && fake_arg_level_to_prune > 1);
+	  TREE_VEC_LENGTH (args) = fake_arg_level_to_prune - 1;
+	  save_args = copy_node (args);
+	  TREE_VEC_LENGTH (args) = levels;
 	}
-      return t;
-    }
+      else
+	save_args = args;
 
-  /* We could not find any argument packs that work.  */
-  if (len < 0)
-    return error_mark_node;
+      result = make_pack_expansion (pattern);
+      PACK_EXPANSION_EXTRA_ARGS (result) = save_args;
+
+      return result;
+    }
 
   if (need_local_specializations)
     {
@@ -9306,60 +9457,20 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   /* For each argument in each argument pack, substitute into the
      pattern.  */
   result = make_tree_vec (len);
-  for (i = 0; i < len; ++i)
+  if (len > 0)
     {
-      /* For parameter pack, change the substitution of the parameter
-         pack to the ith argument in its argument pack, then expand
-         the pattern.  */
-      for (pack = packs; pack; pack = TREE_CHAIN (pack))
-        {
-          tree parm = TREE_PURPOSE (pack);
-	  tree arg;
-
-	  /* Select the Ith argument from the pack.  */
-          if (TREE_CODE (parm) == PARM_DECL)
-            {
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  mark_used (parm);
-		  register_local_specialization (arg, parm);
-		}
-	      else
-		arg = retrieve_local_specialization (parm);
-            }
-          else
-            {
-              int idx, level;
-              template_parm_level_and_index (parm, &level, &idx);
-
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
-		}
-	      else
-		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
-            }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
-        }
-
-      /* Substitute into the PATTERN with the altered arguments.  */
-      if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
-      else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
-
-      if (TREE_VEC_ELT (result, i) == error_mark_node)
+      for (i = 0; i < len; ++i)
 	{
-	  result = error_mark_node;
-	  break;
+	  t = gen_elem_of_pack_expansion_instantiation (pattern, packs,
+							has_empty_args, i,
+							args, complain,
+							in_decl);
+	  TREE_VEC_ELT (result, i) = t;
+      	  if (t == error_mark_node)
+	    {
+	      result = error_mark_node;
+	      break;
+	    }
 	}
     }
 
@@ -9374,6 +9485,10 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       else
         {
           int idx, level;
+
+	  if (TREE_VALUE (pack) == NULL_TREE)
+	    continue;
+
           template_parm_level_and_index (parm, &level, &idx);
           
           /* Update the corresponding argument.  */
@@ -11102,8 +11217,24 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    arg = TMPL_ARG (args, level, idx);
 
 	    if (arg && TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-	      /* See through ARGUMENT_PACK_SELECT arguments. */
-	      arg = ARGUMENT_PACK_SELECT_ARG (arg);
+	      {
+		/* See through ARGUMENT_PACK_SELECT arguments. */
+		arg = ARGUMENT_PACK_SELECT_ARG (arg);
+		/* If the selected argument is an expansion E, that most
+		   likely means we were called from
+		   gen_elem_of_pack_expansion_instantiation during the
+		   substituting of pack an argument pack (which Ith
+		   element is a pack expansion, where I is
+		   ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
+		   In this case, the Ith element resulting from this
+		   substituting is going to be a pack expansion, which
+		   pattern is the pattern of E.  Let's return the
+		   pattern of E, and
+		   gen_elem_of_pack_expansion_instantiation will
+		   build the resulting pack expansion from it.  */
+		if (PACK_EXPANSION_P (arg))
+		  arg = PACK_EXPANSION_PATTERN (arg);
+	      }
 	  }
 
 	if (arg == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..a1c64f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

On 12/12/2012 08:01 AM, Dodji Seketeli wrote:
> Jason Merrill <jason@redhat.com> writes:
>> I'd also like to move the scan and PACK_EXPANSION_EXTRA_ARGS code back
>> out of the loop.
>
> Like this?

Let's put this scanning back in the loop over the packs that we already 
have in the main function.

> +	  if (argument_pack_element_is_expansion_p (arg, i))
> +	    {
> +	      has_expansion_arg = true;
> +	      if (TREE_CODE (parm) == PARM_DECL)
> +		parm = DECL_INITIAL (parm);
> +
> +	      int level = 0, idx = 0;
> +	      template_parm_level_and_index (parm, &level, &idx);
> +
> +	      if (*fake_arg_level_to_prune == 0
> +		  || *fake_arg_level_to_prune > level)
> +		*fake_arg_level_to_prune = level;
> +	    }

This is now assuming that any expansion we see is a fake arg, which 
isn't the case for the testcase in this PR.  Do we really need to 
recognize fake args rather than treating them the same as real args?

> +      /* If one pack has an empty argument and another other doesn't,
> +	 or if for a given index one parameter pack has an expansion
> +	 as argument and another one doesn't, then

This seems to assume that any expansions in different argument packs 
will end up expanding to the same number of elements.  I'm not sure 
that's a valid assumption; I think once we see an expansion, any 
non-expansion arguments after that index are also problematic.

> +  /* If we have empty arguments ... */
> +  if (has_empty_args)
> +    /* ... we just return a pack expansion which pattern is PATTERN
> +       into which ARGS has been substituted.  */;

I want to still handle this before the elements loop, too.

Jason

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

How about the below?

    gcc/cp/
    
    	* pt.c (argument_pack_element_is_expansion_p)
    	(make_argument_pack_select, use_pack_expansion_extra_args_p)
    	(gen_elem_of_pack_expansion_instantiation): New static functions.
    	(has_bare_parameter_packs): Factorized out of ...
    	(check_for_bare_parameter_packs): ... here.
    	(tsubst): When looking through an ARGUMENT_PACK_SELECT tree node,
    	look through the possibly resulting pack expansion as well.
    	(tsubst_pack_expansion): Use use_pack_expansion_extra_p to
    	generalize when to use the PACK_EXPANSION_EXTRA_ARGS mechanism.
    	Use gen_elem_of_pack_expansion_instantiation to build the
    	instantiation piece-wise.  Don't use arg_from_parm_pack_p anymore,
    	as gen_elem_of_pack_expansion_instantiation and the change in
    	tsubst above generalize this particular case.
    	(arg_from_parm_pack_p): Remove this for it's not used by
    	tsubst_pack_expansion anymore.
    
    gcc/testsuite/
    
    	* g++.dg/cpp0x/variadic139.C: New test.
    	* g++.dg/cpp0x/variadic140.C: Likewise.
    	* g++.dg/cpp0x/variadic141.C: Likewise.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index ecb013e..313f7a4 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -201,7 +201,6 @@ static void append_type_to_template_for_access_check_1 (tree, tree, tree,
 static tree listify (tree);
 static tree listify_autos (tree, tree);
 static tree template_parm_to_arg (tree t);
-static bool arg_from_parm_pack_p (tree, tree);
 static tree current_template_args (void);
 static tree tsubst_template_parm (tree, tree, tsubst_flags_t);
 static tree instantiate_alias_template (tree, tree, tsubst_flags_t);
@@ -3308,6 +3307,29 @@ make_pack_expansion (tree arg)
   return result;
 }
 
+/* Return NULL_TREE iff T contains *NO* unexpanded parameter packs.
+   Return the TREE_LIST of unexpanded parameter packs otherwise.  */
+
+static tree
+has_bare_parameter_packs (tree t)
+{
+  tree parameter_packs = NULL_TREE;
+  struct find_parameter_pack_data ppd;
+
+  if (!processing_template_decl || !t || t == error_mark_node)
+    return NULL_TREE;
+
+  if (TREE_CODE (t) == TYPE_DECL)
+    t = TREE_TYPE (t);
+
+  ppd.parameter_packs = &parameter_packs;
+  ppd.visited = pointer_set_create ();
+  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
+  pointer_set_destroy (ppd.visited);
+
+  return parameter_packs;
+}
+
 /* Checks T for any "bare" parameter packs, which have not yet been
    expanded, and issues an error if any are found. This operation can
    only be done on full expressions or types (e.g., an expression
@@ -3325,19 +3347,7 @@ make_pack_expansion (tree arg)
 bool 
 check_for_bare_parameter_packs (tree t)
 {
-  tree parameter_packs = NULL_TREE;
-  struct find_parameter_pack_data ppd;
-
-  if (!processing_template_decl || !t || t == error_mark_node)
-    return false;
-
-  if (TREE_CODE (t) == TYPE_DECL)
-    t = TREE_TYPE (t);
-
-  ppd.parameter_packs = &parameter_packs;
-  ppd.visited = pointer_set_create ();
-  cp_walk_tree (&t, &find_parameter_packs_r, &ppd, ppd.visited);
-  pointer_set_destroy (ppd.visited);
+  tree parameter_packs = has_bare_parameter_packs (t);
 
   if (parameter_packs) 
     {
@@ -3812,42 +3822,6 @@ template_parm_to_arg (tree t)
   return t;
 }
 
-/* This function returns TRUE if PARM_PACK is a template parameter
-   pack and if ARG_PACK is what template_parm_to_arg returned when
-   passed PARM_PACK.  */
-
-static bool
-arg_from_parm_pack_p (tree arg_pack, tree parm_pack)
-{
-  /* For clarity in the comments below let's use the representation
-     argument_pack<elements>' to denote an argument pack and its
-     elements.
-
-     In the 'if' block below, we want to detect cases where
-     ARG_PACK is argument_pack<PARM_PACK...>.  I.e, we want to
-     check if ARG_PACK is an argument pack which sole element is
-     the expansion of PARM_PACK.  That argument pack is typically
-     created by template_parm_to_arg when passed a parameter
-     pack.  */
-
-  if (arg_pack
-      && TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0)))
-    {
-      tree expansion = TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0);
-      tree pattern = PACK_EXPANSION_PATTERN (expansion);
-      if ((TYPE_P (pattern) && same_type_p (pattern, parm_pack))
-	  || (!TYPE_P (pattern) && cp_tree_equal (parm_pack, pattern)))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would
-	   find in the implicit typedef of a class inside the
-	   class itself.  Consider this parameter "unsubstituted",
-	   so that we will maintain the outer pack expansion.  */
-	return true;
-    }
-  return false;
-}
-
 /* Given a set of template parameters, return them as a set of template
    arguments.  The template parameters are represented as a TREE_VEC, in
    the form documented in cp-tree.h for template arguments.  */
@@ -9095,6 +9069,165 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is a
+   pack expansion.  */
+
+static bool
+argument_pack_element_is_expansion_p (tree arg_pack, int i)
+{
+  tree vec = ARGUMENT_PACK_ARGS (arg_pack);
+  if (i >= TREE_VEC_LENGTH (vec))
+    return false;
+  return PACK_EXPANSION_P (TREE_VEC_ELT (vec, i));
+}
+
+
+/* Creates and return an ARGUMENT_PACK_SELECT tree node.  */
+
+static tree
+make_argument_pack_select (tree arg_pack, unsigned index)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+
+  return aps;
+}
+
+/*  This is a subroutine of tsubst_pack_expansion.
+
+    It returns TRUE if we need to use the PACK_EXPANSION_EXTRA_ARGS
+    mechanism to store the (non complete list of) arguments of the
+    substitution and return a non substituted pack expansion, in order
+    to wait for when we have enough arguments to really perform the
+    substitution.  */
+
+static bool
+use_pack_expansion_extra_args_p (tree parm_packs,
+				 int arg_pack_len,
+				 bool has_empty_arg)
+{
+  if (parm_packs == NULL_TREE)
+    return false;
+
+  bool has_expansion_arg = false;
+  for (int i = 0 ; i < arg_pack_len; ++i)
+    {
+      bool has_non_expansion_arg = false;
+      for (tree parm_pack = parm_packs;
+	   parm_pack;
+	   parm_pack = TREE_CHAIN (parm_pack))
+	{
+	  tree arg = TREE_VALUE (parm_pack);
+
+	  if (argument_pack_element_is_expansion_p (arg, i))
+	    has_expansion_arg = true;
+	  else
+	    has_non_expansion_arg = true;
+	}
+
+      /* If one pack has an expansion and another pack has a normal
+	 argument or if one pack has an empty argument another one
+	 hasn't then tsubst_pack_expansion cannot perform the
+	 substitution and need to fall back on the
+	 PACK_EXPANSION_EXTRA mechanism.  */
+      if ((has_expansion_arg && has_non_expansion_arg)
+	  || (has_empty_arg && (has_expansion_arg || has_non_expansion_arg)))
+	return true;
+    }
+  return false;
+}
+
+/* [temp.variadic]/6 says that:
+
+       The instantiation of a pack expansion [...]
+       produces a list E1,E2, ..., En, where N is the number of elements
+       in the pack expansion parameters.
+
+   This subroutine of tsubst_pack_expansion produces one of these Ei.
+
+   PATTERN is the pattern of the pack expansion.  PARM_PACKS is a
+   TREE_LIST in which each TREE_PURPOSE is a parameter pack of
+   PATTERN, and each TREE_VALUE is its corresponding argument pack.
+   INDEX is the index 'i' of the element Ei to produce.  ARGS,
+   COMPLAIN, and IN_DECL are the same parameters as for the
+   tsubst_pack_expansion function.
+
+   The function returns the resulting Ei upon successful completion,
+   or error_mark_node.
+
+   Note that this function possibly modifies the ARGS parameter, so
+   it's the responsibility of the caller to restore it.  */
+
+static tree
+gen_elem_of_pack_expansion_instantiation (tree pattern,
+					  tree parm_packs,
+					  unsigned index,
+					  tree args /* This parm gets
+						       modified.  */,
+					  tsubst_flags_t complain,
+					  tree in_decl)
+{
+  tree t;
+
+  /* For each parameter pack, change the substitution of the parameter
+     pack to the ith argument in its argument pack, then expand the
+     pattern.  */
+  for (tree pack = parm_packs; pack; pack = TREE_CHAIN (pack))
+    {
+      tree parm = TREE_PURPOSE (pack);
+      tree arg_pack = TREE_VALUE (pack);
+      tree aps;			/* instance of ARGUMENT_PACK_SELECT
+				   tree.  */
+
+      /* Select the Ith argument from the pack.  */
+      if (TREE_CODE (parm) == PARM_DECL)
+	{
+	  if (index == 0)
+	    {
+	      aps = make_argument_pack_select (arg_pack, index);
+	      mark_used (parm);
+	      register_local_specialization (aps, parm);
+	    }
+	  else
+	    aps = retrieve_local_specialization (parm);
+	}
+      else
+	{
+	  int idx, level;
+	  template_parm_level_and_index (parm, &level, &idx);
+
+	  if (index == 0)
+	    {
+	      aps = make_argument_pack_select (arg_pack, index);
+	      /* Update the corresponding argument.  */
+	      TMPL_ARG (args, level, idx) = aps;
+	    }
+	  else
+	    /* Re-use the ARGUMENT_PACK_SELECT.  */
+	    aps = TMPL_ARG (args, level, idx);
+	}
+      ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+    }
+
+  /* Substitute into the PATTERN with the (possibly altered)
+     arguments.  */
+  if (!TYPE_P (pattern))
+    t = tsubst_expr (pattern, args, complain, in_decl,
+		     /*integral_constant_expression_p=*/false);
+  else
+    t = tsubst (pattern, args, complain, in_decl);
+
+  /*  If the Ith argument pack element is a pack expansion, then
+      the Ith element resulting from the substituting is going to
+      be a pack expansion as well.  */
+  if (has_bare_parameter_packs (t))
+    t = make_pack_expansion (t);
+
+  return t;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9107,8 +9240,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   tree pattern;
   tree pack, packs = NULL_TREE;
   bool unsubstituted_packs = false;
-  bool real_packs = false;
-  int missing_level = 0;
   int i, len = -1;
   tree result;
   struct pointer_map_t *saved_local_specializations = NULL;
@@ -9187,14 +9318,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	  return result;
 	}
 
-      if (arg_from_parm_pack_p (arg_pack, parm_pack))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would find
-	   in the implicit typedef of a class inside the class itself.
-	   Consider this parameter "unsubstituted", so that we will
-	   maintain the outer pack expansion.  */
-	arg_pack = NULL_TREE;
-          
       if (arg_pack)
         {
           int my_len = 
@@ -9222,13 +9345,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
               return error_mark_node;
             }
 
-	  if (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-	      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
-						 0)))
-	    /* This isn't a real argument pack yet.  */;
-	  else
-	    real_packs = true;
-
           /* Keep track of the parameter packs and their corresponding
              argument packs.  */
           packs = tree_cons (parm_pack, arg_pack, packs);
@@ -9240,38 +9356,30 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	     well as the missing_level counter because function parameter
 	     packs don't have a level.  */
 	  unsubstituted_packs = true;
-	  if (!missing_level || missing_level > level)
-	    missing_level = level;
 	}
     }
 
+  /* We could not find any argument packs that work, so we'll just
+     return an unsubstituted pack expansion.  The caller must be
+     prepared to deal with this.  */
+  if (len < 0)
+    len = 1;
+
+  /*  Do we need to use the PACK_EXPANSION_EXTRA_ARGS mechanism?  */
+  bool use_pack_expansion_extra_args =
+    use_pack_expansion_extra_args_p (packs, len, unsubstituted_packs);
+
   /* We cannot expand this expansion expression, because we don't have
      all of the argument packs we need.  */
-  if (unsubstituted_packs)
+  if (unsubstituted_packs || use_pack_expansion_extra)
     {
-      if (real_packs)
+      if (use_pack_expansion_extra)
 	{
 	  /* We got some full packs, but we can't substitute them in until we
 	     have values for all the packs.  So remember these until then.  */
-	  tree save_args;
 
 	  t = make_pack_expansion (pattern);
-
-	  /* The call to add_to_template_args above assumes no overlap
-	     between saved args and new args, so prune away any fake
-	     args, i.e. those that satisfied arg_from_parm_pack_p above.  */
-	  if (missing_level && levels >= missing_level)
-	    {
-	      gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
-			  && missing_level > 1);
-	      TREE_VEC_LENGTH (args) = missing_level - 1;
-	      save_args = copy_node (args);
-	      TREE_VEC_LENGTH (args) = levels;
-	    }
-	  else
-	    save_args = args;
-
-	  PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
+	  PACK_EXPANSION_EXTRA_ARGS (t) = args;
 	}
       else
 	{
@@ -9289,10 +9397,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       return t;
     }
 
-  /* We could not find any argument packs that work.  */
-  if (len < 0)
-    return error_mark_node;
-
   if (need_local_specializations)
     {
       /* We're in a late-specified return type, so create our own local
@@ -9306,60 +9410,20 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   /* For each argument in each argument pack, substitute into the
      pattern.  */
   result = make_tree_vec (len);
-  for (i = 0; i < len; ++i)
+  if (len > 0)
     {
-      /* For parameter pack, change the substitution of the parameter
-         pack to the ith argument in its argument pack, then expand
-         the pattern.  */
-      for (pack = packs; pack; pack = TREE_CHAIN (pack))
-        {
-          tree parm = TREE_PURPOSE (pack);
-	  tree arg;
-
-	  /* Select the Ith argument from the pack.  */
-          if (TREE_CODE (parm) == PARM_DECL)
-            {
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  mark_used (parm);
-		  register_local_specialization (arg, parm);
-		}
-	      else
-		arg = retrieve_local_specialization (parm);
-            }
-          else
-            {
-              int idx, level;
-              template_parm_level_and_index (parm, &level, &idx);
-
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
-		}
-	      else
-		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
-            }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
-        }
-
-      /* Substitute into the PATTERN with the altered arguments.  */
-      if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
-      else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
-
-      if (TREE_VEC_ELT (result, i) == error_mark_node)
+      for (i = 0; i < len; ++i)
 	{
-	  result = error_mark_node;
-	  break;
+	  t = gen_elem_of_pack_expansion_instantiation (pattern, packs,
+							i,
+							args, complain,
+							in_decl);
+	  TREE_VEC_ELT (result, i) = t;
+      	  if (t == error_mark_node)
+	    {
+	      result = error_mark_node;
+	      break;
+	    }
 	}
     }
 
@@ -9374,6 +9438,10 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       else
         {
           int idx, level;
+
+	  if (TREE_VALUE (pack) == NULL_TREE)
+	    continue;
+
           template_parm_level_and_index (parm, &level, &idx);
           
           /* Update the corresponding argument.  */
@@ -11102,8 +11170,24 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    arg = TMPL_ARG (args, level, idx);
 
 	    if (arg && TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-	      /* See through ARGUMENT_PACK_SELECT arguments. */
-	      arg = ARGUMENT_PACK_SELECT_ARG (arg);
+	      {
+		/* See through ARGUMENT_PACK_SELECT arguments. */
+		arg = ARGUMENT_PACK_SELECT_ARG (arg);
+		/* If the selected argument is an expansion E, that most
+		   likely means we were called from
+		   gen_elem_of_pack_expansion_instantiation during the
+		   substituting of pack an argument pack (which Ith
+		   element is a pack expansion, where I is
+		   ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
+		   In this case, the Ith element resulting from this
+		   substituting is going to be a pack expansion, which
+		   pattern is the pattern of E.  Let's return the
+		   pattern of E, and
+		   gen_elem_of_pack_expansion_instantiation will
+		   build the resulting pack expansion from it.  */
+		if (PACK_EXPANSION_P (arg))
+		  arg = PACK_EXPANSION_PATTERN (arg);
+	      }
 	  }
 
 	if (arg == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..a1c64f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");


-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

Sorry it's taken so long for me to respond to this; I forgot about it 
over the holiday break.  The patch is in good shape now, just a few 
tweaks left:

On 12/19/2012 01:21 PM, Dodji Seketeli wrote:
> +      tree aps;			/* instance of ARGUMENT_PACK_SELECT
> +				   tree.  */

Odd comment formatting.

> +  /* We could not find any argument packs that work, so we'll just
> +     return an unsubstituted pack expansion.  The caller must be
> +     prepared to deal with this.  */

I still find this comment confusing.  I think it would be more correct 
to say "we'll just return a substituted pack expansion".

Also, it seems like the unsubstituted_packs code does what we want, so 
you could use that directly rather than change len.

> +  if (unsubstituted_packs || use_pack_expansion_extra)
>      {
> -      if (real_packs)
> +      if (use_pack_expansion_extra)

It seems like the outer "if" is redundant here.

> +  /*  If the Ith argument pack element is a pack expansion, then
> +      the Ith element resulting from the substituting is going to
> +      be a pack expansion as well.  */
> +  if (has_bare_parameter_packs (t))
> +    t = make_pack_expansion (t);

Instead of walking through 't' here, I think it would be cheaper to 
remember if the pack element was a pack expansion.  In which case maybe 
we don't need to split out has_bare_parameter_packs.

> +  if (len > 0)
>      {
> +      for (i = 0; i < len; ++i)
>  	{

The "if" seems redundant here, too.

Jason

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Dodji Seketeli]

Jason Merrill <jason@redhat.com> writes:

> On 12/19/2012 01:21 PM, Dodji Seketeli wrote:
>> +      tree aps;			/* instance of ARGUMENT_PACK_SELECT
>> +				   tree.  */
>
> Odd comment formatting.

Oops, sorry for that.  Fixed now.
>
>> +  /* We could not find any argument packs that work, so we'll just
>> +     return an unsubstituted pack expansion.  The caller must be
>> +     prepared to deal with this.  */
>
> I still find this comment confusing.  I think it would be more correct
> to say "we'll just return a substituted pack expansion".
>
> Also, it seems like the unsubstituted_packs code does what we want, so
> you could use that directly rather than change len.

Yeah, I got rid of the test + change altogether.

>
>> +  if (unsubstituted_packs || use_pack_expansion_extra)
>>      {
>> -      if (real_packs)
>> +      if (use_pack_expansion_extra)
>
> It seems like the outer "if" is redundant here.

Right, fixed.

>
>> +  /*  If the Ith argument pack element is a pack expansion, then
>> +      the Ith element resulting from the substituting is going to
>> +      be a pack expansion as well.  */
>> +  if (has_bare_parameter_packs (t))
>> +    t = make_pack_expansion (t);
>
> Instead of walking through 't' here, I think it would be cheaper to
> remember if the pack element was a pack expansion.  In which case
> maybe we don't need to split out has_bare_parameter_packs.

I wonder why I haven't done that to begin with.  It's definitely
cheaper.

>> +  if (len > 0)
>>      {
>> +      for (i = 0; i < len; ++i)
>>  	{
>
> The "if" seems redundant here, too.

Removed.

Bootstrapped and tested on x86_64-unknown-linux-gnu against trunk.

gcc/cp/

	* pt.c (argument_pack_element_is_expansion_p)
	(make_argument_pack_select, use_pack_expansion_extra_args_p)
	(gen_elem_of_pack_expansion_instantiation): New static functions.
	(tsubst): When looking through an ARGUMENT_PACK_SELECT tree node,
	look through the possibly resulting pack expansion as well.
	(tsubst_pack_expansion): Use use_pack_expansion_extra_p to
	generalize when to use the PACK_EXPANSION_EXTRA_ARGS mechanism.
	Use gen_elem_of_pack_expansion_instantiation to build the
	instantiation piece-wise.  Don't use arg_from_parm_pack_p anymore,
	as gen_elem_of_pack_expansion_instantiation and the change in
	tsubst above generalize this particular case.
	(arg_from_parm_pack_p): Remove this for it's not used by
	tsubst_pack_expansion anymore.

gcc/testsuite/

	* g++.dg/cpp0x/variadic139.C: New test.
	* g++.dg/cpp0x/variadic140.C: Likewise.
	* g++.dg/cpp0x/variadic141.C: Likewise.
---
 gcc/cp/pt.c                              | 359 ++++++++++++++++++-------------
 gcc/testsuite/g++.dg/cpp0x/variadic139.C |  14 ++
 gcc/testsuite/g++.dg/cpp0x/variadic140.C |  22 ++
 gcc/testsuite/g++.dg/cpp0x/variadic141.C |  22 ++
 4 files changed, 270 insertions(+), 147 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic139.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic140.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic141.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 8ddc143..447e18d 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -201,7 +201,6 @@ static void append_type_to_template_for_access_check_1 (tree, tree, tree,
 static tree listify (tree);
 static tree listify_autos (tree, tree);
 static tree template_parm_to_arg (tree t);
-static bool arg_from_parm_pack_p (tree, tree);
 static tree current_template_args (void);
 static tree tsubst_template_parm (tree, tree, tsubst_flags_t);
 static tree instantiate_alias_template (tree, tree, tsubst_flags_t);
@@ -3816,42 +3815,6 @@ template_parm_to_arg (tree t)
   return t;
 }
 
-/* This function returns TRUE if PARM_PACK is a template parameter
-   pack and if ARG_PACK is what template_parm_to_arg returned when
-   passed PARM_PACK.  */
-
-static bool
-arg_from_parm_pack_p (tree arg_pack, tree parm_pack)
-{
-  /* For clarity in the comments below let's use the representation
-     argument_pack<elements>' to denote an argument pack and its
-     elements.
-
-     In the 'if' block below, we want to detect cases where
-     ARG_PACK is argument_pack<PARM_PACK...>.  I.e, we want to
-     check if ARG_PACK is an argument pack which sole element is
-     the expansion of PARM_PACK.  That argument pack is typically
-     created by template_parm_to_arg when passed a parameter
-     pack.  */
-
-  if (arg_pack
-      && TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0)))
-    {
-      tree expansion = TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0);
-      tree pattern = PACK_EXPANSION_PATTERN (expansion);
-      if ((TYPE_P (pattern) && same_type_p (pattern, parm_pack))
-	  || (!TYPE_P (pattern) && cp_tree_equal (parm_pack, pattern)))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would
-	   find in the implicit typedef of a class inside the
-	   class itself.  Consider this parameter "unsubstituted",
-	   so that we will maintain the outer pack expansion.  */
-	return true;
-    }
-  return false;
-}
-
 /* Given a set of template parameters, return them as a set of template
    arguments.  The template parameters are represented as a TREE_VEC, in
    the form documented in cp-tree.h for template arguments.  */
@@ -9148,6 +9111,168 @@ make_fnparm_pack (tree spec_parm)
   return extract_fnparm_pack (NULL_TREE, &spec_parm);
 }
 
+/* Return true iff the Ith element of the argument pack ARG_PACK is a
+   pack expansion.  */
+
+static bool
+argument_pack_element_is_expansion_p (tree arg_pack, int i)
+{
+  tree vec = ARGUMENT_PACK_ARGS (arg_pack);
+  if (i >= TREE_VEC_LENGTH (vec))
+    return false;
+  return PACK_EXPANSION_P (TREE_VEC_ELT (vec, i));
+}
+
+
+/* Creates and return an ARGUMENT_PACK_SELECT tree node.  */
+
+static tree
+make_argument_pack_select (tree arg_pack, unsigned index)
+{
+  tree aps = make_node (ARGUMENT_PACK_SELECT);
+
+  ARGUMENT_PACK_SELECT_FROM_PACK (aps) = arg_pack;
+  ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+
+  return aps;
+}
+
+/*  This is a subroutine of tsubst_pack_expansion.
+
+    It returns TRUE if we need to use the PACK_EXPANSION_EXTRA_ARGS
+    mechanism to store the (non complete list of) arguments of the
+    substitution and return a non substituted pack expansion, in order
+    to wait for when we have enough arguments to really perform the
+    substitution.  */
+
+static bool
+use_pack_expansion_extra_args_p (tree parm_packs,
+				 int arg_pack_len,
+				 bool has_empty_arg)
+{
+  if (parm_packs == NULL_TREE)
+    return false;
+
+  bool has_expansion_arg = false;
+  for (int i = 0 ; i < arg_pack_len; ++i)
+    {
+      bool has_non_expansion_arg = false;
+      for (tree parm_pack = parm_packs;
+	   parm_pack;
+	   parm_pack = TREE_CHAIN (parm_pack))
+	{
+	  tree arg = TREE_VALUE (parm_pack);
+
+	  if (argument_pack_element_is_expansion_p (arg, i))
+	    has_expansion_arg = true;
+	  else
+	    has_non_expansion_arg = true;
+	}
+
+      /* If one pack has an expansion and another pack has a normal
+	 argument or if one pack has an empty argument another one
+	 hasn't then tsubst_pack_expansion cannot perform the
+	 substitution and need to fall back on the
+	 PACK_EXPANSION_EXTRA mechanism.  */
+      if ((has_expansion_arg && has_non_expansion_arg)
+	  || (has_empty_arg && (has_expansion_arg || has_non_expansion_arg)))
+	return true;
+    }
+  return false;
+}
+
+/* [temp.variadic]/6 says that:
+
+       The instantiation of a pack expansion [...]
+       produces a list E1,E2, ..., En, where N is the number of elements
+       in the pack expansion parameters.
+
+   This subroutine of tsubst_pack_expansion produces one of these Ei.
+
+   PATTERN is the pattern of the pack expansion.  PARM_PACKS is a
+   TREE_LIST in which each TREE_PURPOSE is a parameter pack of
+   PATTERN, and each TREE_VALUE is its corresponding argument pack.
+   INDEX is the index 'i' of the element Ei to produce.  ARGS,
+   COMPLAIN, and IN_DECL are the same parameters as for the
+   tsubst_pack_expansion function.
+
+   The function returns the resulting Ei upon successful completion,
+   or error_mark_node.
+
+   Note that this function possibly modifies the ARGS parameter, so
+   it's the responsibility of the caller to restore it.  */
+
+static tree
+gen_elem_of_pack_expansion_instantiation (tree pattern,
+					  tree parm_packs,
+					  unsigned index,
+					  tree args /* This parm gets
+						       modified.  */,
+					  tsubst_flags_t complain,
+					  tree in_decl)
+{
+  tree t;
+  bool ith_elem_is_expansion = false;
+
+  /* For each parameter pack, change the substitution of the parameter
+     pack to the ith argument in its argument pack, then expand the
+     pattern.  */
+  for (tree pack = parm_packs; pack; pack = TREE_CHAIN (pack))
+    {
+      tree parm = TREE_PURPOSE (pack);
+      tree arg_pack = TREE_VALUE (pack);
+      tree aps;			/* instance of ARGUMENT_PACK_SELECT.  */
+
+      ith_elem_is_expansion |=
+	PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
+					index));
+      /* Select the Ith argument from the pack.  */
+      if (TREE_CODE (parm) == PARM_DECL)
+	{
+	  if (index == 0)
+	    {
+	      aps = make_argument_pack_select (arg_pack, index);
+	      mark_used (parm);
+	      register_local_specialization (aps, parm);
+	    }
+	  else
+	    aps = retrieve_local_specialization (parm);
+	}
+      else
+	{
+	  int idx, level;
+	  template_parm_level_and_index (parm, &level, &idx);
+
+	  if (index == 0)
+	    {
+	      aps = make_argument_pack_select (arg_pack, index);
+	      /* Update the corresponding argument.  */
+	      TMPL_ARG (args, level, idx) = aps;
+	    }
+	  else
+	    /* Re-use the ARGUMENT_PACK_SELECT.  */
+	    aps = TMPL_ARG (args, level, idx);
+	}
+      ARGUMENT_PACK_SELECT_INDEX (aps) = index;
+    }
+
+  /* Substitute into the PATTERN with the (possibly altered)
+     arguments.  */
+  if (!TYPE_P (pattern))
+    t = tsubst_expr (pattern, args, complain, in_decl,
+		     /*integral_constant_expression_p=*/false);
+  else
+    t = tsubst (pattern, args, complain, in_decl);
+
+  /*  If the Ith argument pack element is a pack expansion, then
+      the Ith element resulting from the substituting is going to
+      be a pack expansion as well.  */
+  if (ith_elem_is_expansion)
+    t = make_pack_expansion (t);
+
+  return t;
+}
+
 /* Substitute ARGS into T, which is an pack expansion
    (i.e. TYPE_PACK_EXPANSION or EXPR_PACK_EXPANSION). Returns a
    TREE_VEC with the substituted arguments, a PACK_EXPANSION_* node
@@ -9160,8 +9285,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   tree pattern;
   tree pack, packs = NULL_TREE;
   bool unsubstituted_packs = false;
-  bool real_packs = false;
-  int missing_level = 0;
   int i, len = -1;
   tree result;
   struct pointer_map_t *saved_local_specializations = NULL;
@@ -9240,14 +9363,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	  return result;
 	}
 
-      if (arg_from_parm_pack_p (arg_pack, parm_pack))
-	/* The argument pack that the parameter maps to is just an
-	   expansion of the parameter itself, such as one would find
-	   in the implicit typedef of a class inside the class itself.
-	   Consider this parameter "unsubstituted", so that we will
-	   maintain the outer pack expansion.  */
-	arg_pack = NULL_TREE;
-          
       if (arg_pack)
         {
           int my_len = 
@@ -9275,13 +9390,6 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
               return error_mark_node;
             }
 
-	  if (TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
-	      && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
-						 0)))
-	    /* This isn't a real argument pack yet.  */;
-	  else
-	    real_packs = true;
-
           /* Keep track of the parameter packs and their corresponding
              argument packs.  */
           packs = tree_cons (parm_pack, arg_pack, packs);
@@ -9293,59 +9401,39 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
 	     well as the missing_level counter because function parameter
 	     packs don't have a level.  */
 	  unsubstituted_packs = true;
-	  if (!missing_level || missing_level > level)
-	    missing_level = level;
 	}
     }
 
+  /*  Do we need to use the PACK_EXPANSION_EXTRA_ARGS mechanism?  */
+  bool use_pack_expansion_extra_args =
+    use_pack_expansion_extra_args_p (packs, len, unsubstituted_packs);
+
   /* We cannot expand this expansion expression, because we don't have
      all of the argument packs we need.  */
-  if (unsubstituted_packs)
+  if (use_pack_expansion_extra_args)
     {
-      if (real_packs)
-	{
-	  /* We got some full packs, but we can't substitute them in until we
-	     have values for all the packs.  So remember these until then.  */
-	  tree save_args;
-
-	  t = make_pack_expansion (pattern);
+      /* We got some full packs, but we can't substitute them in until we
+	 have values for all the packs.  So remember these until then.  */
 
-	  /* The call to add_to_template_args above assumes no overlap
-	     between saved args and new args, so prune away any fake
-	     args, i.e. those that satisfied arg_from_parm_pack_p above.  */
-	  if (missing_level && levels >= missing_level)
-	    {
-	      gcc_assert (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args)
-			  && missing_level > 1);
-	      TREE_VEC_LENGTH (args) = missing_level - 1;
-	      save_args = copy_node (args);
-	      TREE_VEC_LENGTH (args) = levels;
-	    }
-	  else
-	    save_args = args;
-
-	  PACK_EXPANSION_EXTRA_ARGS (t) = save_args;
-	}
+      t = make_pack_expansion (pattern);
+      PACK_EXPANSION_EXTRA_ARGS (t) = args;
+      return t;
+    }
+  else if (unsubstituted_packs)
+    {
+      /* There were no real arguments, we're just replacing a parameter
+	 pack with another version of itself. Substitute into the
+	 pattern and return a PACK_EXPANSION_*. The caller will need to
+	 deal with that.  */
+      if (TREE_CODE (t) == EXPR_PACK_EXPANSION)
+	t = tsubst_expr (pattern, args, complain, in_decl,
+			 /*integral_constant_expression_p=*/false);
       else
-	{
-	  /* There were no real arguments, we're just replacing a parameter
-	     pack with another version of itself. Substitute into the
-	     pattern and return a PACK_EXPANSION_*. The caller will need to
-	     deal with that.  */
-	  if (TREE_CODE (t) == EXPR_PACK_EXPANSION)
-	    t = tsubst_expr (pattern, args, complain, in_decl,
-			     /*integral_constant_expression_p=*/false);
-	  else
-	    t = tsubst (pattern, args, complain, in_decl);
-	  t = make_pack_expansion (t);
-	}
+	t = tsubst (pattern, args, complain, in_decl);
+      t = make_pack_expansion (t);
       return t;
     }
 
-  /* We could not find any argument packs that work.  */
-  if (len < 0)
-    return error_mark_node;
-
   if (need_local_specializations)
     {
       /* We're in a late-specified return type, so create our own local
@@ -9361,55 +9449,12 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
   result = make_tree_vec (len);
   for (i = 0; i < len; ++i)
     {
-      /* For parameter pack, change the substitution of the parameter
-         pack to the ith argument in its argument pack, then expand
-         the pattern.  */
-      for (pack = packs; pack; pack = TREE_CHAIN (pack))
-        {
-          tree parm = TREE_PURPOSE (pack);
-	  tree arg;
-
-	  /* Select the Ith argument from the pack.  */
-          if (TREE_CODE (parm) == PARM_DECL)
-            {
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  mark_used (parm);
-		  register_local_specialization (arg, parm);
-		}
-	      else
-		arg = retrieve_local_specialization (parm);
-            }
-          else
-            {
-              int idx, level;
-              template_parm_level_and_index (parm, &level, &idx);
-
-	      if (i == 0)
-		{
-		  arg = make_node (ARGUMENT_PACK_SELECT);
-		  ARGUMENT_PACK_SELECT_FROM_PACK (arg) = TREE_VALUE (pack);
-		  /* Update the corresponding argument.  */
-		  TMPL_ARG (args, level, idx) = arg;
-		}
-	      else
-		/* Re-use the ARGUMENT_PACK_SELECT.  */
-		arg = TMPL_ARG (args, level, idx);
-            }
-	  ARGUMENT_PACK_SELECT_INDEX (arg) = i;
-        }
-
-      /* Substitute into the PATTERN with the altered arguments.  */
-      if (!TYPE_P (pattern))
-        TREE_VEC_ELT (result, i) = 
-          tsubst_expr (pattern, args, complain, in_decl,
-                       /*integral_constant_expression_p=*/false);
-      else
-        TREE_VEC_ELT (result, i) = tsubst (pattern, args, complain, in_decl);
-
-      if (TREE_VEC_ELT (result, i) == error_mark_node)
+      t = gen_elem_of_pack_expansion_instantiation (pattern, packs,
+						    i,
+						    args, complain,
+						    in_decl);
+      TREE_VEC_ELT (result, i) = t;
+      if (t == error_mark_node)
 	{
 	  result = error_mark_node;
 	  break;
@@ -9427,6 +9472,10 @@ tsubst_pack_expansion (tree t, tree args, tsubst_flags_t complain,
       else
         {
           int idx, level;
+
+	  if (TREE_VALUE (pack) == NULL_TREE)
+	    continue;
+
           template_parm_level_and_index (parm, &level, &idx);
           
           /* Update the corresponding argument.  */
@@ -11163,8 +11212,24 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	    arg = TMPL_ARG (args, level, idx);
 
 	    if (arg && TREE_CODE (arg) == ARGUMENT_PACK_SELECT)
-	      /* See through ARGUMENT_PACK_SELECT arguments. */
-	      arg = ARGUMENT_PACK_SELECT_ARG (arg);
+	      {
+		/* See through ARGUMENT_PACK_SELECT arguments. */
+		arg = ARGUMENT_PACK_SELECT_ARG (arg);
+		/* If the selected argument is an expansion E, that most
+		   likely means we were called from
+		   gen_elem_of_pack_expansion_instantiation during the
+		   substituting of pack an argument pack (which Ith
+		   element is a pack expansion, where I is
+		   ARGUMENT_PACK_SELECT_INDEX) into a pack expansion.
+		   In this case, the Ith element resulting from this
+		   substituting is going to be a pack expansion, which
+		   pattern is the pattern of E.  Let's return the
+		   pattern of E, and
+		   gen_elem_of_pack_expansion_instantiation will
+		   build the resulting pack expansion from it.  */
+		if (PACK_EXPANSION_P (arg))
+		  arg = PACK_EXPANSION_PATTERN (arg);
+	      }
 	  }
 
 	if (arg == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic139.C b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
new file mode 100644
index 0000000..a1c64f3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic139.C
@@ -0,0 +1,14 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List {};
+template<int T> struct Z {static const int value = T;};
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...U>
+struct F
+{
+  using N = LZ<U::value...>;
+};
+
+F<Z<1>, Z<2> >::N A;
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic140.C b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
new file mode 100644
index 0000000..17ca9e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic140.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., T::value_square...>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic141.C b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
new file mode 100644
index 0000000..6b893a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic141.C
@@ -0,0 +1,22 @@
+// Origin: PR c++/53609
+// { dg-do compile { target c++11 } }
+
+template<class...I> struct List{ static const bool is_ok = false;};
+template<int T> struct Z
+{
+  static const int value = T;
+  static const int value_square = T * T;
+};
+
+template<template<int> class U>
+struct List<U<2>, U<3>, U<4>, U<9>> { static const bool is_ok = true;};
+
+template<int...T> using LZ = List<Z<T>...>;
+
+template<class...T>
+struct F
+{
+  using N = LZ<T::value..., Z<4>::value, Z<9>::value>;
+};
+
+static_assert (F<Z<2>, Z<3>>::N::is_ok, "");
-- 
1.7.11.7


-- 
		Dodji

Re: [PATCH] PR c++/53609 - Wrong argument deduction for pack expansion in argument pack

[Jason Merrill]

On 01/21/2013 03:09 PM, Dodji Seketeli wrote:
> +      ith_elem_is_expansion |=
> +	PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack),
> +					index));

Let's use argument_pack_element_is_expansion_p here, too.

> +  /*  Do we need to use the PACK_EXPANSION_EXTRA_ARGS mechanism?  */
> +  bool use_pack_expansion_extra_args =
> +    use_pack_expansion_extra_args_p (packs, len, unsubstituted_packs);
> +
>    /* We cannot expand this expansion expression, because we don't have
>       all of the argument packs we need.  */
> -  if (unsubstituted_packs)
> +  if (use_pack_expansion_extra_args)

I think we don't need the variable anymore.

> -  /* We could not find any argument packs that work.  */
> -  if (len < 0)
> -    return error_mark_node;

Let's replace this with an assert that len can't be <0.

OK with these changes.

Jason