[PATCH] c++: passing partially inst tmpl as ttp [PR110566]

[PATCH] c++: passing partially inst tmpl as ttp [PR110566]

[Patrick Palka]

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk/13?

-- >8 --

Since the arguments 'pargs' passed to the coerce_template_parms from
coerce_template_template_parms are always a full set, we need to make sure
we always pass the parameters of the most general template because if the
template is partially instantiated then the levels won't match up.  In the
testcase below during said call to coerce_template_parms the parameters
are {X, Y} both level 1, but the arguments are {{int}, {N, M}}, which
leads to a crash during auto deduction of X and Y.

	PR c++/110566

gcc/cp/ChangeLog:

	* pt.cc (coerce_template_template_parms): Simplify by using
	DECL_INNERMOST_TEMPLATE_PARMS and removing redundant asserts.
	Always pass the parameters of the most general template to
	coerce_template_parms.

gcc/testsuite/ChangeLog:

	* g++.dg/template/ttp38.C: New test.
---
 gcc/cp/pt.cc                          | 12 +++++-------
 gcc/testsuite/g++.dg/template/ttp38.C | 12 ++++++++++++
 2 files changed, 17 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/ttp38.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index d882e9dd117..8723868823e 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -8073,12 +8073,10 @@ coerce_template_template_parms (tree parm_tmpl,
   tree parm, arg;
   int variadic_p = 0;
 
-  tree parm_parms = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (parm_tmpl));
-  tree arg_parms_full = DECL_TEMPLATE_PARMS (arg_tmpl);
-  tree arg_parms = INNERMOST_TEMPLATE_PARMS (arg_parms_full);
-
-  gcc_assert (TREE_CODE (parm_parms) == TREE_VEC);
-  gcc_assert (TREE_CODE (arg_parms) == TREE_VEC);
+  tree parm_parms = DECL_INNERMOST_TEMPLATE_PARMS (parm_tmpl);
+  tree arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (arg_tmpl);
+  tree gen_arg_tmpl = most_general_template (arg_tmpl);
+  tree gen_arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (gen_arg_tmpl);
 
   nparms = TREE_VEC_LENGTH (parm_parms);
   nargs = TREE_VEC_LENGTH (arg_parms);
@@ -8134,7 +8132,7 @@ coerce_template_template_parms (tree parm_tmpl,
 	scope_args = TI_ARGS (tinfo);
       pargs = add_to_template_args (scope_args, pargs);
 
-      pargs = coerce_template_parms (arg_parms, pargs, NULL_TREE, tf_none);
+      pargs = coerce_template_parms (gen_arg_parms, pargs, NULL_TREE, tf_none);
       if (pargs != error_mark_node)
 	{
 	  tree targs = make_tree_vec (nargs);
diff --git a/gcc/testsuite/g++.dg/template/ttp38.C b/gcc/testsuite/g++.dg/template/ttp38.C
new file mode 100644
index 00000000000..7d25d291e81
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ttp38.C
@@ -0,0 +1,12 @@
+// PR c++/110566
+// { dg-do compile { target c++20 } }
+
+template<template<int N, int M> class>
+struct A;
+
+template<class T>
+struct B {
+  template<int X, int Y> struct C;
+};
+
+using type = A<B<int>::C>;
-- 
2.41.0.376.gcba07a324d

Re: [PATCH] c++: passing partially inst tmpl as ttp [PR110566]

[Patrick Palka]

On Wed, Jul 19, 2023 at 2:05 PM Patrick Palka <ppalka@redhat.com> wrote:
>
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/13?
>
> -- >8 --
>
> Since the arguments 'pargs' passed to the coerce_template_parms from
> coerce_template_template_parms are always a full set, we need to make sure
> we always pass the parameters of the most general template because if the
> template is partially instantiated then the levels won't match up.  In the
> testcase below during said call to coerce_template_parms the parameters
> are {X, Y} both level 1, but the arguments are {{int}, {N, M}}, which
> leads to a crash during auto deduction of X and Y.
>
>         PR c++/110566
>
> gcc/cp/ChangeLog:
>
>         * pt.cc (coerce_template_template_parms): Simplify by using
>         DECL_INNERMOST_TEMPLATE_PARMS and removing redundant asserts.
>         Always pass the parameters of the most general template to
>         coerce_template_parms.
>
> gcc/testsuite/ChangeLog:
>
>         * g++.dg/template/ttp38.C: New test.
> ---
>  gcc/cp/pt.cc                          | 12 +++++-------
>  gcc/testsuite/g++.dg/template/ttp38.C | 12 ++++++++++++
>  2 files changed, 17 insertions(+), 7 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/template/ttp38.C
>
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index d882e9dd117..8723868823e 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -8073,12 +8073,10 @@ coerce_template_template_parms (tree parm_tmpl,
>    tree parm, arg;
>    int variadic_p = 0;
>
> -  tree parm_parms = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (parm_tmpl));
> -  tree arg_parms_full = DECL_TEMPLATE_PARMS (arg_tmpl);
> -  tree arg_parms = INNERMOST_TEMPLATE_PARMS (arg_parms_full);
> -
> -  gcc_assert (TREE_CODE (parm_parms) == TREE_VEC);
> -  gcc_assert (TREE_CODE (arg_parms) == TREE_VEC);
> +  tree parm_parms = DECL_INNERMOST_TEMPLATE_PARMS (parm_tmpl);
> +  tree arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (arg_tmpl);
> +  tree gen_arg_tmpl = most_general_template (arg_tmpl);
> +  tree gen_arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (gen_arg_tmpl);
>
>    nparms = TREE_VEC_LENGTH (parm_parms);
>    nargs = TREE_VEC_LENGTH (arg_parms);
> @@ -8134,7 +8132,7 @@ coerce_template_template_parms (tree parm_tmpl,
>         scope_args = TI_ARGS (tinfo);
>        pargs = add_to_template_args (scope_args, pargs);
>
> -      pargs = coerce_template_parms (arg_parms, pargs, NULL_TREE, tf_none);
> +      pargs = coerce_template_parms (gen_arg_parms, pargs, NULL_TREE, tf_none);
>        if (pargs != error_mark_node)
>         {
>           tree targs = make_tree_vec (nargs);
> diff --git a/gcc/testsuite/g++.dg/template/ttp38.C b/gcc/testsuite/g++.dg/template/ttp38.C
> new file mode 100644
> index 00000000000..7d25d291e81
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp38.C
> @@ -0,0 +1,12 @@
> +// PR c++/110566
> +// { dg-do compile { target c++20 } }
> +
> +template<template<int N, int M> class>
> +struct A;
> +
> +template<class T>
> +struct B {
> +  template<int X, int Y> struct C;

Oops, I botched a git commit --amend.  The parameter list here should
be 'auto X, auto Y'.

> +};
> +
> +using type = A<B<int>::C>;
> --
> 2.41.0.376.gcba07a324d
>

Re: [PATCH] c++: passing partially inst tmpl as ttp [PR110566]

[Jason Merrill]

On 7/19/23 14:05, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/13?
> 
> -- >8 --
> 
> Since the arguments 'pargs' passed to the coerce_template_parms from
> coerce_template_template_parms are always a full set, we need to make sure
> we always pass the parameters of the most general template because if the
> template is partially instantiated then the levels won't match up.

Hmm, so then my comment "In that case we might end up adding more levels 
than needed, but that shouldn't be a problem; any args we need to refer 
to are at the right level." is wrong for auto template parms?

So I guess we likely need to do more to assure that pargs has the right 
number of levels if there are autos in the innermost arg parms.

Also, most_general_template doesn't work for TTPs, so that probably 
won't help handle their partial instantiations.

And is it right for an alias template in a partial specialization?

> In the
> testcase below during said call to coerce_template_parms the parameters
> are {X, Y} both level 1, but the arguments are {{int}, {N, M}}, which
> leads to a crash during auto deduction of X and Y.
> 
> 	PR c++/110566

Since this is a regression from the patch for PR c++/108179, please list 
that PR here as well, to help avoid backporting that patch without this one.

> gcc/cp/ChangeLog:
> 
> 	* pt.cc (coerce_template_template_parms): Simplify by using
> 	DECL_INNERMOST_TEMPLATE_PARMS and removing redundant asserts.
> 	Always pass the parameters of the most general template to
> 	coerce_template_parms.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/ttp38.C: New test.
> ---
>   gcc/cp/pt.cc                          | 12 +++++-------
>   gcc/testsuite/g++.dg/template/ttp38.C | 12 ++++++++++++
>   2 files changed, 17 insertions(+), 7 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/ttp38.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index d882e9dd117..8723868823e 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -8073,12 +8073,10 @@ coerce_template_template_parms (tree parm_tmpl,
>     tree parm, arg;
>     int variadic_p = 0;
>   
> -  tree parm_parms = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (parm_tmpl));
> -  tree arg_parms_full = DECL_TEMPLATE_PARMS (arg_tmpl);
> -  tree arg_parms = INNERMOST_TEMPLATE_PARMS (arg_parms_full);
> -
> -  gcc_assert (TREE_CODE (parm_parms) == TREE_VEC);
> -  gcc_assert (TREE_CODE (arg_parms) == TREE_VEC);
> +  tree parm_parms = DECL_INNERMOST_TEMPLATE_PARMS (parm_tmpl);
> +  tree arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (arg_tmpl);
> +  tree gen_arg_tmpl = most_general_template (arg_tmpl);
> +  tree gen_arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (gen_arg_tmpl);
>   
>     nparms = TREE_VEC_LENGTH (parm_parms);
>     nargs = TREE_VEC_LENGTH (arg_parms);
> @@ -8134,7 +8132,7 @@ coerce_template_template_parms (tree parm_tmpl,
>   	scope_args = TI_ARGS (tinfo);
>         pargs = add_to_template_args (scope_args, pargs);
>   
> -      pargs = coerce_template_parms (arg_parms, pargs, NULL_TREE, tf_none);
> +      pargs = coerce_template_parms (gen_arg_parms, pargs, NULL_TREE, tf_none);
>         if (pargs != error_mark_node)
>   	{
>   	  tree targs = make_tree_vec (nargs);
> diff --git a/gcc/testsuite/g++.dg/template/ttp38.C b/gcc/testsuite/g++.dg/template/ttp38.C
> new file mode 100644
> index 00000000000..7d25d291e81
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp38.C
> @@ -0,0 +1,12 @@
> +// PR c++/110566
> +// { dg-do compile { target c++20 } }
> +
> +template<template<int N, int M> class>
> +struct A;
> +
> +template<class T>
> +struct B {
> +  template<int X, int Y> struct C;
> +};
> +
> +using type = A<B<int>::C>;

Re: [PATCH] c++: passing partially inst tmpl as ttp [PR110566]

[Patrick Palka]

On Wed, 19 Jul 2023, Jason Merrill wrote:

> On 7/19/23 14:05, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk/13?
> > 
> > -- >8 --
> > 
> > Since the arguments 'pargs' passed to the coerce_template_parms from
> > coerce_template_template_parms are always a full set, we need to make sure
> > we always pass the parameters of the most general template because if the
> > template is partially instantiated then the levels won't match up.
> 
> Hmm, so then my comment "In that case we might end up adding more levels than
> needed, but that shouldn't be a problem; any args we need to refer to are at
> the right level." is wrong for auto template parms?

I suppose, but only for the ttp case I think?  It seems all is well when
passing an ordinary template as a ttp as long as we use the parameters of the
most general template for the coercion.  I can't come up with a counterexample
at least.

> 
> So I guess we likely need to do more to assure that pargs has the right number
> of levels if there are autos in the innermost arg parms.
> 
> Also, most_general_template doesn't work for TTPs, so that probably won't help
> handle their partial instantiations.

Ah, yeah :( Here's an analagous testcase that we still ICE on due to this:

  template<template<int N, int M> class>
  struct A;

  template<class U>
  struct B {
    template<class T>
    struct C {
      template<template<auto X, auto Y> class TT>
      using type = A<TT>;
    };
  };

  template struct B<int>::C<int>;

I think I have a fix using get_innermost_template_args.

> 
> And is it right for an alias template in a partial specialization?

IIUC yes, e.g. tsubst directly relies on most_general_template to work
for alias templates (when substituting an alias template specialization).

> 
> > In the
> > testcase below during said call to coerce_template_parms the parameters
> > are {X, Y} both level 1, but the arguments are {{int}, {N, M}}, which
> > leads to a crash during auto deduction of X and Y.
> > 
> > 	PR c++/110566
> 
> Since this is a regression from the patch for PR c++/108179, please list that
> PR here as well, to help avoid backporting that patch without this one.
> 
> > gcc/cp/ChangeLog:
> > 
> > 	* pt.cc (coerce_template_template_parms): Simplify by using
> > 	DECL_INNERMOST_TEMPLATE_PARMS and removing redundant asserts.
> > 	Always pass the parameters of the most general template to
> > 	coerce_template_parms.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/template/ttp38.C: New test.
> > ---
> >   gcc/cp/pt.cc                          | 12 +++++-------
> >   gcc/testsuite/g++.dg/template/ttp38.C | 12 ++++++++++++
> >   2 files changed, 17 insertions(+), 7 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/template/ttp38.C
> > 
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index d882e9dd117..8723868823e 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -8073,12 +8073,10 @@ coerce_template_template_parms (tree parm_tmpl,
> >     tree parm, arg;
> >     int variadic_p = 0;
> >   -  tree parm_parms = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS
> > (parm_tmpl));
> > -  tree arg_parms_full = DECL_TEMPLATE_PARMS (arg_tmpl);
> > -  tree arg_parms = INNERMOST_TEMPLATE_PARMS (arg_parms_full);
> > -
> > -  gcc_assert (TREE_CODE (parm_parms) == TREE_VEC);
> > -  gcc_assert (TREE_CODE (arg_parms) == TREE_VEC);
> > +  tree parm_parms = DECL_INNERMOST_TEMPLATE_PARMS (parm_tmpl);
> > +  tree arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (arg_tmpl);
> > +  tree gen_arg_tmpl = most_general_template (arg_tmpl);
> > +  tree gen_arg_parms = DECL_INNERMOST_TEMPLATE_PARMS (gen_arg_tmpl);
> >       nparms = TREE_VEC_LENGTH (parm_parms);
> >     nargs = TREE_VEC_LENGTH (arg_parms);
> > @@ -8134,7 +8132,7 @@ coerce_template_template_parms (tree parm_tmpl,
> >   	scope_args = TI_ARGS (tinfo);
> >         pargs = add_to_template_args (scope_args, pargs);
> >   -      pargs = coerce_template_parms (arg_parms, pargs, NULL_TREE,
> > tf_none);
> > +      pargs = coerce_template_parms (gen_arg_parms, pargs, NULL_TREE,
> > tf_none);
> >         if (pargs != error_mark_node)
> >   	{
> >   	  tree targs = make_tree_vec (nargs);
> > diff --git a/gcc/testsuite/g++.dg/template/ttp38.C
> > b/gcc/testsuite/g++.dg/template/ttp38.C
> > new file mode 100644
> > index 00000000000..7d25d291e81
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/template/ttp38.C
> > @@ -0,0 +1,12 @@
> > +// PR c++/110566
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<template<int N, int M> class>
> > +struct A;
> > +
> > +template<class T>
> > +struct B {
> > +  template<int X, int Y> struct C;
> > +};
> > +
> > +using type = A<B<int>::C>;
> 
> 

Re: [PATCH] c++: passing partially inst tmpl as ttp [PR110566]

[Jason Merrill]

On 7/20/23 12:00, Patrick Palka wrote:
> On Wed, 19 Jul 2023, Jason Merrill wrote:
> 
>> On 7/19/23 14:05, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk/13?
>>>
>>> -- >8 --
>>>
>>> Since the arguments 'pargs' passed to the coerce_template_parms from
>>> coerce_template_template_parms are always a full set, we need to make sure
>>> we always pass the parameters of the most general template because if the
>>> template is partially instantiated then the levels won't match up.
>>
>> Hmm, so then my comment "In that case we might end up adding more levels than
>> needed, but that shouldn't be a problem; any args we need to refer to are at
>> the right level." is wrong for auto template parms?
> 
> I suppose, but only for the ttp case I think?  It seems all is well when
> passing an ordinary template as a ttp as long as we use the parameters of the
> most general template for the coercion.  I can't come up with a counterexample
> at least.

Agreed; the quoted comment is already only about passing a TTP, as 
ordinary templates always have DECL_CONTEXT set properly.

Jason