Re: [PATCH] c++: generic targs and identity substitution [PR105956]

[Patrick Palka <ppalka@redhat.com>]

On Thu, 7 Jul 2022, Jason Merrill wrote:

> On 7/6/22 15:26, Patrick Palka wrote:
> > On Tue, 5 Jul 2022, Jason Merrill wrote:
> > 
> > > On 7/5/22 10:06, Patrick Palka wrote:
> > > > On Fri, 1 Jul 2022, Jason Merrill wrote:
> > > > 
> > > > > On 6/29/22 13:42, Patrick Palka wrote:
> > > > > > In r13-1045-gcb7fd1ea85feea I assumed that substitution into generic
> > > > > > DECL_TI_ARGS corresponds to an identity mapping of the given
> > > > > > arguments,
> > > > > > and hence its safe to always elide such substitution.  But this PR
> > > > > > demonstrates that such a substitution isn't always the identity
> > > > > > mapping,
> > > > > > in particular when there's an ARGUMENT_PACK_SELECT argument, which
> > > > > > gets
> > > > > > handled specially during substitution:
> > > > > > 
> > > > > >      * when substituting an APS into a template parameter, we strip
> > > > > > the
> > > > > >        APS to its underlying argument;
> > > > > >      * and when substituting an APS into a pack expansion, we strip
> > > > > > the
> > > > > >        APS to its underlying argument pack.
> > > > > 
> > > > > Ah, right.  For instance, in variadic96.C we have
> > > > > 
> > > > >       10	template < typename... T >
> > > > >       11	struct derived
> > > > >       12	  : public base< T, derived< T... > >...
> > > > > 
> > > > > so when substituting into the base-specifier, we're approaching it
> > > > > from
> > > > > the
> > > > > outside in, so when we get to the inner T... we need some way to find
> > > > > the
> > > > > T
> > > > > pack again.  It might be possible to remove the need for APS by
> > > > > substituting
> > > > > inner pack expansions before outer ones, which could improve
> > > > > worst-case
> > > > > complexity, but I don't know how relevant that is in real code; I
> > > > > imagine
> > > > > most
> > > > > inner pack expansions are as simple as this one.
> > > > 
> > > > Aha, that makes sense.
> > > > 
> > > > > 
> > > > > > In this testcase, when expanding the pack expansion pattern (idx +
> > > > > > Ns)...
> > > > > > with Ns={0,1}, we specialize idx twice, first with Ns=APS<0,{0,1}>
> > > > > > and
> > > > > > then Ns=APS<1,{0,1}>.  The DECL_TI_ARGS of idx are the generic
> > > > > > template
> > > > > > arguments of the enclosing class template impl, so before r13-1045,
> > > > > > we'd substitute into its DECL_TI_ARGS which gave Ns={0,1} as
> > > > > > desired.
> > > > > > But after r13-1045, we elide this substitution and end up attempting
> > > > > > to
> > > > > > hash the original Ns argument, an APS, which ICEs.
> > > > > > 
> > > > > > So this patch partially reverts this part of r13-1045.  I considered
> > > > > > using preserve_args in this case instead, but that'd break the
> > > > > > static_assert in the testcase because preserve_args always strips
> > > > > > APS to
> > > > > > its underlying argument, but here we want to strip it to its
> > > > > > underlying
> > > > > > argument pack, so we'd incorrectly end up forming the
> > > > > > specializations
> > > > > > impl<0>::idx and impl<1>::idx instead of impl<0,1>::idx.
> > > > > > 
> > > > > > Although we can't elide the substitution into DECL_TI_ARGS in light
> > > > > > of
> > > > > > ARGUMENT_PACK_SELECT, it should still be safe to elide template
> > > > > > argument
> > > > > > coercion in the case of a non-template decl, which this patch
> > > > > > preserves.
> > > > > > 
> > > > > > It's unfortunate that we need to remove this optimization just
> > > > > > because
> > > > > > it doesn't hold for one special tree code.  So this patch implements
> > > > > > a
> > > > > > heuristic in tsubst_template_args to avoid allocating a new TREE_VEC
> > > > > > if
> > > > > > the substituted elements are identical to those of a level from
> > > > > > ARGS.
> > > > > > It turns out that about 30% of all calls to tsubst_template_args
> > > > > > benefit
> > > > > > from this optimization, and it reduces memory usage by about 1.5%
> > > > > > for
> > > > > > e.g. stdc++.h (relative to r13-1045).  (This is the maybe_reuse
> > > > > > stuff,
> > > > > > the rest of the changes to tsubst_template_args are just drive-by
> > > > > > cleanups.)
> > > > > > 
> > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > > > > > for
> > > > > > trunk?  Patch generated with -w to ignore noisy whitespace changes.
> > > > > > 
> > > > > > 	PR c++/105956
> > > > > > 
> > > > > > gcc/cp/ChangeLog:
> > > > > > 
> > > > > > 	* pt.cc (tsubst_template_args): Move variable declarations
> > > > > > 	closer to their first use.  Replace 'orig_t' with 'r'.  Rename
> > > > > > 	'need_new' to 'const_subst_p'.  Heuristically detect if the
> > > > > > 	substituted elements are identical to that of a level from
> > > > > > 	'args' and avoid allocating a new TREE_VEC if so.
> > > > > > 	(tsubst_decl) <case TYPE_DECL, case VAR_DECL>: Revert
> > > > > > 	r13-1045-gcb7fd1ea85feea change for avoiding substitution into
> > > > > > 	DECL_TI_ARGS, but still avoid coercion in this case.
> > > > > > 
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > 
> > > > > > 	* g++.dg/cpp0x/variadic183.C: New test.
> > > > > > ---
> > > > > >     gcc/cp/pt.cc                             | 113
> > > > > > ++++++++++++++---------
> > > > > >     gcc/testsuite/g++.dg/cpp0x/variadic183.C |  14 +++
> > > > > >     2 files changed, 85 insertions(+), 42 deletions(-)
> > > > > >     create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic183.C
> > > > > > 
> > > > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > > > > index 8672da123f4..7898834faa6 100644
> > > > > > --- a/gcc/cp/pt.cc
> > > > > > +++ b/gcc/cp/pt.cc
> > > > > > @@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
> > > > > >          Fixed by: C++20 modules.  */
> > > > > >       #include "config.h"
> > > > > > +#define INCLUDE_ALGORITHM // for std::equal
> > > > > >     #include "system.h"
> > > > > >     #include "coretypes.h"
> > > > > >     #include "cp-tree.h"
> > > > > > @@ -13544,17 +13545,22 @@ tsubst_argument_pack (tree orig_arg, tree
> > > > > > args,
> > > > > > tsubst_flags_t complain,
> > > > > >     tree
> > > > > >     tsubst_template_args (tree t, tree args, tsubst_flags_t
> > > > > > complain,
> > > > > > tree
> > > > > > in_decl)
> > > > > >     {
> > > > > > -  tree orig_t = t;
> > > > > > -  int len, need_new = 0, i, expanded_len_adjust = 0, out;
> > > > > > -  tree *elts;
> > > > > > -
> > > > > >       if (t == error_mark_node)
> > > > > >         return error_mark_node;
> > > > > >     -  len = TREE_VEC_LENGTH (t);
> > > > > > -  elts = XALLOCAVEC (tree, len);
> > > > > > +  const int len = TREE_VEC_LENGTH (t);
> > > > > > +  tree *elts = XALLOCAVEC (tree, len);
> > > > > > +  int expanded_len_adjust = 0;
> > > > > >     -  for (i = 0; i < len; i++)
> > > > > > +  /* True iff no element of T was changed by the substitution.  */
> > > > > > +  bool const_subst_p = true;
> > > > > > +
> > > > > > +  /* If MAYBE_REUSE is non-NULL, as an optimization we'll try to
> > > > > > reuse
> > > > > > and
> > > > > > +     return this TREE_VEC instead of allocating a new one if
> > > > > > possible.
> > > > > > This
> > > > > > +     will either be ARGS or a level from ARGS.  */
> > > > > > +  tree maybe_reuse = NULL_TREE;
> > > > > > +
> > > > > > +  for (int i = 0; i < len; i++)
> > > > > >         {
> > > > > >           tree orig_arg = TREE_VEC_ELT (t, i);
> > > > > >           tree new_arg;
> > > > > > @@ -13580,56 +13586,90 @@ tsubst_template_args (tree t, tree args,
> > > > > > tsubst_flags_t complain, tree in_decl)
> > > > > >           else if (ARGUMENT_PACK_P (orig_arg))
> > > > > >     	new_arg = tsubst_argument_pack (orig_arg, args, complain,
> > > > > > in_decl);
> > > > > >           else
> > > > > > +	{
> > > > > >     	  new_arg = tsubst_template_arg (orig_arg, args, complain,
> > > > > > in_decl);
> > > > > >     +	  /* If T heuristically appears to be a set of generic
> > > > > > template
> > > > > > +	     arguments, set MAYBE_REUSE to the corresponding level
> > > > > > from
> > > > > > +	     ARGS.  */
> > > > > > +	  if (maybe_reuse == NULL_TREE && orig_arg != NULL_TREE)
> > > > > > +	    {
> > > > > > +	      if (TEMPLATE_PARM_P (orig_arg))
> > > > > > +		{
> > > > > 
> > > > > This doesn't handle the case of variadic template parameters, which
> > > > > are
> > > > > represented in the generic args with a pack expansion.  If this is a
> > > > > choice,
> > > > > there should be a comment about it.
> > > > 
> > > > AFAICT substituting such a pack expansion will never be an identity
> > > > transform -- the relevant targ during tsubst_pack_expansion will be an
> > > > ARGUMENT_PACK, and we'll return the TREE_VEC from that ARGUMENT_PACK
> > > > instead of the ARGUMENT_PACK itself, so there's no way we can reuse (a
> > > > level from) 'args' in this case.
> > > 
> > > Hmm, when replacing a level of T... we start with a TREE_VEC containing an
> > > ARGUMENT_PACK around a TREE_VEC containing a PACK_EXPANSION, and
> > > substitution
> > > should replace that with the argument level, a TREE_VEC containing an
> > > ARGUMENT_PACK around a TREE_VEC containing an actual set of args, so reuse
> > > ought to be possible?
> > 
> > Ah, I think see what you mean.  For that, I think it suffices to just
> > handle in tsubst_template_args the case where we're substituting into a
> > single-elt TREE_VEC such as {T...}.  That'll allow us to reuse the inner
> > TREE_VEC containing the actual set of args at least.
> 
> Sounds good.
> 
> > And AFAICT that's
> > the best we can do, since we'll always be creating a new ARGUMENT_PACK
> > which doesn't appear in any other TREE_VEC we have at our disposal.
> 
> Why always?  Why can't tsubst_argument_pack recognize the {T...} case and
> return the corresponding ARGUMENT_PACK from args?
> 
> template <class... Ts> struct A
> {
>   A<Ts...>* a;
> };
> 
> A<int,char> a;
> 
> With your patch, the substitution into A<Ts...> reuses the inner TREE_VEC, but
> why not reuse the ARGUMENT_PACK as well?  And why not set maybe_reuse for the
> enclosing arg level?

D'oh, you're totally right, I didn't notice this opportunity.  Here's a
patch that makes tsubst_argument_pack reuse the ARGUMENT_PACK from
'args' when possible, and in turn makes tsubst_template_args reuse the
enclosing arg level.   Relative to the previous patch, this increases
the % of calls to tsubst_template_args that benefit from this
optimization a further 10% (30% -> 40%), and reduces memory usage for
e.g. zip.cpp from range-v3 a further 2.5% (1.5% -> 4%).

-- >8 --

Subject: [PATCH] c++: generic targs and identity substitution [PR105956]

In r13-1045-gcb7fd1ea85feea I assumed that substitution into generic
DECL_TI_ARGS corresponds to an identity mapping of the given arguments,
and hence its safe to always elide such substitution.  But this PR
demonstrates that such a substitution isn't always the identity mapping,
in particular when there's an ARGUMENT_PACK_SELECT argument, which gets
handled specially during substitution:

  * when substituting an APS into a template parameter, we strip the
    APS to its underlying argument;
  * and when substituting an APS into a pack expansion, we strip the
    APS to its underlying argument pack.

In this testcase, when expanding the pack expansion pattern (idx + Ns)...
with Ns={0,1}, we specialize idx twice, first with Ns=APS<0,{0,1}> and
then Ns=APS<1,{0,1}>.  The DECL_TI_ARGS of idx are the generic template
arguments of the enclosing class template impl, so before r13-1045,
we'd substitute into its DECL_TI_ARGS which gave Ns={0,1} as desired.
But after r13-1045, we elide this substitution and end up attempting to
hash the original Ns argument, an APS, which ICEs.

So this patch reverts that part of r13-1045.  I considered using
preserve_args in this case instead, but that'd break the static_assert
in the testcase because preserve_args always strips APS to its
underlying argument, but here we want to strip it to its underlying
argument pack, so we'd incorrectly end up forming the specializations
impl<0>::idx and impl<1>::idx instead of impl<0,1>::idx.

Although we can't elide the substitution into DECL_TI_ARGS in light of
ARGUMENT_PACK_SELECT, it should still be safe to elide template argument
coercion in the case of a non-template decl, which this patch preserves.

It's unfortunate that we need to remove this optimization just because
it doesn't hold for one special tree code.  So this patch implements a
heuristic in tsubst_template_args to avoid allocating a new TREE_VEC if
the substituted elements are identical to those of a level from ARGS,
and a similar heuristic for tsubst_argument_pack.  It turns out that
about 40% of all calls to tsubst_template_args benefit from this, and it
reduces memory usage by about 4% for e.g. zip.cpp from range-v3 (relative
to r13-1045) which more than makes up for the reversion.

	PR c++/105956

gcc/cp/ChangeLog:

	* pt.cc (template_arg_to_parm): Define.
	(tsubst_argument_pack): Try to reuse the corresponding
	ARGUMENT_PACK from 'args' when substituting into an
	ARGUMENT_PACK for a generic template argument.
	(tsubst_template_args): Move variable declarations
	closer to their first use.  Replace 'orig_t' with 'r'.  Rename
	'need_new' to 'const_subst_p'.  Heuristically detect if the
	substituted elements are identical to that of a level from
	'args' and avoid allocating a new TREE_VEC if so.  Assert that
	'out' never exceeds the length of the new TREE_VEC, and remove
	dead ARGUMENT_PACK_P test.
	(tsubst_decl) <case TYPE_DECL, case VAR_DECL>: Revert
	r13-1045-gcb7fd1ea85feea change for avoiding substitution into
	DECL_TI_ARGS, but still avoid coercion in this case.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp0x/variadic183.C: New test.
---
 gcc/cp/pt.cc                             | 172 ++++++++++++++++-------
 gcc/testsuite/g++.dg/cpp0x/variadic183.C |  14 ++
 2 files changed, 138 insertions(+), 48 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/variadic183.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 8672da123f4..85f3462a0ac 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -27,6 +27,7 @@ along with GCC; see the file COPYING3.  If not see
      Fixed by: C++20 modules.  */
 
 #include "config.h"
+#define INCLUDE_ALGORITHM // for std::equal
 #include "system.h"
 #include "coretypes.h"
 #include "cp-tree.h"
@@ -4916,6 +4917,33 @@ template_parm_to_arg (tree t)
   return t;
 }
 
+/* If T looks like a generic template argument produced by template_parm_to_arg,
+   return the corresponding template parameter, otherwise return NULL_TREE.  */
+
+static tree
+template_arg_to_parm (tree t)
+{
+  if (t == NULL_TREE)
+    return NULL_TREE;
+
+  if (ARGUMENT_PACK_P (t))
+    {
+      tree args = ARGUMENT_PACK_ARGS (t);
+      if (TREE_VEC_LENGTH (args) == 1
+	  && PACK_EXPANSION_P (TREE_VEC_ELT (args, 0)))
+	{
+	  t = PACK_EXPANSION_PATTERN (TREE_VEC_ELT (args, 0));
+	  if (REFERENCE_REF_P (t))
+	    t = TREE_OPERAND (t, 0);
+	}
+    }
+
+  if (TEMPLATE_PARM_P (t))
+    return t;
+
+  return NULL_TREE;
+}
+
 /* Given a single level of template parameters (a TREE_VEC), return it
    as a set of template arguments.  */
 
@@ -13516,12 +13544,38 @@ tree
 tsubst_argument_pack (tree orig_arg, tree args, tsubst_flags_t complain,
 		      tree in_decl)
 {
+  /* This flag is used only during deduction, and we don't expect to
+     see such ARGUMENT_PACKs during substitution.  */
+  gcc_assert (!ARGUMENT_PACK_INCOMPLETE_P (orig_arg));
+
   /* Substitute into each of the arguments.  */
   tree pack_args = tsubst_template_args (ARGUMENT_PACK_ARGS (orig_arg),
 					 args, complain, in_decl);
-  tree new_arg = error_mark_node;
-  if (pack_args != error_mark_node)
+  if (pack_args == error_mark_node)
+    return error_mark_node;
+
+  if (pack_args == ARGUMENT_PACK_ARGS (orig_arg))
+    return orig_arg;
+
+  /* If we're substituting into an ARGUMENT_PACK for a generic template
+     argument, we might be able to avoid allocating a new ARGUMENT_PACK
+     by reusing the corresponding ARGUMENT_PACK from ARGS if the
+     substituted result is identical to it.  */
+  if (tree parm = template_arg_to_parm (orig_arg))
     {
+      int level, index;
+      template_parm_level_and_index (parm, &level, &index);
+      if (TMPL_ARGS_DEPTH (args) >= level)
+	if (tree arg = TMPL_ARG (args, level, index))
+	  if (TREE_CODE (arg) == TREE_CODE (orig_arg)
+	      && ARGUMENT_PACK_ARGS (arg) == pack_args)
+	    {
+	      gcc_assert (!ARGUMENT_PACK_INCOMPLETE_P (arg));
+	      return arg;
+	    }
+    }
+
+  tree new_arg;
   if (TYPE_P (orig_arg))
     {
       new_arg = cxx_make_type (TREE_CODE (orig_arg));
@@ -13532,10 +13586,7 @@ tsubst_argument_pack (tree orig_arg, tree args, tsubst_flags_t complain,
       new_arg = make_node (TREE_CODE (orig_arg));
       TREE_CONSTANT (new_arg) = TREE_CONSTANT (orig_arg);
     }
-
   ARGUMENT_PACK_ARGS (new_arg) = pack_args;
-    }
-
   return new_arg;
 }
 
@@ -13544,17 +13595,22 @@ tsubst_argument_pack (tree orig_arg, tree args, tsubst_flags_t complain,
 tree
 tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 {
-  tree orig_t = t;
-  int len, need_new = 0, i, expanded_len_adjust = 0, out;
-  tree *elts;
-
   if (t == error_mark_node)
     return error_mark_node;
 
-  len = TREE_VEC_LENGTH (t);
-  elts = XALLOCAVEC (tree, len);
+  const int len = TREE_VEC_LENGTH (t);
+  tree *elts = XALLOCAVEC (tree, len);
+  int expanded_len_adjust = 0;
 
-  for (i = 0; i < len; i++)
+  /* True iff the substituted result is identical to T.  */
+  bool const_subst_p = true;
+
+  /* If MAYBE_REUSE is a TREE_VEC, as an optimization we'll try to reuse and
+     return this TREE_VEC instead of allocating a new one if possible.  This
+     will either be ARGS or a level from ARGS.  */
+  tree maybe_reuse = NULL_TREE;
+
+  for (int i = 0; i < len; i++)
     {
       tree orig_arg = TREE_VEC_ELT (t, i);
       tree new_arg;
@@ -13587,49 +13643,80 @@ tsubst_template_args (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 
       elts[i] = new_arg;
       if (new_arg != orig_arg)
-	need_new = 1;
+	const_subst_p = false;
     }
 
-  if (!need_new)
+  if (const_subst_p)
     return t;
 
+  /* If T appears to be a set of generic template arguments, set
+     MAYBE_REUSE to the corresponding level from ARGS.  */
+  if (tree parm = template_arg_to_parm (TREE_VEC_ELT (t, 0)))
+    {
+      int level, index;
+      template_parm_level_and_index (parm, &level, &index);
+      if (index == 0 && TMPL_ARGS_DEPTH (args) >= level)
+	maybe_reuse = TMPL_ARGS_LEVEL (args, level);
+    }
+  /* If ARGS and T are both multi-level, the substituted result may be
+     identical to ARGS (if each level was pairwise identical).  */
+  else if (TMPL_ARGS_HAVE_MULTIPLE_LEVELS (t)
+	   && TMPL_ARGS_HAVE_MULTIPLE_LEVELS (args))
+    maybe_reuse = args;
+
+  /* Return MAYBE_REUSE and avoid allocating a new TREE_VEC if the substituted
+     result is identical to it.  */
+  if (maybe_reuse != NULL_TREE
+      && TREE_VEC_LENGTH (maybe_reuse) == len
+      && std::equal (elts, elts+len, TREE_VEC_BEGIN (maybe_reuse)))
+    return maybe_reuse;
+
+  /* If T consists of only a pack expansion for which substitution yielded
+     a TREE_VEC of the expanded elements, then reuse that TREE_VEC instead
+     of effectively making a copy.  */
+  if (len == 1
+      && PACK_EXPANSION_P (TREE_VEC_ELT (t, 0))
+      && TREE_CODE (elts[0]) == TREE_VEC)
+    return elts[0];
+
   /* Make space for the expanded arguments coming from template
      argument packs.  */
-  t = make_tree_vec (len + expanded_len_adjust);
-  /* ORIG_T can contain TREE_VECs. That happens if ORIG_T contains the
+  tree r = make_tree_vec (len + expanded_len_adjust);
+  /* T can contain TREE_VECs. That happens if T contains the
      arguments for a member template.
-     In that case each TREE_VEC in ORIG_T represents a level of template
-     arguments, and ORIG_T won't carry any non defaulted argument count.
+     In that case each TREE_VEC in T represents a level of template
+     arguments, and T won't carry any non defaulted argument count.
      It will rather be the nested TREE_VECs that will carry one.
-     In other words, ORIG_T carries a non defaulted argument count only
+     In other words, T carries a non defaulted argument count only
      if it doesn't contain any nested TREE_VEC.  */
-  if (NON_DEFAULT_TEMPLATE_ARGS_COUNT (orig_t))
+  if (NON_DEFAULT_TEMPLATE_ARGS_COUNT (t))
     {
-      int count = GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (orig_t);
+      int count = GET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (t);
       count += expanded_len_adjust;
-      SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (t, count);
+      SET_NON_DEFAULT_TEMPLATE_ARGS_COUNT (r, count);
     }
-  for (i = 0, out = 0; i < len; i++)
+
+  int out = 0;
+  for (int i = 0; i < len; i++)
     {
-      tree orig_arg = TREE_VEC_ELT (orig_t, i);
+      tree orig_arg = TREE_VEC_ELT (t, i);
       if (orig_arg
-	  && (PACK_EXPANSION_P (orig_arg) || ARGUMENT_PACK_P (orig_arg))
+	  && PACK_EXPANSION_P (orig_arg)
           && TREE_CODE (elts[i]) == TREE_VEC)
         {
-          int idx;
-
           /* Now expand the template argument pack "in place".  */
-          for (idx = 0; idx < TREE_VEC_LENGTH (elts[i]); idx++, out++)
-            TREE_VEC_ELT (t, out) = TREE_VEC_ELT (elts[i], idx);
+	  for (int idx = 0; idx < TREE_VEC_LENGTH (elts[i]); idx++, out++)
+	    TREE_VEC_ELT (r, out) = TREE_VEC_ELT (elts[i], idx);
         }
       else
         {
-          TREE_VEC_ELT (t, out) = elts[i];
+	  TREE_VEC_ELT (r, out) = elts[i];
           out++;
         }
     }
+  gcc_assert (out <= TREE_VEC_LENGTH (r));
 
-  return t;
+  return r;
 }
 
 /* Substitute ARGS into one level PARMS of template parameters.  */
@@ -14965,32 +15052,21 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain)
 
 	    if (!spec)
 	      {
-		int args_depth = TMPL_ARGS_DEPTH (args);
-		int parms_depth = TMPL_ARGS_DEPTH (DECL_TI_ARGS (t));
 		tmpl = DECL_TI_TEMPLATE (t);
 		gen_tmpl = most_general_template (tmpl);
-		if (args_depth == parms_depth
-		    && !PRIMARY_TEMPLATE_P (gen_tmpl))
-		  /* The DECL_TI_ARGS in this case are the generic template
-		     arguments for the enclosing class template, so we can
-		     shortcut substitution (which would just be the identity
-		     mapping).  */
-		  argvec = args;
-		else
-		  {
 		argvec = tsubst (DECL_TI_ARGS (t), args, complain, in_decl);
-		    /* Coerce the innermost arguments again if necessary.  If
-		       there's fewer levels of args than of parms, then the
-		       substitution could not have changed the innermost args
-		       (modulo level lowering).  */
-		    if (args_depth >= parms_depth && argvec != error_mark_node)
+		if (argvec != error_mark_node
+		    && PRIMARY_TEMPLATE_P (gen_tmpl)
+		    && TMPL_ARGS_DEPTH (args) >= TMPL_ARGS_DEPTH (argvec))
+		  /* We're fully specializing a template declaration, so
+		     we need to coerce the innermost arguments corresponding to
+		     the template.  */
 		  argvec = (coerce_innermost_template_parms
 			    (DECL_TEMPLATE_PARMS (gen_tmpl),
 			     argvec, t, complain,
 			     /*all*/true, /*defarg*/true));
 		if (argvec == error_mark_node)
 		  RETURN (error_mark_node);
-		  }
 		hash = spec_hasher::hash (gen_tmpl, argvec);
 		spec = retrieve_specialization (gen_tmpl, argvec, hash);
 	      }
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic183.C b/gcc/testsuite/g++.dg/cpp0x/variadic183.C
new file mode 100644
index 00000000000..27444ebb594
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic183.C
@@ -0,0 +1,14 @@
+// PR c++/105956
+// { dg-do compile { target c++11 } }
+
+template<int...> struct list;
+
+template<int... Ns> struct impl {
+  static const int idx = 0;
+  using type = list<(idx + Ns)...>;
+
+  static constexpr const int* a[2] = {(Ns, &idx)...};
+  static_assert(a[0] == &idx && a[1] == &idx, "");
+};
+
+template struct impl<0, 1>;
-- 
2.37.0.3.g30cc8d0f14