c++: Don't set DECL_CONTEXT to nested template-template parameters [PR98881]

Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?

-- >8 --

When streaming in a nested template-template parameter as in the
attached testcase, we end up reaching the containing template-template
parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
this (nested) template-template parameter, as it should already be the
struct that the outer template-template parameter is declared on.

	PR c++/98881

gcc/cp/ChangeLog:

	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
	for checking purposes. Don't consider a template template
	parameter as the owning template.
	(trees_in::tpl_parms_fini): Don't consider a template template
	parameter as the owning template.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
---
 gcc/cp/module.cc                                | 17 ++++++++++++-----
 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
 3 files changed, 36 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C

On 3/2/24 01:54, Nathaniel Shead wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> When streaming in a nested template-template parameter as in the
> attached testcase, we end up reaching the containing template-template
> parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> this (nested) template-template parameter, as it should already be the
> struct that the outer template-template parameter is declared on.

So in the case where tmpl is a template template parameter we want 
DECL_CONTEXT (parm) to be the same as DECL_CONTEXT (tmpl)?  Let's check 
that instead of ignoring it.

> 	PR c++/98881
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> 	for checking purposes. Don't consider a template template
> 	parameter as the owning template.
> 	(trees_in::tpl_parms_fini): Don't consider a template template
> 	parameter as the owning template.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>   gcc/cp/module.cc                                | 17 ++++++++++++-----
>   gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
>   gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
>   3 files changed, 36 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 67f132d28d7..5663d01ed9c 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>   	  tree dflt = TREE_PURPOSE (parm);
>   	  tree_node (dflt);
>   
> -	  if (streaming_p ())
> +	  if (CHECKING_P && streaming_p ())
>   	    {
> +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> +		 streaming in is correct.  */
>   	      tree decl = TREE_VALUE (parm);
> -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> +		  /* A template template parm is not the owning template.  */
> +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
>   		{
>   		  tree ctx = DECL_CONTEXT (decl);
>   		  tree inner = DECL_TEMPLATE_RESULT (decl);
> @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>   	    return false;
>   	  TREE_PURPOSE (parm) = dflt;
>   
> +	  /* Original template template parms have a context
> +	     of their owning template.  Reduced ones do not.
> +	     But if TMPL is itself a template template parm
> +	     then it cannot be the owning template.  */
>   	  tree decl = TREE_VALUE (parm);
> -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
>   	    {
>   	      tree inner = DECL_TEMPLATE_RESULT (decl);
>   	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>   			  : DECL_INITIAL (inner));
>   	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
>   			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -	      /* Original template template parms have a context
> -		 of their owning template.  Reduced ones do not.  */
>   	      if (original)
>   		DECL_CONTEXT (decl) = tmpl;
>   	    }
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> new file mode 100644
> index 00000000000..21bbc054fa3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> @@ -0,0 +1,11 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +template <typename P> struct X {};
> +
> +template<template <typename> typename TT>
> +struct X<TT<int>> {
> +  template<template <typename> typename UU>
> +  void f (X<UU<int>>&);
> +};
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> new file mode 100644
> index 00000000000..234e822faa9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> @@ -0,0 +1,13 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "tpl-tpl-parm-3_a.H";
> +
> +template <typename T> struct Y {};
> +template <typename T> struct Z {};
> +
> +void foo() {
> +  X<Y<int>> y;
> +  X<Z<int>> z;
> +  y.f(z);
> +}

On Mon, Mar 04, 2024 at 06:01:48PM -0500, Jason Merrill wrote:
> On 3/2/24 01:54, Nathaniel Shead wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > 
> > -- >8 --
> > 
> > When streaming in a nested template-template parameter as in the
> > attached testcase, we end up reaching the containing template-template
> > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > this (nested) template-template parameter, as it should already be the
> > struct that the outer template-template parameter is declared on.
> 
> So in the case where tmpl is a template template parameter we want
> DECL_CONTEXT (parm) to be the same as DECL_CONTEXT (tmpl)?  Let's check that
> instead of ignoring it.

No, I don't think so. I guess the closest is that we if we keep
iterating through all the nested 'DECL_CONTEXT's of tmpl we should
eventually reach the template result of parm's context, if we find
enough template infos etc.? I'm not entirely sure how to go about this.

But in particular, in the current test case we have:

- tmpl = <template_decl 0x7ffff7ffa300 UU>
- DECL_CONTEXT (tmpl) = <template_decl 0x7ffff7ffa500 f>

- decl = <template_decl 0x7ffff7ffa180 TT>
- DECL_CONTEXT (decl) = <template_decl 0x7ffff7ffa200 X> 

('decl' is the declaration associated with 'parm', a tree_list.)
And it's not immediately obvious to me how to unify these.

> > 	PR c++/98881
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> > 	for checking purposes. Don't consider a template template
> > 	parameter as the owning template.
> > 	(trees_in::tpl_parms_fini): Don't consider a template template
> > 	parameter as the owning template.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> >   gcc/cp/module.cc                                | 17 ++++++++++++-----
> >   gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
> >   gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
> >   3 files changed, 36 insertions(+), 5 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> >   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 67f132d28d7..5663d01ed9c 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >   	  tree dflt = TREE_PURPOSE (parm);
> >   	  tree_node (dflt);
> > -	  if (streaming_p ())
> > +	  if (CHECKING_P && streaming_p ())
> >   	    {
> > +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> > +		 streaming in is correct.  */
> >   	      tree decl = TREE_VALUE (parm);
> > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> > +		  /* A template template parm is not the owning template.  */
> > +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> >   		{
> >   		  tree ctx = DECL_CONTEXT (decl);
> >   		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >   	    return false;
> >   	  TREE_PURPOSE (parm) = dflt;
> > +	  /* Original template template parms have a context
> > +	     of their owning template.  Reduced ones do not.
> > +	     But if TMPL is itself a template template parm
> > +	     then it cannot be the owning template.  */
> >   	  tree decl = TREE_VALUE (parm);
> > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> > +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> >   	    {
> >   	      tree inner = DECL_TEMPLATE_RESULT (decl);
> >   	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >   			  : DECL_INITIAL (inner));
> >   	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> >   			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > -	      /* Original template template parms have a context
> > -		 of their owning template.  Reduced ones do not.  */
> >   	      if (original)
> >   		DECL_CONTEXT (decl) = tmpl;
> >   	    }
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > new file mode 100644
> > index 00000000000..21bbc054fa3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > @@ -0,0 +1,11 @@
> > +// PR c++/98881
> > +// { dg-additional-options "-fmodule-header" }
> > +// { dg-module-cmi {} }
> > +
> > +template <typename P> struct X {};
> > +
> > +template<template <typename> typename TT>
> > +struct X<TT<int>> {
> > +  template<template <typename> typename UU>
> > +  void f (X<UU<int>>&);
> > +};
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > new file mode 100644
> > index 00000000000..234e822faa9
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > @@ -0,0 +1,13 @@
> > +// PR c++/98881
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import "tpl-tpl-parm-3_a.H";
> > +
> > +template <typename T> struct Y {};
> > +template <typename T> struct Z {};
> > +
> > +void foo() {
> > +  X<Y<int>> y;
> > +  X<Z<int>> z;
> > +  y.f(z);
> > +}
>

On Sat, 2 Mar 2024, Nathaniel Shead wrote:

> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> 
> -- >8 --
> 
> When streaming in a nested template-template parameter as in the
> attached testcase, we end up reaching the containing template-template
> parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> this (nested) template-template parameter, as it should already be the
> struct that the outer template-template parameter is declared on.
> 
> 	PR c++/98881
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> 	for checking purposes. Don't consider a template template
> 	parameter as the owning template.
> 	(trees_in::tpl_parms_fini): Don't consider a template template
> 	parameter as the owning template.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> ---
>  gcc/cp/module.cc                                | 17 ++++++++++++-----
>  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
>  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
>  3 files changed, 36 insertions(+), 5 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 67f132d28d7..5663d01ed9c 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>  	  tree dflt = TREE_PURPOSE (parm);
>  	  tree_node (dflt);
>  
> -	  if (streaming_p ())
> +	  if (CHECKING_P && streaming_p ())
>  	    {
> +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> +		 streaming in is correct.  */
>  	      tree decl = TREE_VALUE (parm);
> -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> +		  /* A template template parm is not the owning template.  */
> +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
>  		{
>  		  tree ctx = DECL_CONTEXT (decl);
>  		  tree inner = DECL_TEMPLATE_RESULT (decl);
> @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>  	    return false;
>  	  TREE_PURPOSE (parm) = dflt;
>  
> +	  /* Original template template parms have a context
> +	     of their owning template.  Reduced ones do not.
> +	     But if TMPL is itself a template template parm
> +	     then it cannot be the owning template.  */
>  	  tree decl = TREE_VALUE (parm);
> -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))

IIUC a TEMPLATE_DECL inside a template parameter list always represents
a template template parm, so won't this effectively disable the
DECL_CONTEXT setting logic?

>  	    {
>  	      tree inner = DECL_TEMPLATE_RESULT (decl);
>  	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>  			  : DECL_INITIAL (inner));
>  	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
>  			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -	      /* Original template template parms have a context
> -		 of their owning template.  Reduced ones do not.  */
>  	      if (original)
>  		DECL_CONTEXT (decl) = tmpl;
>  	    }
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> new file mode 100644
> index 00000000000..21bbc054fa3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> @@ -0,0 +1,11 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +template <typename P> struct X {};
> +
> +template<template <typename> typename TT>
> +struct X<TT<int>> {
> +  template<template <typename> typename UU>
> +  void f (X<UU<int>>&);
> +};

I wonder why the partial specialization is relevant here?  I can't
seem to trigger the ICE without the partial specialization.
Slightly further reduced to not use bound ttps:

    template<template<class> class TT>
    struct X { };

    template<template<class> class TT> requires true
    struct X<TT> {
      template<template<class> class UU>
      void f(X<UU>);
    };

Maybe the expectation is that tpl_parms_fini for UU should be called
with tpl_levels=1 (so that we stream only its own template parameters)
but it's instead called with tpl_levels=3 for some reason?

IIUC that assert should always hold in the first iteration of the loop
(for the ttp's own template parameters).  Perhaps for subsequent
iterations we need to actually stream the contexts?

Ah, but we also ICE in the same spot with:

  template<template<class> class TT> struct X;      // #1
  template<template<class> class UU> struct X { };  // #2

because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
of each ttp as-is instead of trying to avoid it via an apparently
fragile assumption?

> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> new file mode 100644
> index 00000000000..234e822faa9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> @@ -0,0 +1,13 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "tpl-tpl-parm-3_a.H";
> +
> +template <typename T> struct Y {};
> +template <typename T> struct Z {};
> +
> +void foo() {
> +  X<Y<int>> y;
> +  X<Z<int>> z;
> +  y.f(z);
> +}
> -- 
> 2.43.2
> 
>

On Mon, Mar 04, 2024 at 07:14:54PM -0500, Patrick Palka wrote:
> On Sat, 2 Mar 2024, Nathaniel Shead wrote:
> 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > 
> > -- >8 --
> > 
> > When streaming in a nested template-template parameter as in the
> > attached testcase, we end up reaching the containing template-template
> > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > this (nested) template-template parameter, as it should already be the
> > struct that the outer template-template parameter is declared on.
> > 
> > 	PR c++/98881
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> > 	for checking purposes. Don't consider a template template
> > 	parameter as the owning template.
> > 	(trees_in::tpl_parms_fini): Don't consider a template template
> > 	parameter as the owning template.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > ---
> >  gcc/cp/module.cc                                | 17 ++++++++++++-----
> >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
> >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
> >  3 files changed, 36 insertions(+), 5 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 67f132d28d7..5663d01ed9c 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >  	  tree dflt = TREE_PURPOSE (parm);
> >  	  tree_node (dflt);
> >  
> > -	  if (streaming_p ())
> > +	  if (CHECKING_P && streaming_p ())
> >  	    {
> > +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> > +		 streaming in is correct.  */
> >  	      tree decl = TREE_VALUE (parm);
> > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> > +		  /* A template template parm is not the owning template.  */
> > +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> >  		{
> >  		  tree ctx = DECL_CONTEXT (decl);
> >  		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >  	    return false;
> >  	  TREE_PURPOSE (parm) = dflt;
> >  
> > +	  /* Original template template parms have a context
> > +	     of their owning template.  Reduced ones do not.
> > +	     But if TMPL is itself a template template parm
> > +	     then it cannot be the owning template.  */
> >  	  tree decl = TREE_VALUE (parm);
> > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> > +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> 
> IIUC a TEMPLATE_DECL inside a template parameter list always represents
> a template template parm, so won't this effectively disable the
> DECL_CONTEXT setting logic?

This is only when 'tmpl' (i.e. the containing TEMPLATE_DECL that we're
streaming) is itself a template template parm.

> >  	    {
> >  	      tree inner = DECL_TEMPLATE_RESULT (decl);
> >  	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >  			  : DECL_INITIAL (inner));
> >  	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> >  			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > -	      /* Original template template parms have a context
> > -		 of their owning template.  Reduced ones do not.  */
> >  	      if (original)
> >  		DECL_CONTEXT (decl) = tmpl;
> >  	    }
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > new file mode 100644
> > index 00000000000..21bbc054fa3
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > @@ -0,0 +1,11 @@
> > +// PR c++/98881
> > +// { dg-additional-options "-fmodule-header" }
> > +// { dg-module-cmi {} }
> > +
> > +template <typename P> struct X {};
> > +
> > +template<template <typename> typename TT>
> > +struct X<TT<int>> {
> > +  template<template <typename> typename UU>
> > +  void f (X<UU<int>>&);
> > +};
> 
> I wonder why the partial specialization is relevant here?  I can't
> seem to trigger the ICE without the partial specialization.
> Slightly further reduced to not use bound ttps:
> 
>     template<template<class> class TT>
>     struct X { };
> 
>     template<template<class> class TT> requires true
>     struct X<TT> {
>       template<template<class> class UU>
>       void f(X<UU>);
>     };
> 
> Maybe the expectation is that tpl_parms_fini for UU should be called
> with tpl_levels=1 (so that we stream only its own template parameters)
> but it's instead called with tpl_levels=3 for some reason?
> 
> IIUC that assert should always hold in the first iteration of the loop
> (for the ttp's own template parameters).  Perhaps for subsequent
> iterations we need to actually stream the contexts?
> 
> Ah, but we also ICE in the same spot with:
> 
>   template<template<class> class TT> struct X;      // #1
>   template<template<class> class UU> struct X { };  // #2
> 
> because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
> bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
> of each ttp as-is instead of trying to avoid it via an apparently
> fragile assumption?
> 

That seems reasonable to me. Probably better to aim for correctness at
this stage rather than relying on things I don't understand for slight
performance gains.

I started putting together a patch to always restream the DECL_CONTEXT,
but as I was doing so I realised that actually, we stream the context
anyway, and this value should be correct to start with? I don't think
deduplication should affect this, but I've added to the test to make
sure that there's no issues here that I can see.

So maybe something like this? Or might there be something I'm missing?

Bootstrapped and regtested (only modules.exp so far) on
x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?

-- >8 --

Subject: [PATCH] c++: Remove special handling of DECL_CONTEXT for template template parms [PR98881]

When streaming in a nested template-template parameter as in the
attached testcase, we end up reaching the containing template-template
parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
this (nested) template-template parameter, as it should already be the
struct that the outer template-template parameter is declared on.

The precise logic for what DECL_CONTEXT should be for a template
template parameter in various situations seems rather obscure. However,
it turns out in this case that the correct solution is to just do less
work: normal streaming already streams the correct value of DECL_CONTEXT
anyway, so there should be no need to redetermine them here.

	PR c++/98881

gcc/cp/ChangeLog:

	* module.cc (trees_out::tpl_parms_fini): Remove special handling
	for DECL_CONTEXT of template template parameters.
	(trees_in::tpl_parms_fini): Likewise.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/tpl-tpl-parm-3.h: New test.
	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
	* g++.dg/modules/tpl-tpl-parm-3_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Patrick Palka <ppalka@redhat.com>
Reviewed-by: Jason Merrill <jason@redhat.com> 
---
 gcc/cp/module.cc                              | 33 -------------------
 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h | 12 +++++++
 .../g++.dg/modules/tpl-tpl-parm-3_a.H         |  5 +++
 .../g++.dg/modules/tpl-tpl-parm-3_b.C         |  5 +++
 .../g++.dg/modules/tpl-tpl-parm-3_c.C         | 15 +++++++++
 5 files changed, 37 insertions(+), 33 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 67f132d28d7..dbb851baa40 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -10125,24 +10125,6 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
 	  tree parm = TREE_VEC_ELT (vec, ix);
 	  tree dflt = TREE_PURPOSE (parm);
 	  tree_node (dflt);
-
-	  if (streaming_p ())
-	    {
-	      tree decl = TREE_VALUE (parm);
-	      if (TREE_CODE (decl) == TEMPLATE_DECL)
-		{
-		  tree ctx = DECL_CONTEXT (decl);
-		  tree inner = DECL_TEMPLATE_RESULT (decl);
-		  tree tpi = (TREE_CODE (inner) == TYPE_DECL
-			      ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
-			      : DECL_INITIAL (inner));
-		  bool original = (TEMPLATE_PARM_LEVEL (tpi)
-				   == TEMPLATE_PARM_ORIG_LEVEL (tpi));
-		  /* Original template template parms have a context
-		     of their owning template.  Reduced ones do not.  */
-		  gcc_checking_assert (original ? ctx == tmpl : !ctx);
-		}
-	    }
 	}
     }
 }
@@ -10163,21 +10145,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
 	  if (get_overrun ())
 	    return false;
 	  TREE_PURPOSE (parm) = dflt;
-
-	  tree decl = TREE_VALUE (parm);
-	  if (TREE_CODE (decl) == TEMPLATE_DECL)
-	    {
-	      tree inner = DECL_TEMPLATE_RESULT (decl);
-	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
-			  ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
-			  : DECL_INITIAL (inner));
-	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
-			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
-	      /* Original template template parms have a context
-		 of their owning template.  Reduced ones do not.  */
-	      if (original)
-		DECL_CONTEXT (decl) = tmpl;
-	    }
 	}
     }
   return true;
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
new file mode 100644
index 00000000000..2fc088f0f39
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
@@ -0,0 +1,12 @@
+// PR c++/98881
+
+template <typename P> struct X {};
+
+template<template <typename> typename TT>
+struct X<TT<int>> {
+  template<template <typename> typename UU>
+  void f (X<UU<int>>&);
+};
+
+template<template<class> class TT> struct Y;      // #1
+template<template<class> class UU> struct Y { };  // #2
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
new file mode 100644
index 00000000000..47464081f25
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
@@ -0,0 +1,5 @@
+// PR c++/98881
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+#include "tpl-tpl-parm-3.h"
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
new file mode 100644
index 00000000000..1e3cc6c444f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
@@ -0,0 +1,5 @@
+// PR c++/98881
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+
+#include "tpl-tpl-parm-3.h"
+import "tpl-tpl-parm-3_a.H";
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
new file mode 100644
index 00000000000..5edba6cc64f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
@@ -0,0 +1,15 @@
+// PR c++/98881
+// { dg-additional-options "-fmodules-ts" }
+
+import "tpl-tpl-parm-3_a.H";
+
+template <typename T> struct A {};
+template <typename T> struct B {};
+
+void foo() {
+  X<A<int>> a;
+  X<B<int>> b;
+  a.f(b);
+
+  Y<A> y;
+}

On Tue, 5 Mar 2024, Nathaniel Shead wrote:

> On Mon, Mar 04, 2024 at 07:14:54PM -0500, Patrick Palka wrote:
> > On Sat, 2 Mar 2024, Nathaniel Shead wrote:
> > 
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > 
> > > -- >8 --
> > > 
> > > When streaming in a nested template-template parameter as in the
> > > attached testcase, we end up reaching the containing template-template
> > > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > > this (nested) template-template parameter, as it should already be the
> > > struct that the outer template-template parameter is declared on.
> > > 
> > > 	PR c++/98881
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> > > 	for checking purposes. Don't consider a template template
> > > 	parameter as the owning template.
> > > 	(trees_in::tpl_parms_fini): Don't consider a template template
> > > 	parameter as the owning template.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > > 
> > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > ---
> > >  gcc/cp/module.cc                                | 17 ++++++++++++-----
> > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
> > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
> > >  3 files changed, 36 insertions(+), 5 deletions(-)
> > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > > 
> > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > index 67f132d28d7..5663d01ed9c 100644
> > > --- a/gcc/cp/module.cc
> > > +++ b/gcc/cp/module.cc
> > > @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > >  	  tree dflt = TREE_PURPOSE (parm);
> > >  	  tree_node (dflt);
> > >  
> > > -	  if (streaming_p ())
> > > +	  if (CHECKING_P && streaming_p ())
> > >  	    {
> > > +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> > > +		 streaming in is correct.  */
> > >  	      tree decl = TREE_VALUE (parm);
> > > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> > > +		  /* A template template parm is not the owning template.  */
> > > +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > >  		{
> > >  		  tree ctx = DECL_CONTEXT (decl);
> > >  		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > > @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > >  	    return false;
> > >  	  TREE_PURPOSE (parm) = dflt;
> > >  
> > > +	  /* Original template template parms have a context
> > > +	     of their owning template.  Reduced ones do not.
> > > +	     But if TMPL is itself a template template parm
> > > +	     then it cannot be the owning template.  */
> > >  	  tree decl = TREE_VALUE (parm);
> > > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> > > +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > 
> > IIUC a TEMPLATE_DECL inside a template parameter list always represents
> > a template template parm, so won't this effectively disable the
> > DECL_CONTEXT setting logic?
> 
> This is only when 'tmpl' (i.e. the containing TEMPLATE_DECL that we're
> streaming) is itself a template template parm.

D'oh, makes sense.

> 
> > >  	    {
> > >  	      tree inner = DECL_TEMPLATE_RESULT (decl);
> > >  	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > > @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > >  			  : DECL_INITIAL (inner));
> > >  	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > >  			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > > -	      /* Original template template parms have a context
> > > -		 of their owning template.  Reduced ones do not.  */
> > >  	      if (original)
> > >  		DECL_CONTEXT (decl) = tmpl;
> > >  	    }
> > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > new file mode 100644
> > > index 00000000000..21bbc054fa3
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > @@ -0,0 +1,11 @@
> > > +// PR c++/98881
> > > +// { dg-additional-options "-fmodule-header" }
> > > +// { dg-module-cmi {} }
> > > +
> > > +template <typename P> struct X {};
> > > +
> > > +template<template <typename> typename TT>
> > > +struct X<TT<int>> {
> > > +  template<template <typename> typename UU>
> > > +  void f (X<UU<int>>&);
> > > +};
> > 
> > I wonder why the partial specialization is relevant here?  I can't
> > seem to trigger the ICE without the partial specialization.
> > Slightly further reduced to not use bound ttps:
> > 
> >     template<template<class> class TT>
> >     struct X { };
> > 
> >     template<template<class> class TT> requires true
> >     struct X<TT> {
> >       template<template<class> class UU>
> >       void f(X<UU>);
> >     };
> > 
> > Maybe the expectation is that tpl_parms_fini for UU should be called
> > with tpl_levels=1 (so that we stream only its own template parameters)
> > but it's instead called with tpl_levels=3 for some reason?
> > 
> > IIUC that assert should always hold in the first iteration of the loop
> > (for the ttp's own template parameters).  Perhaps for subsequent
> > iterations we need to actually stream the contexts?
> > 
> > Ah, but we also ICE in the same spot with:
> > 
> >   template<template<class> class TT> struct X;      // #1
> >   template<template<class> class UU> struct X { };  // #2
> > 
> > because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
> > bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
> > of each ttp as-is instead of trying to avoid it via an apparently
> > fragile assumption?
> > 
> 
> That seems reasonable to me. Probably better to aim for correctness at
> this stage rather than relying on things I don't understand for slight
> performance gains.
> 
> I started putting together a patch to always restream the DECL_CONTEXT,
> but as I was doing so I realised that actually, we stream the context
> anyway, and this value should be correct to start with? I don't think
> deduplication should affect this, but I've added to the test to make
> sure that there's no issues here that I can see.

Where do we otherwise stream the context?  In trees_out::core_vals we
seem to avoid streaming the context for template parms (including ttps):

    if (!DECL_TEMPLATE_PARM_P (t))
      WT (t->decl_minimal.context);

> 
> So maybe something like this? Or might there be something I'm missing?
> 
> Bootstrapped and regtested (only modules.exp so far) on
> x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
> 
> -- >8 --
> 
> Subject: [PATCH] c++: Remove special handling of DECL_CONTEXT for template template parms [PR98881]
> 
> When streaming in a nested template-template parameter as in the
> attached testcase, we end up reaching the containing template-template
> parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> this (nested) template-template parameter, as it should already be the
> struct that the outer template-template parameter is declared on.
> 
> The precise logic for what DECL_CONTEXT should be for a template
> template parameter in various situations seems rather obscure. However,
> it turns out in this case that the correct solution is to just do less
> work: normal streaming already streams the correct value of DECL_CONTEXT
> anyway, so there should be no need to redetermine them here.
> 
> 	PR c++/98881
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_out::tpl_parms_fini): Remove special handling
> 	for DECL_CONTEXT of template template parameters.
> 	(trees_in::tpl_parms_fini): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/tpl-tpl-parm-3.h: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> Reviewed-by: Patrick Palka <ppalka@redhat.com>
> Reviewed-by: Jason Merrill <jason@redhat.com> 
> ---
>  gcc/cp/module.cc                              | 33 -------------------
>  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h | 12 +++++++
>  .../g++.dg/modules/tpl-tpl-parm-3_a.H         |  5 +++
>  .../g++.dg/modules/tpl-tpl-parm-3_b.C         |  5 +++
>  .../g++.dg/modules/tpl-tpl-parm-3_c.C         | 15 +++++++++
>  5 files changed, 37 insertions(+), 33 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 67f132d28d7..dbb851baa40 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -10125,24 +10125,6 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>  	  tree parm = TREE_VEC_ELT (vec, ix);
>  	  tree dflt = TREE_PURPOSE (parm);
>  	  tree_node (dflt);
> -
> -	  if (streaming_p ())
> -	    {
> -	      tree decl = TREE_VALUE (parm);
> -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> -		{
> -		  tree ctx = DECL_CONTEXT (decl);
> -		  tree inner = DECL_TEMPLATE_RESULT (decl);
> -		  tree tpi = (TREE_CODE (inner) == TYPE_DECL
> -			      ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> -			      : DECL_INITIAL (inner));
> -		  bool original = (TEMPLATE_PARM_LEVEL (tpi)
> -				   == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -		  /* Original template template parms have a context
> -		     of their owning template.  Reduced ones do not.  */
> -		  gcc_checking_assert (original ? ctx == tmpl : !ctx);
> -		}
> -	    }
>  	}
>      }
>  }
> @@ -10163,21 +10145,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>  	  if (get_overrun ())
>  	    return false;
>  	  TREE_PURPOSE (parm) = dflt;
> -
> -	  tree decl = TREE_VALUE (parm);
> -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> -	    {
> -	      tree inner = DECL_TEMPLATE_RESULT (decl);
> -	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> -			  ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> -			  : DECL_INITIAL (inner));
> -	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> -			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -	      /* Original template template parms have a context
> -		 of their owning template.  Reduced ones do not.  */
> -	      if (original)
> -		DECL_CONTEXT (decl) = tmpl;
> -	    }
>  	}
>      }
>    return true;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> new file mode 100644
> index 00000000000..2fc088f0f39
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> @@ -0,0 +1,12 @@
> +// PR c++/98881
> +
> +template <typename P> struct X {};
> +
> +template<template <typename> typename TT>
> +struct X<TT<int>> {
> +  template<template <typename> typename UU>
> +  void f (X<UU<int>>&);
> +};
> +
> +template<template<class> class TT> struct Y;      // #1
> +template<template<class> class UU> struct Y { };  // #2
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> new file mode 100644
> index 00000000000..47464081f25
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> @@ -0,0 +1,5 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +#include "tpl-tpl-parm-3.h"
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> new file mode 100644
> index 00000000000..1e3cc6c444f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> @@ -0,0 +1,5 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> +
> +#include "tpl-tpl-parm-3.h"
> +import "tpl-tpl-parm-3_a.H";
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> new file mode 100644
> index 00000000000..5edba6cc64f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> @@ -0,0 +1,15 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "tpl-tpl-parm-3_a.H";
> +
> +template <typename T> struct A {};
> +template <typename T> struct B {};
> +
> +void foo() {
> +  X<A<int>> a;
> +  X<B<int>> b;
> +  a.f(b);
> +
> +  Y<A> y;
> +}
> -- 
> 2.43.2
> 
>

On Mon, Mar 04, 2024 at 09:26:00PM -0500, Patrick Palka wrote:
> On Tue, 5 Mar 2024, Nathaniel Shead wrote:
> 
> > On Mon, Mar 04, 2024 at 07:14:54PM -0500, Patrick Palka wrote:
> > > On Sat, 2 Mar 2024, Nathaniel Shead wrote:
> > > 
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > > 
> > > > -- >8 --
> > > > 
> > > > When streaming in a nested template-template parameter as in the
> > > > attached testcase, we end up reaching the containing template-template
> > > > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > > > this (nested) template-template parameter, as it should already be the
> > > > struct that the outer template-template parameter is declared on.
> > > > 
> > > > 	PR c++/98881
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> > > > 	for checking purposes. Don't consider a template template
> > > > 	parameter as the owning template.
> > > > 	(trees_in::tpl_parms_fini): Don't consider a template template
> > > > 	parameter as the owning template.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > > > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > > > 
> > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > > ---
> > > >  gcc/cp/module.cc                                | 17 ++++++++++++-----
> > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
> > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
> > > >  3 files changed, 36 insertions(+), 5 deletions(-)
> > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > > > 
> > > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > > index 67f132d28d7..5663d01ed9c 100644
> > > > --- a/gcc/cp/module.cc
> > > > +++ b/gcc/cp/module.cc
> > > > @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > >  	  tree dflt = TREE_PURPOSE (parm);
> > > >  	  tree_node (dflt);
> > > >  
> > > > -	  if (streaming_p ())
> > > > +	  if (CHECKING_P && streaming_p ())
> > > >  	    {
> > > > +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> > > > +		 streaming in is correct.  */
> > > >  	      tree decl = TREE_VALUE (parm);
> > > > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > +		  /* A template template parm is not the owning template.  */
> > > > +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > >  		{
> > > >  		  tree ctx = DECL_CONTEXT (decl);
> > > >  		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > > > @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > >  	    return false;
> > > >  	  TREE_PURPOSE (parm) = dflt;
> > > >  
> > > > +	  /* Original template template parms have a context
> > > > +	     of their owning template.  Reduced ones do not.
> > > > +	     But if TMPL is itself a template template parm
> > > > +	     then it cannot be the owning template.  */
> > > >  	  tree decl = TREE_VALUE (parm);
> > > > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > 
> > > IIUC a TEMPLATE_DECL inside a template parameter list always represents
> > > a template template parm, so won't this effectively disable the
> > > DECL_CONTEXT setting logic?
> > 
> > This is only when 'tmpl' (i.e. the containing TEMPLATE_DECL that we're
> > streaming) is itself a template template parm.
> 
> D'oh, makes sense.
> 
> > 
> > > >  	    {
> > > >  	      tree inner = DECL_TEMPLATE_RESULT (decl);
> > > >  	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > > > @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > >  			  : DECL_INITIAL (inner));
> > > >  	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > > >  			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > > > -	      /* Original template template parms have a context
> > > > -		 of their owning template.  Reduced ones do not.  */
> > > >  	      if (original)
> > > >  		DECL_CONTEXT (decl) = tmpl;
> > > >  	    }
> > > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > new file mode 100644
> > > > index 00000000000..21bbc054fa3
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > @@ -0,0 +1,11 @@
> > > > +// PR c++/98881
> > > > +// { dg-additional-options "-fmodule-header" }
> > > > +// { dg-module-cmi {} }
> > > > +
> > > > +template <typename P> struct X {};
> > > > +
> > > > +template<template <typename> typename TT>
> > > > +struct X<TT<int>> {
> > > > +  template<template <typename> typename UU>
> > > > +  void f (X<UU<int>>&);
> > > > +};
> > > 
> > > I wonder why the partial specialization is relevant here?  I can't
> > > seem to trigger the ICE without the partial specialization.
> > > Slightly further reduced to not use bound ttps:
> > > 
> > >     template<template<class> class TT>
> > >     struct X { };
> > > 
> > >     template<template<class> class TT> requires true
> > >     struct X<TT> {
> > >       template<template<class> class UU>
> > >       void f(X<UU>);
> > >     };
> > > 
> > > Maybe the expectation is that tpl_parms_fini for UU should be called
> > > with tpl_levels=1 (so that we stream only its own template parameters)
> > > but it's instead called with tpl_levels=3 for some reason?
> > > 
> > > IIUC that assert should always hold in the first iteration of the loop
> > > (for the ttp's own template parameters).  Perhaps for subsequent
> > > iterations we need to actually stream the contexts?
> > > 
> > > Ah, but we also ICE in the same spot with:
> > > 
> > >   template<template<class> class TT> struct X;      // #1
> > >   template<template<class> class UU> struct X { };  // #2
> > > 
> > > because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
> > > bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
> > > of each ttp as-is instead of trying to avoid it via an apparently
> > > fragile assumption?
> > > 
> > 
> > That seems reasonable to me. Probably better to aim for correctness at
> > this stage rather than relying on things I don't understand for slight
> > performance gains.
> > 
> > I started putting together a patch to always restream the DECL_CONTEXT,
> > but as I was doing so I realised that actually, we stream the context
> > anyway, and this value should be correct to start with? I don't think
> > deduplication should affect this, but I've added to the test to make
> > sure that there's no issues here that I can see.
> 
> Where do we otherwise stream the context?  In trees_out::core_vals we
> seem to avoid streaming the context for template parms (including ttps):
> 
>     if (!DECL_TEMPLATE_PARM_P (t))
>       WT (t->decl_minimal.context);

Ah whoops, I was looking at the wrong 'context' (type_common.context). I
guess I need to find a testcase that actually relies on DECL_CONTEXT
being set correctly for template-template parameters.

> > 
> > So maybe something like this? Or might there be something I'm missing?
> > 
> > Bootstrapped and regtested (only modules.exp so far) on
> > x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
> > 
> > -- >8 --
> > 
> > Subject: [PATCH] c++: Remove special handling of DECL_CONTEXT for template template parms [PR98881]
> > 
> > When streaming in a nested template-template parameter as in the
> > attached testcase, we end up reaching the containing template-template
> > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > this (nested) template-template parameter, as it should already be the
> > struct that the outer template-template parameter is declared on.
> > 
> > The precise logic for what DECL_CONTEXT should be for a template
> > template parameter in various situations seems rather obscure. However,
> > it turns out in this case that the correct solution is to just do less
> > work: normal streaming already streams the correct value of DECL_CONTEXT
> > anyway, so there should be no need to redetermine them here.
> > 
> > 	PR c++/98881
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* module.cc (trees_out::tpl_parms_fini): Remove special handling
> > 	for DECL_CONTEXT of template template parameters.
> > 	(trees_in::tpl_parms_fini): Likewise.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/modules/tpl-tpl-parm-3.h: New test.
> > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > 	* g++.dg/modules/tpl-tpl-parm-3_c.C: New test.
> > 
> > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > Reviewed-by: Patrick Palka <ppalka@redhat.com>
> > Reviewed-by: Jason Merrill <jason@redhat.com> 
> > ---
> >  gcc/cp/module.cc                              | 33 -------------------
> >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h | 12 +++++++
> >  .../g++.dg/modules/tpl-tpl-parm-3_a.H         |  5 +++
> >  .../g++.dg/modules/tpl-tpl-parm-3_b.C         |  5 +++
> >  .../g++.dg/modules/tpl-tpl-parm-3_c.C         | 15 +++++++++
> >  5 files changed, 37 insertions(+), 33 deletions(-)
> >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> > 
> > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > index 67f132d28d7..dbb851baa40 100644
> > --- a/gcc/cp/module.cc
> > +++ b/gcc/cp/module.cc
> > @@ -10125,24 +10125,6 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >  	  tree parm = TREE_VEC_ELT (vec, ix);
> >  	  tree dflt = TREE_PURPOSE (parm);
> >  	  tree_node (dflt);
> > -
> > -	  if (streaming_p ())
> > -	    {
> > -	      tree decl = TREE_VALUE (parm);
> > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > -		{
> > -		  tree ctx = DECL_CONTEXT (decl);
> > -		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > -		  tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > -			      ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> > -			      : DECL_INITIAL (inner));
> > -		  bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > -				   == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > -		  /* Original template template parms have a context
> > -		     of their owning template.  Reduced ones do not.  */
> > -		  gcc_checking_assert (original ? ctx == tmpl : !ctx);
> > -		}
> > -	    }
> >  	}
> >      }
> >  }
> > @@ -10163,21 +10145,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> >  	  if (get_overrun ())
> >  	    return false;
> >  	  TREE_PURPOSE (parm) = dflt;
> > -
> > -	  tree decl = TREE_VALUE (parm);
> > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > -	    {
> > -	      tree inner = DECL_TEMPLATE_RESULT (decl);
> > -	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > -			  ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> > -			  : DECL_INITIAL (inner));
> > -	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > -			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > -	      /* Original template template parms have a context
> > -		 of their owning template.  Reduced ones do not.  */
> > -	      if (original)
> > -		DECL_CONTEXT (decl) = tmpl;
> > -	    }
> >  	}
> >      }
> >    return true;
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> > new file mode 100644
> > index 00000000000..2fc088f0f39
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> > @@ -0,0 +1,12 @@
> > +// PR c++/98881
> > +
> > +template <typename P> struct X {};
> > +
> > +template<template <typename> typename TT>
> > +struct X<TT<int>> {
> > +  template<template <typename> typename UU>
> > +  void f (X<UU<int>>&);
> > +};
> > +
> > +template<template<class> class TT> struct Y;      // #1
> > +template<template<class> class UU> struct Y { };  // #2
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > new file mode 100644
> > index 00000000000..47464081f25
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > @@ -0,0 +1,5 @@
> > +// PR c++/98881
> > +// { dg-additional-options "-fmodule-header" }
> > +// { dg-module-cmi {} }
> > +
> > +#include "tpl-tpl-parm-3.h"
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > new file mode 100644
> > index 00000000000..1e3cc6c444f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > @@ -0,0 +1,5 @@
> > +// PR c++/98881
> > +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> > +
> > +#include "tpl-tpl-parm-3.h"
> > +import "tpl-tpl-parm-3_a.H";
> > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> > new file mode 100644
> > index 00000000000..5edba6cc64f
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> > @@ -0,0 +1,15 @@
> > +// PR c++/98881
> > +// { dg-additional-options "-fmodules-ts" }
> > +
> > +import "tpl-tpl-parm-3_a.H";
> > +
> > +template <typename T> struct A {};
> > +template <typename T> struct B {};
> > +
> > +void foo() {
> > +  X<A<int>> a;
> > +  X<B<int>> b;
> > +  a.f(b);
> > +
> > +  Y<A> y;
> > +}
> > -- 
> > 2.43.2
> > 
> > 
>

On Tue, 5 Mar 2024, Nathaniel Shead wrote:

> On Mon, Mar 04, 2024 at 09:26:00PM -0500, Patrick Palka wrote:
> > On Tue, 5 Mar 2024, Nathaniel Shead wrote:
> > 
> > > On Mon, Mar 04, 2024 at 07:14:54PM -0500, Patrick Palka wrote:
> > > > On Sat, 2 Mar 2024, Nathaniel Shead wrote:
> > > > 
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > > > 
> > > > > -- >8 --
> > > > > 
> > > > > When streaming in a nested template-template parameter as in the
> > > > > attached testcase, we end up reaching the containing template-template
> > > > > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > > > > this (nested) template-template parameter, as it should already be the
> > > > > struct that the outer template-template parameter is declared on.
> > > > > 
> > > > > 	PR c++/98881
> > > > > 
> > > > > gcc/cp/ChangeLog:
> > > > > 
> > > > > 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> > > > > 	for checking purposes. Don't consider a template template
> > > > > 	parameter as the owning template.
> > > > > 	(trees_in::tpl_parms_fini): Don't consider a template template
> > > > > 	parameter as the owning template.
> > > > > 
> > > > > gcc/testsuite/ChangeLog:
> > > > > 
> > > > > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > > > > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > > > > 
> > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > > > ---
> > > > >  gcc/cp/module.cc                                | 17 ++++++++++++-----
> > > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
> > > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
> > > > >  3 files changed, 36 insertions(+), 5 deletions(-)
> > > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > > > > 
> > > > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > > > index 67f132d28d7..5663d01ed9c 100644
> > > > > --- a/gcc/cp/module.cc
> > > > > +++ b/gcc/cp/module.cc
> > > > > @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > >  	  tree dflt = TREE_PURPOSE (parm);
> > > > >  	  tree_node (dflt);
> > > > >  
> > > > > -	  if (streaming_p ())
> > > > > +	  if (CHECKING_P && streaming_p ())
> > > > >  	    {
> > > > > +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> > > > > +		 streaming in is correct.  */
> > > > >  	      tree decl = TREE_VALUE (parm);
> > > > > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > > +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > > +		  /* A template template parm is not the owning template.  */
> > > > > +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > > >  		{
> > > > >  		  tree ctx = DECL_CONTEXT (decl);
> > > > >  		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > > > > @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > >  	    return false;
> > > > >  	  TREE_PURPOSE (parm) = dflt;
> > > > >  
> > > > > +	  /* Original template template parms have a context
> > > > > +	     of their owning template.  Reduced ones do not.
> > > > > +	     But if TMPL is itself a template template parm
> > > > > +	     then it cannot be the owning template.  */
> > > > >  	  tree decl = TREE_VALUE (parm);
> > > > > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > > +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > > +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > > 
> > > > IIUC a TEMPLATE_DECL inside a template parameter list always represents
> > > > a template template parm, so won't this effectively disable the
> > > > DECL_CONTEXT setting logic?
> > > 
> > > This is only when 'tmpl' (i.e. the containing TEMPLATE_DECL that we're
> > > streaming) is itself a template template parm.
> > 
> > D'oh, makes sense.
> > 
> > > 
> > > > >  	    {
> > > > >  	      tree inner = DECL_TEMPLATE_RESULT (decl);
> > > > >  	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > > > > @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > >  			  : DECL_INITIAL (inner));
> > > > >  	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > > > >  			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > > > > -	      /* Original template template parms have a context
> > > > > -		 of their owning template.  Reduced ones do not.  */
> > > > >  	      if (original)
> > > > >  		DECL_CONTEXT (decl) = tmpl;
> > > > >  	    }
> > > > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > new file mode 100644
> > > > > index 00000000000..21bbc054fa3
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > @@ -0,0 +1,11 @@
> > > > > +// PR c++/98881
> > > > > +// { dg-additional-options "-fmodule-header" }
> > > > > +// { dg-module-cmi {} }
> > > > > +
> > > > > +template <typename P> struct X {};
> > > > > +
> > > > > +template<template <typename> typename TT>
> > > > > +struct X<TT<int>> {
> > > > > +  template<template <typename> typename UU>
> > > > > +  void f (X<UU<int>>&);
> > > > > +};
> > > > 
> > > > I wonder why the partial specialization is relevant here?  I can't
> > > > seem to trigger the ICE without the partial specialization.
> > > > Slightly further reduced to not use bound ttps:
> > > > 
> > > >     template<template<class> class TT>
> > > >     struct X { };
> > > > 
> > > >     template<template<class> class TT> requires true
> > > >     struct X<TT> {
> > > >       template<template<class> class UU>
> > > >       void f(X<UU>);
> > > >     };
> > > > 
> > > > Maybe the expectation is that tpl_parms_fini for UU should be called
> > > > with tpl_levels=1 (so that we stream only its own template parameters)
> > > > but it's instead called with tpl_levels=3 for some reason?
> > > > 
> > > > IIUC that assert should always hold in the first iteration of the loop
> > > > (for the ttp's own template parameters).  Perhaps for subsequent
> > > > iterations we need to actually stream the contexts?
> > > > 
> > > > Ah, but we also ICE in the same spot with:
> > > > 
> > > >   template<template<class> class TT> struct X;      // #1
> > > >   template<template<class> class UU> struct X { };  // #2
> > > > 
> > > > because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
> > > > bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
> > > > of each ttp as-is instead of trying to avoid it via an apparently
> > > > fragile assumption?
> > > > 
> > > 
> > > That seems reasonable to me. Probably better to aim for correctness at
> > > this stage rather than relying on things I don't understand for slight
> > > performance gains.
> > > 
> > > I started putting together a patch to always restream the DECL_CONTEXT,
> > > but as I was doing so I realised that actually, we stream the context
> > > anyway, and this value should be correct to start with? I don't think
> > > deduplication should affect this, but I've added to the test to make
> > > sure that there's no issues here that I can see.
> > 
> > Where do we otherwise stream the context?  In trees_out::core_vals we
> > seem to avoid streaming the context for template parms (including ttps):
> > 
> >     if (!DECL_TEMPLATE_PARM_P (t))
> >       WT (t->decl_minimal.context);
> 
> Ah whoops, I was looking at the wrong 'context' (type_common.context). I
> guess I need to find a testcase that actually relies on DECL_CONTEXT
> being set correctly for template-template parameters.

It might be hard to come up with such a testcase since DECL_CONTEXT
has been unreliably set for ttps probably since inception (for example
for level-lowered ttps), so most logic is already robust to context-less
ttps.  But I suppose we might as well stream it anyway.

> 
> > > 
> > > So maybe something like this? Or might there be something I'm missing?
> > > 
> > > Bootstrapped and regtested (only modules.exp so far) on
> > > x86_64-pc-linux-gnu, OK for trunk if full regtest succeeds?
> > > 
> > > -- >8 --
> > > 
> > > Subject: [PATCH] c++: Remove special handling of DECL_CONTEXT for template template parms [PR98881]
> > > 
> > > When streaming in a nested template-template parameter as in the
> > > attached testcase, we end up reaching the containing template-template
> > > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > > this (nested) template-template parameter, as it should already be the
> > > struct that the outer template-template parameter is declared on.
> > > 
> > > The precise logic for what DECL_CONTEXT should be for a template
> > > template parameter in various situations seems rather obscure. However,
> > > it turns out in this case that the correct solution is to just do less
> > > work: normal streaming already streams the correct value of DECL_CONTEXT
> > > anyway, so there should be no need to redetermine them here.
> > > 
> > > 	PR c++/98881
> > > 
> > > gcc/cp/ChangeLog:
> > > 
> > > 	* module.cc (trees_out::tpl_parms_fini): Remove special handling
> > > 	for DECL_CONTEXT of template template parameters.
> > > 	(trees_in::tpl_parms_fini): Likewise.
> > > 
> > > gcc/testsuite/ChangeLog:
> > > 
> > > 	* g++.dg/modules/tpl-tpl-parm-3.h: New test.
> > > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > > 	* g++.dg/modules/tpl-tpl-parm-3_c.C: New test.
> > > 
> > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > Reviewed-by: Patrick Palka <ppalka@redhat.com>
> > > Reviewed-by: Jason Merrill <jason@redhat.com> 
> > > ---
> > >  gcc/cp/module.cc                              | 33 -------------------
> > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h | 12 +++++++
> > >  .../g++.dg/modules/tpl-tpl-parm-3_a.H         |  5 +++
> > >  .../g++.dg/modules/tpl-tpl-parm-3_b.C         |  5 +++
> > >  .../g++.dg/modules/tpl-tpl-parm-3_c.C         | 15 +++++++++
> > >  5 files changed, 37 insertions(+), 33 deletions(-)
> > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> > > 
> > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > index 67f132d28d7..dbb851baa40 100644
> > > --- a/gcc/cp/module.cc
> > > +++ b/gcc/cp/module.cc
> > > @@ -10125,24 +10125,6 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > >  	  tree parm = TREE_VEC_ELT (vec, ix);
> > >  	  tree dflt = TREE_PURPOSE (parm);
> > >  	  tree_node (dflt);
> > > -
> > > -	  if (streaming_p ())
> > > -	    {
> > > -	      tree decl = TREE_VALUE (parm);
> > > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > -		{
> > > -		  tree ctx = DECL_CONTEXT (decl);
> > > -		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > > -		  tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > > -			      ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> > > -			      : DECL_INITIAL (inner));
> > > -		  bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > > -				   == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > > -		  /* Original template template parms have a context
> > > -		     of their owning template.  Reduced ones do not.  */
> > > -		  gcc_checking_assert (original ? ctx == tmpl : !ctx);
> > > -		}
> > > -	    }
> > >  	}
> > >      }
> > >  }
> > > @@ -10163,21 +10145,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > >  	  if (get_overrun ())
> > >  	    return false;
> > >  	  TREE_PURPOSE (parm) = dflt;
> > > -
> > > -	  tree decl = TREE_VALUE (parm);
> > > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > -	    {
> > > -	      tree inner = DECL_TEMPLATE_RESULT (decl);
> > > -	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > > -			  ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> > > -			  : DECL_INITIAL (inner));
> > > -	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > > -			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > > -	      /* Original template template parms have a context
> > > -		 of their owning template.  Reduced ones do not.  */
> > > -	      if (original)
> > > -		DECL_CONTEXT (decl) = tmpl;
> > > -	    }
> > >  	}
> > >      }
> > >    return true;
> > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> > > new file mode 100644
> > > index 00000000000..2fc088f0f39
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> > > @@ -0,0 +1,12 @@
> > > +// PR c++/98881
> > > +
> > > +template <typename P> struct X {};
> > > +
> > > +template<template <typename> typename TT>
> > > +struct X<TT<int>> {
> > > +  template<template <typename> typename UU>
> > > +  void f (X<UU<int>>&);
> > > +};
> > > +
> > > +template<template<class> class TT> struct Y;      // #1
> > > +template<template<class> class UU> struct Y { };  // #2
> > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > new file mode 100644
> > > index 00000000000..47464081f25
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > @@ -0,0 +1,5 @@
> > > +// PR c++/98881
> > > +// { dg-additional-options "-fmodule-header" }
> > > +// { dg-module-cmi {} }
> > > +
> > > +#include "tpl-tpl-parm-3.h"
> > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > > new file mode 100644
> > > index 00000000000..1e3cc6c444f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > > @@ -0,0 +1,5 @@
> > > +// PR c++/98881
> > > +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> > > +
> > > +#include "tpl-tpl-parm-3.h"
> > > +import "tpl-tpl-parm-3_a.H";
> > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> > > new file mode 100644
> > > index 00000000000..5edba6cc64f
> > > --- /dev/null
> > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> > > @@ -0,0 +1,15 @@
> > > +// PR c++/98881
> > > +// { dg-additional-options "-fmodules-ts" }
> > > +
> > > +import "tpl-tpl-parm-3_a.H";
> > > +
> > > +template <typename T> struct A {};
> > > +template <typename T> struct B {};
> > > +
> > > +void foo() {
> > > +  X<A<int>> a;
> > > +  X<B<int>> b;
> > > +  a.f(b);
> > > +
> > > +  Y<A> y;
> > > +}
> > > -- 
> > > 2.43.2
> > > 
> > > 
> > 
> 
>

On Mon, Mar 04, 2024 at 10:07:33PM -0500, Patrick Palka wrote:
> On Tue, 5 Mar 2024, Nathaniel Shead wrote:
> 
> > On Mon, Mar 04, 2024 at 09:26:00PM -0500, Patrick Palka wrote:
> > > On Tue, 5 Mar 2024, Nathaniel Shead wrote:
> > > 
> > > > On Mon, Mar 04, 2024 at 07:14:54PM -0500, Patrick Palka wrote:
> > > > > On Sat, 2 Mar 2024, Nathaniel Shead wrote:
> > > > > 
> > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > > > > 
> > > > > > -- >8 --
> > > > > > 
> > > > > > When streaming in a nested template-template parameter as in the
> > > > > > attached testcase, we end up reaching the containing template-template
> > > > > > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > > > > > this (nested) template-template parameter, as it should already be the
> > > > > > struct that the outer template-template parameter is declared on.
> > > > > > 
> > > > > > 	PR c++/98881
> > > > > > 
> > > > > > gcc/cp/ChangeLog:
> > > > > > 
> > > > > > 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> > > > > > 	for checking purposes. Don't consider a template template
> > > > > > 	parameter as the owning template.
> > > > > > 	(trees_in::tpl_parms_fini): Don't consider a template template
> > > > > > 	parameter as the owning template.
> > > > > > 
> > > > > > gcc/testsuite/ChangeLog:
> > > > > > 
> > > > > > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > > > > > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > > > > > 
> > > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > > > > ---
> > > > > >  gcc/cp/module.cc                                | 17 ++++++++++++-----
> > > > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
> > > > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
> > > > > >  3 files changed, 36 insertions(+), 5 deletions(-)
> > > > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > > > > > 
> > > > > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > > > > index 67f132d28d7..5663d01ed9c 100644
> > > > > > --- a/gcc/cp/module.cc
> > > > > > +++ b/gcc/cp/module.cc
> > > > > > @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > > >  	  tree dflt = TREE_PURPOSE (parm);
> > > > > >  	  tree_node (dflt);
> > > > > >  
> > > > > > -	  if (streaming_p ())
> > > > > > +	  if (CHECKING_P && streaming_p ())
> > > > > >  	    {
> > > > > > +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> > > > > > +		 streaming in is correct.  */
> > > > > >  	      tree decl = TREE_VALUE (parm);
> > > > > > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > > > +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > > > +		  /* A template template parm is not the owning template.  */
> > > > > > +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > > > >  		{
> > > > > >  		  tree ctx = DECL_CONTEXT (decl);
> > > > > >  		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > > > > > @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > > >  	    return false;
> > > > > >  	  TREE_PURPOSE (parm) = dflt;
> > > > > >  
> > > > > > +	  /* Original template template parms have a context
> > > > > > +	     of their owning template.  Reduced ones do not.
> > > > > > +	     But if TMPL is itself a template template parm
> > > > > > +	     then it cannot be the owning template.  */
> > > > > >  	  tree decl = TREE_VALUE (parm);
> > > > > > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > > > +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > > > +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > > > 
> > > > > IIUC a TEMPLATE_DECL inside a template parameter list always represents
> > > > > a template template parm, so won't this effectively disable the
> > > > > DECL_CONTEXT setting logic?
> > > > 
> > > > This is only when 'tmpl' (i.e. the containing TEMPLATE_DECL that we're
> > > > streaming) is itself a template template parm.
> > > 
> > > D'oh, makes sense.
> > > 
> > > > 
> > > > > >  	    {
> > > > > >  	      tree inner = DECL_TEMPLATE_RESULT (decl);
> > > > > >  	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > > > > > @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > > >  			  : DECL_INITIAL (inner));
> > > > > >  	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > > > > >  			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > > > > > -	      /* Original template template parms have a context
> > > > > > -		 of their owning template.  Reduced ones do not.  */
> > > > > >  	      if (original)
> > > > > >  		DECL_CONTEXT (decl) = tmpl;
> > > > > >  	    }
> > > > > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > > new file mode 100644
> > > > > > index 00000000000..21bbc054fa3
> > > > > > --- /dev/null
> > > > > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > > @@ -0,0 +1,11 @@
> > > > > > +// PR c++/98881
> > > > > > +// { dg-additional-options "-fmodule-header" }
> > > > > > +// { dg-module-cmi {} }
> > > > > > +
> > > > > > +template <typename P> struct X {};
> > > > > > +
> > > > > > +template<template <typename> typename TT>
> > > > > > +struct X<TT<int>> {
> > > > > > +  template<template <typename> typename UU>
> > > > > > +  void f (X<UU<int>>&);
> > > > > > +};
> > > > > 
> > > > > I wonder why the partial specialization is relevant here?  I can't
> > > > > seem to trigger the ICE without the partial specialization.
> > > > > Slightly further reduced to not use bound ttps:
> > > > > 
> > > > >     template<template<class> class TT>
> > > > >     struct X { };
> > > > > 
> > > > >     template<template<class> class TT> requires true
> > > > >     struct X<TT> {
> > > > >       template<template<class> class UU>
> > > > >       void f(X<UU>);
> > > > >     };
> > > > > 
> > > > > Maybe the expectation is that tpl_parms_fini for UU should be called
> > > > > with tpl_levels=1 (so that we stream only its own template parameters)
> > > > > but it's instead called with tpl_levels=3 for some reason?
> > > > > 
> > > > > IIUC that assert should always hold in the first iteration of the loop
> > > > > (for the ttp's own template parameters).  Perhaps for subsequent
> > > > > iterations we need to actually stream the contexts?
> > > > > 
> > > > > Ah, but we also ICE in the same spot with:
> > > > > 
> > > > >   template<template<class> class TT> struct X;      // #1
> > > > >   template<template<class> class UU> struct X { };  // #2
> > > > > 
> > > > > because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
> > > > > bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
> > > > > of each ttp as-is instead of trying to avoid it via an apparently
> > > > > fragile assumption?
> > > > > 
> > > > 
> > > > That seems reasonable to me. Probably better to aim for correctness at
> > > > this stage rather than relying on things I don't understand for slight
> > > > performance gains.
> > > > 
> > > > I started putting together a patch to always restream the DECL_CONTEXT,
> > > > but as I was doing so I realised that actually, we stream the context
> > > > anyway, and this value should be correct to start with? I don't think
> > > > deduplication should affect this, but I've added to the test to make
> > > > sure that there's no issues here that I can see.
> > > 
> > > Where do we otherwise stream the context?  In trees_out::core_vals we
> > > seem to avoid streaming the context for template parms (including ttps):
> > > 
> > >     if (!DECL_TEMPLATE_PARM_P (t))
> > >       WT (t->decl_minimal.context);
> > 
> > Ah whoops, I was looking at the wrong 'context' (type_common.context). I
> > guess I need to find a testcase that actually relies on DECL_CONTEXT
> > being set correctly for template-template parameters.
> 
> It might be hard to come up with such a testcase since DECL_CONTEXT
> has been unreliably set for ttps probably since inception (for example
> for level-lowered ttps), so most logic is already robust to context-less
> ttps.  But I suppose we might as well stream it anyway.

Fair enough. I've tried for a while but I haven't been able to work
anything out; maybe another time.

Otherwise here's an updated version of the patch. Bootstrapped and
regtested (so far just modules.exp) on x86_64-pc-linux-gnu: OK for
trunk if full regtest succeeds?

-- >8 --

Subject: [PATCH] c++: Stream DECL_CONTEXT for template template parms [PR98881]

When streaming in a nested template-template parameter as in the
attached testcase, we end up reaching the containing template-template
parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
this (nested) template-template parameter, as it should already be the
struct that the outer template-template parameter is declared on.

The precise logic for what DECL_CONTEXT should be for a template
template parameter in various situations seems rather obscure. Rather
than trying to determine the assumptions that need to hold, it seems
simpler to just always re-stream the DECL_CONTEXT as needed for now.

	PR c++/98881

gcc/cp/ChangeLog:

	* module.cc (trees_out::tpl_parms_fini): Stream out DECL_CONTEXT
	for template template parameters.
	(trees_in::tpl_parms_fini): Read it.

gcc/testsuite/ChangeLog:

	* g++.dg/modules/tpl-tpl-parm-3.h: New test.
	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
	* g++.dg/modules/tpl-tpl-parm-3_c.C: New test.

Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
Reviewed-by: Patrick Palka <ppalka@redhat.com>
Reviewed-by: Jason Merrill <jason@redhat.com>
---
 gcc/cp/module.cc                              | 41 +++++--------------
 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h | 12 ++++++
 .../g++.dg/modules/tpl-tpl-parm-3_a.H         |  5 +++
 .../g++.dg/modules/tpl-tpl-parm-3_b.C         |  5 +++
 .../g++.dg/modules/tpl-tpl-parm-3_c.C         | 15 +++++++
 5 files changed, 47 insertions(+), 31 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
 create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C

diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 67f132d28d7..fcb449563ab 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -10126,23 +10126,12 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
 	  tree dflt = TREE_PURPOSE (parm);
 	  tree_node (dflt);
 
-	  if (streaming_p ())
-	    {
-	      tree decl = TREE_VALUE (parm);
-	      if (TREE_CODE (decl) == TEMPLATE_DECL)
-		{
-		  tree ctx = DECL_CONTEXT (decl);
-		  tree inner = DECL_TEMPLATE_RESULT (decl);
-		  tree tpi = (TREE_CODE (inner) == TYPE_DECL
-			      ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
-			      : DECL_INITIAL (inner));
-		  bool original = (TEMPLATE_PARM_LEVEL (tpi)
-				   == TEMPLATE_PARM_ORIG_LEVEL (tpi));
-		  /* Original template template parms have a context
-		     of their owning template.  Reduced ones do not.  */
-		  gcc_checking_assert (original ? ctx == tmpl : !ctx);
-		}
-	    }
+	  /* Template template parameters need a context of their owning
+	     template. This is quite tricky to infer correctly on stream-in
+	     (see PR c++/98881) so we'll just provide it directly.  */
+	  tree decl = TREE_VALUE (parm);
+	  if (TREE_CODE (decl) == TEMPLATE_DECL)
+	    tree_node (DECL_CONTEXT (decl));
 	}
     }
 }
@@ -10160,24 +10149,14 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
 	{
 	  tree parm = TREE_VEC_ELT (vec, ix);
 	  tree dflt = tree_node ();
-	  if (get_overrun ())
-	    return false;
 	  TREE_PURPOSE (parm) = dflt;
 
 	  tree decl = TREE_VALUE (parm);
 	  if (TREE_CODE (decl) == TEMPLATE_DECL)
-	    {
-	      tree inner = DECL_TEMPLATE_RESULT (decl);
-	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
-			  ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
-			  : DECL_INITIAL (inner));
-	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
-			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
-	      /* Original template template parms have a context
-		 of their owning template.  Reduced ones do not.  */
-	      if (original)
-		DECL_CONTEXT (decl) = tmpl;
-	    }
+	    DECL_CONTEXT (decl) = tree_node ();
+
+	  if (get_overrun ())
+	    return false;
 	}
     }
   return true;
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
new file mode 100644
index 00000000000..b0dcf353c23
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
@@ -0,0 +1,12 @@
+// PR c++/98881
+
+template <typename P> struct X {};
+
+template<template <typename> typename TT>
+struct X<TT<int>> {
+  template<template <typename> typename UU>
+  void f (X<UU<int>>&);
+};
+
+template<template<class> class TT> struct Y;
+template<template<class> class UU> struct Y { };
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
new file mode 100644
index 00000000000..47464081f25
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
@@ -0,0 +1,5 @@
+// PR c++/98881
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+#include "tpl-tpl-parm-3.h"
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
new file mode 100644
index 00000000000..1e3cc6c444f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
@@ -0,0 +1,5 @@
+// PR c++/98881
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+
+#include "tpl-tpl-parm-3.h"
+import "tpl-tpl-parm-3_a.H";
diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
new file mode 100644
index 00000000000..5edba6cc64f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
@@ -0,0 +1,15 @@
+// PR c++/98881
+// { dg-additional-options "-fmodules-ts" }
+
+import "tpl-tpl-parm-3_a.H";
+
+template <typename T> struct A {};
+template <typename T> struct B {};
+
+void foo() {
+  X<A<int>> a;
+  X<B<int>> b;
+  a.f(b);
+
+  Y<A> y;
+}

On Tue, 5 Mar 2024, Nathaniel Shead wrote:

> On Mon, Mar 04, 2024 at 10:07:33PM -0500, Patrick Palka wrote:
> > On Tue, 5 Mar 2024, Nathaniel Shead wrote:
> > 
> > > On Mon, Mar 04, 2024 at 09:26:00PM -0500, Patrick Palka wrote:
> > > > On Tue, 5 Mar 2024, Nathaniel Shead wrote:
> > > > 
> > > > > On Mon, Mar 04, 2024 at 07:14:54PM -0500, Patrick Palka wrote:
> > > > > > On Sat, 2 Mar 2024, Nathaniel Shead wrote:
> > > > > > 
> > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
> > > > > > > 
> > > > > > > -- >8 --
> > > > > > > 
> > > > > > > When streaming in a nested template-template parameter as in the
> > > > > > > attached testcase, we end up reaching the containing template-template
> > > > > > > parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> > > > > > > this (nested) template-template parameter, as it should already be the
> > > > > > > struct that the outer template-template parameter is declared on.
> > > > > > > 
> > > > > > > 	PR c++/98881
> > > > > > > 
> > > > > > > gcc/cp/ChangeLog:
> > > > > > > 
> > > > > > > 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
> > > > > > > 	for checking purposes. Don't consider a template template
> > > > > > > 	parameter as the owning template.
> > > > > > > 	(trees_in::tpl_parms_fini): Don't consider a template template
> > > > > > > 	parameter as the owning template.
> > > > > > > 
> > > > > > > gcc/testsuite/ChangeLog:
> > > > > > > 
> > > > > > > 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> > > > > > > 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> > > > > > > 
> > > > > > > Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> > > > > > > ---
> > > > > > >  gcc/cp/module.cc                                | 17 ++++++++++++-----
> > > > > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
> > > > > > >  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
> > > > > > >  3 files changed, 36 insertions(+), 5 deletions(-)
> > > > > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > > >  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> > > > > > > 
> > > > > > > diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> > > > > > > index 67f132d28d7..5663d01ed9c 100644
> > > > > > > --- a/gcc/cp/module.cc
> > > > > > > +++ b/gcc/cp/module.cc
> > > > > > > @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > > > >  	  tree dflt = TREE_PURPOSE (parm);
> > > > > > >  	  tree_node (dflt);
> > > > > > >  
> > > > > > > -	  if (streaming_p ())
> > > > > > > +	  if (CHECKING_P && streaming_p ())
> > > > > > >  	    {
> > > > > > > +	      /* Sanity check that the DECL_CONTEXT we'll infer when
> > > > > > > +		 streaming in is correct.  */
> > > > > > >  	      tree decl = TREE_VALUE (parm);
> > > > > > > -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > > > > +	      if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > > > > +		  /* A template template parm is not the owning template.  */
> > > > > > > +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > > > > >  		{
> > > > > > >  		  tree ctx = DECL_CONTEXT (decl);
> > > > > > >  		  tree inner = DECL_TEMPLATE_RESULT (decl);
> > > > > > > @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > > > >  	    return false;
> > > > > > >  	  TREE_PURPOSE (parm) = dflt;
> > > > > > >  
> > > > > > > +	  /* Original template template parms have a context
> > > > > > > +	     of their owning template.  Reduced ones do not.
> > > > > > > +	     But if TMPL is itself a template template parm
> > > > > > > +	     then it cannot be the owning template.  */
> > > > > > >  	  tree decl = TREE_VALUE (parm);
> > > > > > > -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> > > > > > > +	  if (TREE_CODE (decl) == TEMPLATE_DECL
> > > > > > > +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> > > > > > 
> > > > > > IIUC a TEMPLATE_DECL inside a template parameter list always represents
> > > > > > a template template parm, so won't this effectively disable the
> > > > > > DECL_CONTEXT setting logic?
> > > > > 
> > > > > This is only when 'tmpl' (i.e. the containing TEMPLATE_DECL that we're
> > > > > streaming) is itself a template template parm.
> > > > 
> > > > D'oh, makes sense.
> > > > 
> > > > > 
> > > > > > >  	    {
> > > > > > >  	      tree inner = DECL_TEMPLATE_RESULT (decl);
> > > > > > >  	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> > > > > > > @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
> > > > > > >  			  : DECL_INITIAL (inner));
> > > > > > >  	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> > > > > > >  			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> > > > > > > -	      /* Original template template parms have a context
> > > > > > > -		 of their owning template.  Reduced ones do not.  */
> > > > > > >  	      if (original)
> > > > > > >  		DECL_CONTEXT (decl) = tmpl;
> > > > > > >  	    }
> > > > > > > diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > > > new file mode 100644
> > > > > > > index 00000000000..21bbc054fa3
> > > > > > > --- /dev/null
> > > > > > > +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> > > > > > > @@ -0,0 +1,11 @@
> > > > > > > +// PR c++/98881
> > > > > > > +// { dg-additional-options "-fmodule-header" }
> > > > > > > +// { dg-module-cmi {} }
> > > > > > > +
> > > > > > > +template <typename P> struct X {};
> > > > > > > +
> > > > > > > +template<template <typename> typename TT>
> > > > > > > +struct X<TT<int>> {
> > > > > > > +  template<template <typename> typename UU>
> > > > > > > +  void f (X<UU<int>>&);
> > > > > > > +};
> > > > > > 
> > > > > > I wonder why the partial specialization is relevant here?  I can't
> > > > > > seem to trigger the ICE without the partial specialization.
> > > > > > Slightly further reduced to not use bound ttps:
> > > > > > 
> > > > > >     template<template<class> class TT>
> > > > > >     struct X { };
> > > > > > 
> > > > > >     template<template<class> class TT> requires true
> > > > > >     struct X<TT> {
> > > > > >       template<template<class> class UU>
> > > > > >       void f(X<UU>);
> > > > > >     };
> > > > > > 
> > > > > > Maybe the expectation is that tpl_parms_fini for UU should be called
> > > > > > with tpl_levels=1 (so that we stream only its own template parameters)
> > > > > > but it's instead called with tpl_levels=3 for some reason?
> > > > > > 
> > > > > > IIUC that assert should always hold in the first iteration of the loop
> > > > > > (for the ttp's own template parameters).  Perhaps for subsequent
> > > > > > iterations we need to actually stream the contexts?
> > > > > > 
> > > > > > Ah, but we also ICE in the same spot with:
> > > > > > 
> > > > > >   template<template<class> class TT> struct X;      // #1
> > > > > >   template<template<class> class UU> struct X { };  // #2
> > > > > > 
> > > > > > because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
> > > > > > bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
> > > > > > of each ttp as-is instead of trying to avoid it via an apparently
> > > > > > fragile assumption?
> > > > > > 
> > > > > 
> > > > > That seems reasonable to me. Probably better to aim for correctness at
> > > > > this stage rather than relying on things I don't understand for slight
> > > > > performance gains.
> > > > > 
> > > > > I started putting together a patch to always restream the DECL_CONTEXT,
> > > > > but as I was doing so I realised that actually, we stream the context
> > > > > anyway, and this value should be correct to start with? I don't think
> > > > > deduplication should affect this, but I've added to the test to make
> > > > > sure that there's no issues here that I can see.
> > > > 
> > > > Where do we otherwise stream the context?  In trees_out::core_vals we
> > > > seem to avoid streaming the context for template parms (including ttps):
> > > > 
> > > >     if (!DECL_TEMPLATE_PARM_P (t))
> > > >       WT (t->decl_minimal.context);
> > > 
> > > Ah whoops, I was looking at the wrong 'context' (type_common.context). I
> > > guess I need to find a testcase that actually relies on DECL_CONTEXT
> > > being set correctly for template-template parameters.
> > 
> > It might be hard to come up with such a testcase since DECL_CONTEXT
> > has been unreliably set for ttps probably since inception (for example
> > for level-lowered ttps), so most logic is already robust to context-less
> > ttps.  But I suppose we might as well stream it anyway.
> 
> Fair enough. I've tried for a while but I haven't been able to work
> anything out; maybe another time.
> 
> Otherwise here's an updated version of the patch. Bootstrapped and
> regtested (so far just modules.exp) on x86_64-pc-linux-gnu: OK for
> trunk if full regtest succeeds?

LGTM!  Obliviously streaming the context seems like the best way to go
in light of the quirks about when/how it's set.

> 
> -- >8 --
> 
> Subject: [PATCH] c++: Stream DECL_CONTEXT for template template parms [PR98881]
> 
> When streaming in a nested template-template parameter as in the
> attached testcase, we end up reaching the containing template-template
> parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> this (nested) template-template parameter, as it should already be the
> struct that the outer template-template parameter is declared on.
> 
> The precise logic for what DECL_CONTEXT should be for a template
> template parameter in various situations seems rather obscure. Rather
> than trying to determine the assumptions that need to hold, it seems
> simpler to just always re-stream the DECL_CONTEXT as needed for now.
> 
> 	PR c++/98881
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_out::tpl_parms_fini): Stream out DECL_CONTEXT
> 	for template template parameters.
> 	(trees_in::tpl_parms_fini): Read it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/tpl-tpl-parm-3.h: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> Reviewed-by: Patrick Palka <ppalka@redhat.com>
> Reviewed-by: Jason Merrill <jason@redhat.com>
> ---
>  gcc/cp/module.cc                              | 41 +++++--------------
>  gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h | 12 ++++++
>  .../g++.dg/modules/tpl-tpl-parm-3_a.H         |  5 +++
>  .../g++.dg/modules/tpl-tpl-parm-3_b.C         |  5 +++
>  .../g++.dg/modules/tpl-tpl-parm-3_c.C         | 15 +++++++
>  5 files changed, 47 insertions(+), 31 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
>  create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 67f132d28d7..fcb449563ab 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -10126,23 +10126,12 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>  	  tree dflt = TREE_PURPOSE (parm);
>  	  tree_node (dflt);
>  
> -	  if (streaming_p ())
> -	    {
> -	      tree decl = TREE_VALUE (parm);
> -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> -		{
> -		  tree ctx = DECL_CONTEXT (decl);
> -		  tree inner = DECL_TEMPLATE_RESULT (decl);
> -		  tree tpi = (TREE_CODE (inner) == TYPE_DECL
> -			      ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> -			      : DECL_INITIAL (inner));
> -		  bool original = (TEMPLATE_PARM_LEVEL (tpi)
> -				   == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -		  /* Original template template parms have a context
> -		     of their owning template.  Reduced ones do not.  */
> -		  gcc_checking_assert (original ? ctx == tmpl : !ctx);
> -		}
> -	    }
> +	  /* Template template parameters need a context of their owning
> +	     template. This is quite tricky to infer correctly on stream-in
> +	     (see PR c++/98881) so we'll just provide it directly.  */
> +	  tree decl = TREE_VALUE (parm);
> +	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> +	    tree_node (DECL_CONTEXT (decl));
>  	}
>      }
>  }
> @@ -10160,24 +10149,14 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>  	{
>  	  tree parm = TREE_VEC_ELT (vec, ix);
>  	  tree dflt = tree_node ();
> -	  if (get_overrun ())
> -	    return false;
>  	  TREE_PURPOSE (parm) = dflt;
>  
>  	  tree decl = TREE_VALUE (parm);
>  	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> -	    {
> -	      tree inner = DECL_TEMPLATE_RESULT (decl);
> -	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> -			  ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> -			  : DECL_INITIAL (inner));
> -	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> -			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -	      /* Original template template parms have a context
> -		 of their owning template.  Reduced ones do not.  */
> -	      if (original)
> -		DECL_CONTEXT (decl) = tmpl;
> -	    }
> +	    DECL_CONTEXT (decl) = tree_node ();
> +
> +	  if (get_overrun ())
> +	    return false;
>  	}
>      }
>    return true;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> new file mode 100644
> index 00000000000..b0dcf353c23
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> @@ -0,0 +1,12 @@
> +// PR c++/98881
> +
> +template <typename P> struct X {};
> +
> +template<template <typename> typename TT>
> +struct X<TT<int>> {
> +  template<template <typename> typename UU>
> +  void f (X<UU<int>>&);
> +};
> +
> +template<template<class> class TT> struct Y;
> +template<template<class> class UU> struct Y { };
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> new file mode 100644
> index 00000000000..47464081f25
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> @@ -0,0 +1,5 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +#include "tpl-tpl-parm-3.h"
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> new file mode 100644
> index 00000000000..1e3cc6c444f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> @@ -0,0 +1,5 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> +
> +#include "tpl-tpl-parm-3.h"
> +import "tpl-tpl-parm-3_a.H";
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> new file mode 100644
> index 00000000000..5edba6cc64f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> @@ -0,0 +1,15 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "tpl-tpl-parm-3_a.H";
> +
> +template <typename T> struct A {};
> +template <typename T> struct B {};
> +
> +void foo() {
> +  X<A<int>> a;
> +  X<B<int>> b;
> +  a.f(b);
> +
> +  Y<A> y;
> +}
> -- 
> 2.43.2
> 
>

On 3/5/24 00:24, Nathaniel Shead wrote:
> On Mon, Mar 04, 2024 at 10:07:33PM -0500, Patrick Palka wrote:
>> On Tue, 5 Mar 2024, Nathaniel Shead wrote:
>>
>>> On Mon, Mar 04, 2024 at 09:26:00PM -0500, Patrick Palka wrote:
>>>> On Tue, 5 Mar 2024, Nathaniel Shead wrote:
>>>>
>>>>> On Mon, Mar 04, 2024 at 07:14:54PM -0500, Patrick Palka wrote:
>>>>>> On Sat, 2 Mar 2024, Nathaniel Shead wrote:
>>>>>>
>>>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, OK for trunk?
>>>>>>>
>>>>>>> -- >8 --
>>>>>>>
>>>>>>> When streaming in a nested template-template parameter as in the
>>>>>>> attached testcase, we end up reaching the containing template-template
>>>>>>> parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
>>>>>>> this (nested) template-template parameter, as it should already be the
>>>>>>> struct that the outer template-template parameter is declared on.
>>>>>>>
>>>>>>> 	PR c++/98881
>>>>>>>
>>>>>>> gcc/cp/ChangeLog:
>>>>>>>
>>>>>>> 	* module.cc (trees_out::tpl_parms_fini): Clarify logic purely
>>>>>>> 	for checking purposes. Don't consider a template template
>>>>>>> 	parameter as the owning template.
>>>>>>> 	(trees_in::tpl_parms_fini): Don't consider a template template
>>>>>>> 	parameter as the owning template.
>>>>>>>
>>>>>>> gcc/testsuite/ChangeLog:
>>>>>>>
>>>>>>> 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
>>>>>>> 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
>>>>>>>
>>>>>>> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
>>>>>>> ---
>>>>>>>   gcc/cp/module.cc                                | 17 ++++++++++++-----
>>>>>>>   gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H | 11 +++++++++++
>>>>>>>   gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C | 13 +++++++++++++
>>>>>>>   3 files changed, 36 insertions(+), 5 deletions(-)
>>>>>>>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>>>>>>>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
>>>>>>>
>>>>>>> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
>>>>>>> index 67f132d28d7..5663d01ed9c 100644
>>>>>>> --- a/gcc/cp/module.cc
>>>>>>> +++ b/gcc/cp/module.cc
>>>>>>> @@ -10126,10 +10126,14 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>>>>>>>   	  tree dflt = TREE_PURPOSE (parm);
>>>>>>>   	  tree_node (dflt);
>>>>>>>   
>>>>>>> -	  if (streaming_p ())
>>>>>>> +	  if (CHECKING_P && streaming_p ())
>>>>>>>   	    {
>>>>>>> +	      /* Sanity check that the DECL_CONTEXT we'll infer when
>>>>>>> +		 streaming in is correct.  */
>>>>>>>   	      tree decl = TREE_VALUE (parm);
>>>>>>> -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
>>>>>>> +	      if (TREE_CODE (decl) == TEMPLATE_DECL
>>>>>>> +		  /* A template template parm is not the owning template.  */
>>>>>>> +		  && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
>>>>>>>   		{
>>>>>>>   		  tree ctx = DECL_CONTEXT (decl);
>>>>>>>   		  tree inner = DECL_TEMPLATE_RESULT (decl);
>>>>>>> @@ -10164,8 +10168,13 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>>>>>>>   	    return false;
>>>>>>>   	  TREE_PURPOSE (parm) = dflt;
>>>>>>>   
>>>>>>> +	  /* Original template template parms have a context
>>>>>>> +	     of their owning template.  Reduced ones do not.
>>>>>>> +	     But if TMPL is itself a template template parm
>>>>>>> +	     then it cannot be the owning template.  */
>>>>>>>   	  tree decl = TREE_VALUE (parm);
>>>>>>> -	  if (TREE_CODE (decl) == TEMPLATE_DECL)
>>>>>>> +	  if (TREE_CODE (decl) == TEMPLATE_DECL
>>>>>>> +	      && !DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
>>>>>>
>>>>>> IIUC a TEMPLATE_DECL inside a template parameter list always represents
>>>>>> a template template parm, so won't this effectively disable the
>>>>>> DECL_CONTEXT setting logic?
>>>>>
>>>>> This is only when 'tmpl' (i.e. the containing TEMPLATE_DECL that we're
>>>>> streaming) is itself a template template parm.
>>>>
>>>> D'oh, makes sense.
>>>>
>>>>>
>>>>>>>   	    {
>>>>>>>   	      tree inner = DECL_TEMPLATE_RESULT (decl);
>>>>>>>   	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
>>>>>>> @@ -10173,8 +10182,6 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>>>>>>>   			  : DECL_INITIAL (inner));
>>>>>>>   	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
>>>>>>>   			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
>>>>>>> -	      /* Original template template parms have a context
>>>>>>> -		 of their owning template.  Reduced ones do not.  */
>>>>>>>   	      if (original)
>>>>>>>   		DECL_CONTEXT (decl) = tmpl;
>>>>>>>   	    }
>>>>>>> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>>>>>>> new file mode 100644
>>>>>>> index 00000000000..21bbc054fa3
>>>>>>> --- /dev/null
>>>>>>> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>>>>>>> @@ -0,0 +1,11 @@
>>>>>>> +// PR c++/98881
>>>>>>> +// { dg-additional-options "-fmodule-header" }
>>>>>>> +// { dg-module-cmi {} }
>>>>>>> +
>>>>>>> +template <typename P> struct X {};
>>>>>>> +
>>>>>>> +template<template <typename> typename TT>
>>>>>>> +struct X<TT<int>> {
>>>>>>> +  template<template <typename> typename UU>
>>>>>>> +  void f (X<UU<int>>&);
>>>>>>> +};
>>>>>>
>>>>>> I wonder why the partial specialization is relevant here?  I can't
>>>>>> seem to trigger the ICE without the partial specialization.
>>>>>> Slightly further reduced to not use bound ttps:
>>>>>>
>>>>>>      template<template<class> class TT>
>>>>>>      struct X { };
>>>>>>
>>>>>>      template<template<class> class TT> requires true
>>>>>>      struct X<TT> {
>>>>>>        template<template<class> class UU>
>>>>>>        void f(X<UU>);
>>>>>>      };
>>>>>>
>>>>>> Maybe the expectation is that tpl_parms_fini for UU should be called
>>>>>> with tpl_levels=1 (so that we stream only its own template parameters)
>>>>>> but it's instead called with tpl_levels=3 for some reason?
>>>>>>
>>>>>> IIUC that assert should always hold in the first iteration of the loop
>>>>>> (for the ttp's own template parameters).  Perhaps for subsequent
>>>>>> iterations we need to actually stream the contexts?
>>>>>>
>>>>>> Ah, but we also ICE in the same spot with:
>>>>>>
>>>>>>    template<template<class> class TT> struct X;      // #1
>>>>>>    template<template<class> class UU> struct X { };  // #2
>>>>>>
>>>>>> because DECL_CONTEXT of UU is #1 but tmpl is #2 (which seem like a
>>>>>> bug)...  But maybe this suggests we should just stream the DECL_CONTEXT
>>>>>> of each ttp as-is instead of trying to avoid it via an apparently
>>>>>> fragile assumption?
>>>>>>
>>>>>
>>>>> That seems reasonable to me. Probably better to aim for correctness at
>>>>> this stage rather than relying on things I don't understand for slight
>>>>> performance gains.
>>>>>
>>>>> I started putting together a patch to always restream the DECL_CONTEXT,
>>>>> but as I was doing so I realised that actually, we stream the context
>>>>> anyway, and this value should be correct to start with? I don't think
>>>>> deduplication should affect this, but I've added to the test to make
>>>>> sure that there's no issues here that I can see.
>>>>
>>>> Where do we otherwise stream the context?  In trees_out::core_vals we
>>>> seem to avoid streaming the context for template parms (including ttps):
>>>>
>>>>      if (!DECL_TEMPLATE_PARM_P (t))
>>>>        WT (t->decl_minimal.context);
>>>
>>> Ah whoops, I was looking at the wrong 'context' (type_common.context). I
>>> guess I need to find a testcase that actually relies on DECL_CONTEXT
>>> being set correctly for template-template parameters.
>>
>> It might be hard to come up with such a testcase since DECL_CONTEXT
>> has been unreliably set for ttps probably since inception (for example
>> for level-lowered ttps), so most logic is already robust to context-less
>> ttps.  But I suppose we might as well stream it anyway.
> 
> Fair enough. I've tried for a while but I haven't been able to work
> anything out; maybe another time.
> 
> Otherwise here's an updated version of the patch. Bootstrapped and
> regtested (so far just modules.exp) on x86_64-pc-linux-gnu: OK for
> trunk if full regtest succeeds?

OK.

> -- >8 --
> 
> Subject: [PATCH] c++: Stream DECL_CONTEXT for template template parms [PR98881]
> 
> When streaming in a nested template-template parameter as in the
> attached testcase, we end up reaching the containing template-template
> parameter in 'tpl_parms_fini'. We should not set the DECL_CONTEXT to
> this (nested) template-template parameter, as it should already be the
> struct that the outer template-template parameter is declared on.
> 
> The precise logic for what DECL_CONTEXT should be for a template
> template parameter in various situations seems rather obscure. Rather
> than trying to determine the assumptions that need to hold, it seems
> simpler to just always re-stream the DECL_CONTEXT as needed for now.
> 
> 	PR c++/98881
> 
> gcc/cp/ChangeLog:
> 
> 	* module.cc (trees_out::tpl_parms_fini): Stream out DECL_CONTEXT
> 	for template template parameters.
> 	(trees_in::tpl_parms_fini): Read it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/modules/tpl-tpl-parm-3.h: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_a.H: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_b.C: New test.
> 	* g++.dg/modules/tpl-tpl-parm-3_c.C: New test.
> 
> Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
> Reviewed-by: Patrick Palka <ppalka@redhat.com>
> Reviewed-by: Jason Merrill <jason@redhat.com>
> ---
>   gcc/cp/module.cc                              | 41 +++++--------------
>   gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h | 12 ++++++
>   .../g++.dg/modules/tpl-tpl-parm-3_a.H         |  5 +++
>   .../g++.dg/modules/tpl-tpl-parm-3_b.C         |  5 +++
>   .../g++.dg/modules/tpl-tpl-parm-3_c.C         | 15 +++++++
>   5 files changed, 47 insertions(+), 31 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
>   create mode 100644 gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> 
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index 67f132d28d7..fcb449563ab 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -10126,23 +10126,12 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>   	  tree dflt = TREE_PURPOSE (parm);
>   	  tree_node (dflt);
>   
> -	  if (streaming_p ())
> -	    {
> -	      tree decl = TREE_VALUE (parm);
> -	      if (TREE_CODE (decl) == TEMPLATE_DECL)
> -		{
> -		  tree ctx = DECL_CONTEXT (decl);
> -		  tree inner = DECL_TEMPLATE_RESULT (decl);
> -		  tree tpi = (TREE_CODE (inner) == TYPE_DECL
> -			      ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> -			      : DECL_INITIAL (inner));
> -		  bool original = (TEMPLATE_PARM_LEVEL (tpi)
> -				   == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -		  /* Original template template parms have a context
> -		     of their owning template.  Reduced ones do not.  */
> -		  gcc_checking_assert (original ? ctx == tmpl : !ctx);
> -		}
> -	    }
> +	  /* Template template parameters need a context of their owning
> +	     template. This is quite tricky to infer correctly on stream-in
> +	     (see PR c++/98881) so we'll just provide it directly.  */
> +	  tree decl = TREE_VALUE (parm);
> +	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> +	    tree_node (DECL_CONTEXT (decl));
>   	}
>       }
>   }
> @@ -10160,24 +10149,14 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned tpl_levels)
>   	{
>   	  tree parm = TREE_VEC_ELT (vec, ix);
>   	  tree dflt = tree_node ();
> -	  if (get_overrun ())
> -	    return false;
>   	  TREE_PURPOSE (parm) = dflt;
>   
>   	  tree decl = TREE_VALUE (parm);
>   	  if (TREE_CODE (decl) == TEMPLATE_DECL)
> -	    {
> -	      tree inner = DECL_TEMPLATE_RESULT (decl);
> -	      tree tpi = (TREE_CODE (inner) == TYPE_DECL
> -			  ? TEMPLATE_TYPE_PARM_INDEX (TREE_TYPE (decl))
> -			  : DECL_INITIAL (inner));
> -	      bool original = (TEMPLATE_PARM_LEVEL (tpi)
> -			       == TEMPLATE_PARM_ORIG_LEVEL (tpi));
> -	      /* Original template template parms have a context
> -		 of their owning template.  Reduced ones do not.  */
> -	      if (original)
> -		DECL_CONTEXT (decl) = tmpl;
> -	    }
> +	    DECL_CONTEXT (decl) = tree_node ();
> +
> +	  if (get_overrun ())
> +	    return false;
>   	}
>       }
>     return true;
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> new file mode 100644
> index 00000000000..b0dcf353c23
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3.h
> @@ -0,0 +1,12 @@
> +// PR c++/98881
> +
> +template <typename P> struct X {};
> +
> +template<template <typename> typename TT>
> +struct X<TT<int>> {
> +  template<template <typename> typename UU>
> +  void f (X<UU<int>>&);
> +};
> +
> +template<template<class> class TT> struct Y;
> +template<template<class> class UU> struct Y { };
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> new file mode 100644
> index 00000000000..47464081f25
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_a.H
> @@ -0,0 +1,5 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodule-header" }
> +// { dg-module-cmi {} }
> +
> +#include "tpl-tpl-parm-3.h"
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> new file mode 100644
> index 00000000000..1e3cc6c444f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_b.C
> @@ -0,0 +1,5 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
> +
> +#include "tpl-tpl-parm-3.h"
> +import "tpl-tpl-parm-3_a.H";
> diff --git a/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> new file mode 100644
> index 00000000000..5edba6cc64f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/tpl-tpl-parm-3_c.C
> @@ -0,0 +1,15 @@
> +// PR c++/98881
> +// { dg-additional-options "-fmodules-ts" }
> +
> +import "tpl-tpl-parm-3_a.H";
> +
> +template <typename T> struct A {};
> +template <typename T> struct B {};
> +
> +void foo() {
> +  X<A<int>> a;
> +  X<B<int>> b;
> +  a.f(b);
> +
> +  Y<A> y;
> +}