diff mbox series

c++: ttp CTAD equivalence [PR112737]

Message ID 20240131171245.1102131-1-ppalka@redhat.com
State New
Headers show
Series c++: ttp CTAD equivalence [PR112737] | expand

Commit Message

Patrick Palka Jan. 31, 2024, 5:12 p.m. UTC 
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

-- >8 --

Here during declaration matching we undesirably consider the two TT{42}
CTAD expressions to be non-equivalent ultimately because for CTAD
placeholder equivalence we compare the TEMPLATE_DECLs (which uses
pointer identity) and here the corresponding TEMPLATE_DECLs for TT are
different since they're from different scopes.  On the other hand, the
corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they
have the same position and template parameters).  This turns out to be
the root cause of some of the xtreme-header modules regressions.

We don't have this ttp equivalence issue in other contexts because either
the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already
(e.g. when a ttp is used as a targ), or the equivalence logic is relaxed
(e.g. for bound ttps), it seems.

So this patch relaxes ttp CTAD placeholder equivalence accordingly, by
comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL.  The
ctp_hasher doesn't need to be adjusted since it currently doesn't include
CLASS_PLACEHOLDER_TEMPLATE in the hash anyway.

	PR c++/112737

gcc/cp/ChangeLog:

	* typeck.cc (structural_comptypes) <case TEMPLATE_TYPE_PARM>:
	If CLASS_PLACEHOLDER_TEMPLATE is a ttp, compare its
	TEMPLATE_TEMPLATE_PARM instead of its TEMPLATE_DECL.

gcc/testsuite/ChangeLog:

	* g++.dg/template/ttp42: New test.
---
 gcc/cp/typeck.cc                      | 20 +++++++++++++++++---
 gcc/testsuite/g++.dg/template/ttp42.C | 14 ++++++++++++++
 2 files changed, 31 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C

Comments

Jason Merrill Jan. 31, 2024, 7:23 p.m. UTC | #1 
On 1/31/24 12:12, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> -- >8 --
> 
> Here during declaration matching we undesirably consider the two TT{42}
> CTAD expressions to be non-equivalent ultimately because for CTAD
> placeholder equivalence we compare the TEMPLATE_DECLs (which uses
> pointer identity) and here the corresponding TEMPLATE_DECLs for TT are
> different since they're from different scopes.  On the other hand, the
> corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they
> have the same position and template parameters).  This turns out to be
> the root cause of some of the xtreme-header modules regressions.
> 
> We don't have this ttp equivalence issue in other contexts because either
> the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already
> (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed
> (e.g. for bound ttps), it seems.
> 
> So this patch relaxes ttp CTAD placeholder equivalence accordingly, by
> comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL.  The
> ctp_hasher doesn't need to be adjusted since it currently doesn't include
> CLASS_PLACEHOLDER_TEMPLATE in the hash anyway.

Maybe put this handling in cp_tree_equal and call it from here?  Does 
iterative_hash_template_arg need something similar?

> 	PR c++/112737
> 
> gcc/cp/ChangeLog:
> 
> 	* typeck.cc (structural_comptypes) <case TEMPLATE_TYPE_PARM>:
> 	If CLASS_PLACEHOLDER_TEMPLATE is a ttp, compare its
> 	TEMPLATE_TEMPLATE_PARM instead of its TEMPLATE_DECL.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/ttp42: New test.
> ---
>   gcc/cp/typeck.cc                      | 20 +++++++++++++++++---
>   gcc/testsuite/g++.dg/template/ttp42.C | 14 ++++++++++++++
>   2 files changed, 31 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C
> 
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index a15eda3f5f8..be82877e35c 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -1573,9 +1573,23 @@ structural_comptypes (tree t1, tree t2, int strict)
>   	return false;
>         /* If T1 and T2 don't represent the same class template deduction,
>            they aren't equal.  */
> -      if (CLASS_PLACEHOLDER_TEMPLATE (t1)
> -	  != CLASS_PLACEHOLDER_TEMPLATE (t2))
> -	return false;
> +      if (CLASS_PLACEHOLDER_TEMPLATE (t1) || CLASS_PLACEHOLDER_TEMPLATE (t2))
> +	{
> +	  tree tmpl1 = CLASS_PLACEHOLDER_TEMPLATE (t1);
> +	  tree tmpl2 = CLASS_PLACEHOLDER_TEMPLATE (t2);
> +	  if (!tmpl1 || !tmpl2)
> +	    return false;
> +	  if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl1)
> +	      != DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl2))
> +	    return false;
> +	  if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl1)
> +	      /* Treat two CTAD placeholders that use equivalent ttps
> +		 from different scopes as equivalent by comparing their
> +		 TEMPLATE_TEMPLATE_PARMs instead of their TEMPLATE_DECLs.  */
> +	      ? !same_type_p (TREE_TYPE (tmpl1), TREE_TYPE (tmpl2))
> +	      : tmpl1 != tmpl2)
> +	    return false;
> +	}
>         /* Constrained 'auto's are distinct from parms that don't have the same
>   	 constraints.  */
>         if (!equivalent_placeholder_constraints (t1, t2))
> diff --git a/gcc/testsuite/g++.dg/template/ttp42.C b/gcc/testsuite/g++.dg/template/ttp42.C
> new file mode 100644
> index 00000000000..da08e857fc5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp42.C
> @@ -0,0 +1,14 @@
> +// PR c++/112737
> +// { dg-do compile { target c++17 } }
> +
> +template<template<class> class TT>
> +decltype(TT{42}) f(); // #1
> +
> +template<template<class> class TT>
> +decltype(TT{42}) f(); // redeclaration of #1
> +
> +template<class T> struct A { A(T); };
> +
> +int main() {
> +  f<A>();
> +}
Patrick Palka Jan. 31, 2024, 9:03 p.m. UTC | #2 
On Wed, 31 Jan 2024, Jason Merrill wrote:

> On 1/31/24 12:12, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> > 
> > -- >8 --
> > 
> > Here during declaration matching we undesirably consider the two TT{42}
> > CTAD expressions to be non-equivalent ultimately because for CTAD
> > placeholder equivalence we compare the TEMPLATE_DECLs (which uses
> > pointer identity) and here the corresponding TEMPLATE_DECLs for TT are
> > different since they're from different scopes.  On the other hand, the
> > corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they
> > have the same position and template parameters).  This turns out to be
> > the root cause of some of the xtreme-header modules regressions.
> > 
> > We don't have this ttp equivalence issue in other contexts because either
> > the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already
> > (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed
> > (e.g. for bound ttps), it seems.
> > 
> > So this patch relaxes ttp CTAD placeholder equivalence accordingly, by
> > comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL.  The
> > ctp_hasher doesn't need to be adjusted since it currently doesn't include
> > CLASS_PLACEHOLDER_TEMPLATE in the hash anyway.
> 
> Maybe put this handling in cp_tree_equal and call it from here?  Does
> iterative_hash_template_arg need something similar?

I was hoping cp_tree_equal would never be called for a ttp TEMPLATE_DECL
after this patch, and so it wouldn't matter either way, but it turns out
to matter for a function template-id:

  template<template<class> class, class T>
  void g(T);

  template<template<class> class TT, class T>
  decltype(g<TT>(T{})) f(T); // #1

  template<template<class> class TT, class T>
  decltype(g<TT>(T{})) f(T); // redeclaration of #1

  template<class T> struct A { A(T); };

  int main() {
    f<A>(0);
  }

Here we represent TT in g<TT> as a TEMPLATE_DECL because it's not until
coercion that convert_template_argument turns it into a
TEMPLATE_TEMPLATE_PARM, but of course we can't coerce until the call is
non-dependent and we know which function we're calling.  (So TT within
a class, variable or alias template-id would be represented as a
TEMPLATE_TEMPLATE_PARM since we can do coercion ahead of time in that
case.)

So indeed it seems desirable to handle this in cp_tree_equal... like so?
Bootstrap and regtest nearly finished.

-- >8 --

	PR c++/112737

gcc/cp/ChangeLog:

	* pt.cc (iterative_hash_template_arg) <case TEMPLATE_DECL>:
	Adjust hashing to match cp_tree_equal.
	(ctp_hasher::hash): Also hash CLASS_PLACEHOLDER_TEMPLATE.
	* tree.cc (cp_tree_equal) <case TEMPLATE_DECL>: Return true
	for ttp TEMPLATE_DECLs if their TEMPLATE_TEMPLATE_PARMs are
	equivalent.
	* typeck.cc (structural_comptypes) <case TEMPLATE_TYPE_PARM>:
	Use cp_tree_equal to compare CLASS_PLACEHOLDER_TEMPLATE.

gcc/testsuite/ChangeLog:

	* g++.dg/template/ttp42.C: New test.
	* g++.dg/template/ttp42a.C: New test.
---
 gcc/cp/pt.cc                           |  9 +++++++++
 gcc/cp/tree.cc                         |  6 +++++-
 gcc/cp/typeck.cc                       |  4 ++--
 gcc/testsuite/g++.dg/template/ttp42.C  | 14 ++++++++++++++
 gcc/testsuite/g++.dg/template/ttp42a.C | 18 ++++++++++++++++++
 5 files changed, 48 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C
 create mode 100644 gcc/testsuite/g++.dg/template/ttp42a.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 5871cb668d0..ca454758ca7 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -1816,6 +1816,13 @@ iterative_hash_template_arg (tree arg, hashval_t val)
 	}
       return iterative_hash_template_arg (TREE_TYPE (arg), val);
 
+    case TEMPLATE_DECL:
+      if (DECL_TEMPLATE_TEMPLATE_PARM_P (arg))
+	return iterative_hash_template_arg (TREE_TYPE (arg), val);
+      else
+	/* Hash it like any other declaration.  */
+	break;
+
     case TARGET_EXPR:
       return iterative_hash_template_arg (TARGET_EXPR_INITIAL (arg), val);
 
@@ -4499,6 +4506,8 @@ struct ctp_hasher : ggc_ptr_hash<tree_node>
     hashval_t val = iterative_hash_object (code, 0);
     val = iterative_hash_object (TEMPLATE_TYPE_LEVEL (t), val);
     val = iterative_hash_object (TEMPLATE_TYPE_IDX (t), val);
+    if (TREE_CODE (t) == TEMPLATE_TYPE_PARM)
+      val = iterative_hash_template_arg (CLASS_PLACEHOLDER_TEMPLATE (t), val);
     if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM)
       val = iterative_hash_template_arg (TYPE_TI_ARGS (t), val);
     --comparing_specializations;
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 77f57e0f9ac..5c8c05dc168 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -4084,11 +4084,15 @@ cp_tree_equal (tree t1, tree t2)
 	}
       return false;
 
+    case TEMPLATE_DECL:
+      if (DECL_TEMPLATE_TEMPLATE_PARM_P (t1)
+	  && DECL_TEMPLATE_TEMPLATE_PARM_P (t2))
+	return cp_tree_equal (TREE_TYPE (t1), TREE_TYPE (t2));
+      /* Fall through.  */
     case VAR_DECL:
     case CONST_DECL:
     case FIELD_DECL:
     case FUNCTION_DECL:
-    case TEMPLATE_DECL:
     case IDENTIFIER_NODE:
     case SSA_NAME:
     case USING_DECL:
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index a15eda3f5f8..87109e63138 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -1573,8 +1573,8 @@ structural_comptypes (tree t1, tree t2, int strict)
 	return false;
       /* If T1 and T2 don't represent the same class template deduction,
          they aren't equal.  */
-      if (CLASS_PLACEHOLDER_TEMPLATE (t1)
-	  != CLASS_PLACEHOLDER_TEMPLATE (t2))
+      if (!cp_tree_equal (CLASS_PLACEHOLDER_TEMPLATE (t1),
+			  CLASS_PLACEHOLDER_TEMPLATE (t2)))
 	return false;
       /* Constrained 'auto's are distinct from parms that don't have the same
 	 constraints.  */
diff --git a/gcc/testsuite/g++.dg/template/ttp42.C b/gcc/testsuite/g++.dg/template/ttp42.C
new file mode 100644
index 00000000000..da08e857fc5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ttp42.C
@@ -0,0 +1,14 @@
+// PR c++/112737
+// { dg-do compile { target c++17 } }
+
+template<template<class> class TT>
+decltype(TT{42}) f(); // #1
+
+template<template<class> class TT>
+decltype(TT{42}) f(); // redeclaration of #1
+
+template<class T> struct A { A(T); };
+
+int main() {
+  f<A>();
+}
diff --git a/gcc/testsuite/g++.dg/template/ttp42a.C b/gcc/testsuite/g++.dg/template/ttp42a.C
new file mode 100644
index 00000000000..e890deda3a3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ttp42a.C
@@ -0,0 +1,18 @@
+// PR c++/112737
+// A version of ttp42.C using a function template-id instead of ttp CTAD.
+// { dg-do compile { target c++11 } }
+
+template<template<class> class, class T>
+void g(T);
+
+template<template<class> class TT, class T>
+decltype(g<TT>(T{})) f(T); // #1
+
+template<template<class> class TT, class T>
+decltype(g<TT>(T{})) f(T); // redeclaration of #1
+
+template<class T> struct A { A(T); };
+
+int main() {
+  f<A>(0);
+}
Patrick Palka Jan. 31, 2024, 9:04 p.m. UTC | #3 
On Wed, 31 Jan 2024, Patrick Palka wrote:

> On Wed, 31 Jan 2024, Jason Merrill wrote:
> 
> > On 1/31/24 12:12, Patrick Palka wrote:
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > trunk?
> > > 
> > > -- >8 --
> > > 
> > > Here during declaration matching we undesirably consider the two TT{42}
> > > CTAD expressions to be non-equivalent ultimately because for CTAD
> > > placeholder equivalence we compare the TEMPLATE_DECLs (which uses
> > > pointer identity) and here the corresponding TEMPLATE_DECLs for TT are
> > > different since they're from different scopes.  On the other hand, the
> > > corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they
> > > have the same position and template parameters).  This turns out to be
> > > the root cause of some of the xtreme-header modules regressions.
> > > 
> > > We don't have this ttp equivalence issue in other contexts because either
> > > the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already
> > > (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed
> > > (e.g. for bound ttps), it seems.
> > > 
> > > So this patch relaxes ttp CTAD placeholder equivalence accordingly, by
> > > comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL.  The
> > > ctp_hasher doesn't need to be adjusted since it currently doesn't include
> > > CLASS_PLACEHOLDER_TEMPLATE in the hash anyway.
> > 
> > Maybe put this handling in cp_tree_equal and call it from here?  Does
> > iterative_hash_template_arg need something similar?
> 
> I was hoping cp_tree_equal would never be called for a ttp TEMPLATE_DECL
> after this patch, and so it wouldn't matter either way, but it turns out
> to matter for a function template-id:
> 
>   template<template<class> class, class T>
>   void g(T);
> 
>   template<template<class> class TT, class T>
>   decltype(g<TT>(T{})) f(T); // #1
> 
>   template<template<class> class TT, class T>
>   decltype(g<TT>(T{})) f(T); // redeclaration of #1
> 
>   template<class T> struct A { A(T); };
> 
>   int main() {
>     f<A>(0);
>   }
> 
> Here we represent TT in g<TT> as a TEMPLATE_DECL because it's not until
> coercion that convert_template_argument turns it into a
> TEMPLATE_TEMPLATE_PARM, but of course we can't coerce until the call is
> non-dependent and we know which function we're calling.  (So TT within
> a class, variable or alias template-id would be represented as a
> TEMPLATE_TEMPLATE_PARM since we can do coercion ahead of time in that
> case.)
> 
> So indeed it seems desirable to handle this in cp_tree_equal... like so?
> Bootstrap and regtest nearly finished.
> 
> -- >8 --
> 
> 	PR c++/112737
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (iterative_hash_template_arg) <case TEMPLATE_DECL>:
> 	Adjust hashing to match cp_tree_equal.
> 	(ctp_hasher::hash): Also hash CLASS_PLACEHOLDER_TEMPLATE.

I forgot to mention this change to ctp_hasher::hash is just a drive-by
optimization, so that CTAD placeholders don't all get the same hash.

> 	* tree.cc (cp_tree_equal) <case TEMPLATE_DECL>: Return true
> 	for ttp TEMPLATE_DECLs if their TEMPLATE_TEMPLATE_PARMs are
> 	equivalent.
> 	* typeck.cc (structural_comptypes) <case TEMPLATE_TYPE_PARM>:
> 	Use cp_tree_equal to compare CLASS_PLACEHOLDER_TEMPLATE.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/ttp42.C: New test.
> 	* g++.dg/template/ttp42a.C: New test.
> ---
>  gcc/cp/pt.cc                           |  9 +++++++++
>  gcc/cp/tree.cc                         |  6 +++++-
>  gcc/cp/typeck.cc                       |  4 ++--
>  gcc/testsuite/g++.dg/template/ttp42.C  | 14 ++++++++++++++
>  gcc/testsuite/g++.dg/template/ttp42a.C | 18 ++++++++++++++++++
>  5 files changed, 48 insertions(+), 3 deletions(-)
>  create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C
>  create mode 100644 gcc/testsuite/g++.dg/template/ttp42a.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 5871cb668d0..ca454758ca7 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1816,6 +1816,13 @@ iterative_hash_template_arg (tree arg, hashval_t val)
>  	}
>        return iterative_hash_template_arg (TREE_TYPE (arg), val);
>  
> +    case TEMPLATE_DECL:
> +      if (DECL_TEMPLATE_TEMPLATE_PARM_P (arg))
> +	return iterative_hash_template_arg (TREE_TYPE (arg), val);
> +      else
> +	/* Hash it like any other declaration.  */
> +	break;
> +
>      case TARGET_EXPR:
>        return iterative_hash_template_arg (TARGET_EXPR_INITIAL (arg), val);
>  
> @@ -4499,6 +4506,8 @@ struct ctp_hasher : ggc_ptr_hash<tree_node>
>      hashval_t val = iterative_hash_object (code, 0);
>      val = iterative_hash_object (TEMPLATE_TYPE_LEVEL (t), val);
>      val = iterative_hash_object (TEMPLATE_TYPE_IDX (t), val);
> +    if (TREE_CODE (t) == TEMPLATE_TYPE_PARM)
> +      val = iterative_hash_template_arg (CLASS_PLACEHOLDER_TEMPLATE (t), val);
>      if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM)
>        val = iterative_hash_template_arg (TYPE_TI_ARGS (t), val);
>      --comparing_specializations;
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 77f57e0f9ac..5c8c05dc168 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -4084,11 +4084,15 @@ cp_tree_equal (tree t1, tree t2)
>  	}
>        return false;
>  
> +    case TEMPLATE_DECL:
> +      if (DECL_TEMPLATE_TEMPLATE_PARM_P (t1)
> +	  && DECL_TEMPLATE_TEMPLATE_PARM_P (t2))
> +	return cp_tree_equal (TREE_TYPE (t1), TREE_TYPE (t2));
> +      /* Fall through.  */
>      case VAR_DECL:
>      case CONST_DECL:
>      case FIELD_DECL:
>      case FUNCTION_DECL:
> -    case TEMPLATE_DECL:
>      case IDENTIFIER_NODE:
>      case SSA_NAME:
>      case USING_DECL:
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index a15eda3f5f8..87109e63138 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -1573,8 +1573,8 @@ structural_comptypes (tree t1, tree t2, int strict)
>  	return false;
>        /* If T1 and T2 don't represent the same class template deduction,
>           they aren't equal.  */
> -      if (CLASS_PLACEHOLDER_TEMPLATE (t1)
> -	  != CLASS_PLACEHOLDER_TEMPLATE (t2))
> +      if (!cp_tree_equal (CLASS_PLACEHOLDER_TEMPLATE (t1),
> +			  CLASS_PLACEHOLDER_TEMPLATE (t2)))
>  	return false;
>        /* Constrained 'auto's are distinct from parms that don't have the same
>  	 constraints.  */
> diff --git a/gcc/testsuite/g++.dg/template/ttp42.C b/gcc/testsuite/g++.dg/template/ttp42.C
> new file mode 100644
> index 00000000000..da08e857fc5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp42.C
> @@ -0,0 +1,14 @@
> +// PR c++/112737
> +// { dg-do compile { target c++17 } }
> +
> +template<template<class> class TT>
> +decltype(TT{42}) f(); // #1
> +
> +template<template<class> class TT>
> +decltype(TT{42}) f(); // redeclaration of #1
> +
> +template<class T> struct A { A(T); };
> +
> +int main() {
> +  f<A>();
> +}
> diff --git a/gcc/testsuite/g++.dg/template/ttp42a.C b/gcc/testsuite/g++.dg/template/ttp42a.C
> new file mode 100644
> index 00000000000..e890deda3a3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp42a.C
> @@ -0,0 +1,18 @@
> +// PR c++/112737
> +// A version of ttp42.C using a function template-id instead of ttp CTAD.
> +// { dg-do compile { target c++11 } }
> +
> +template<template<class> class, class T>
> +void g(T);
> +
> +template<template<class> class TT, class T>
> +decltype(g<TT>(T{})) f(T); // #1
> +
> +template<template<class> class TT, class T>
> +decltype(g<TT>(T{})) f(T); // redeclaration of #1
> +
> +template<class T> struct A { A(T); };
> +
> +int main() {
> +  f<A>(0);
> +}
> -- 
> 2.43.0.493.gbc7ee2e5e1
> 
>
Jason Merrill Feb. 1, 2024, 2:51 a.m. UTC | #4 
On 1/31/24 16:03, Patrick Palka wrote:
> On Wed, 31 Jan 2024, Jason Merrill wrote:
> 
>> On 1/31/24 12:12, Patrick Palka wrote:
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>> trunk?
>>>
>>> -- >8 --
>>>
>>> Here during declaration matching we undesirably consider the two TT{42}
>>> CTAD expressions to be non-equivalent ultimately because for CTAD
>>> placeholder equivalence we compare the TEMPLATE_DECLs (which uses
>>> pointer identity) and here the corresponding TEMPLATE_DECLs for TT are
>>> different since they're from different scopes.  On the other hand, the
>>> corresponding TEMPLATE_TEMPLATE_PARMs are deemed equivalent (since they
>>> have the same position and template parameters).  This turns out to be
>>> the root cause of some of the xtreme-header modules regressions.
>>>
>>> We don't have this ttp equivalence issue in other contexts because either
>>> the TEMPLATE_TEMPLATE_PARM is used instead of the TEMPLATE_DECL already
>>> (e.g. when a ttp is used as a targ), or the equivalence logic is relaxed
>>> (e.g. for bound ttps), it seems.
>>>
>>> So this patch relaxes ttp CTAD placeholder equivalence accordingly, by
>>> comparing the TEMPLATE_TEMPLATE_PARM instead of the TEMPLATE_DECL.  The
>>> ctp_hasher doesn't need to be adjusted since it currently doesn't include
>>> CLASS_PLACEHOLDER_TEMPLATE in the hash anyway.
>>
>> Maybe put this handling in cp_tree_equal and call it from here?  Does
>> iterative_hash_template_arg need something similar?
> 
> I was hoping cp_tree_equal would never be called for a ttp TEMPLATE_DECL
> after this patch, and so it wouldn't matter either way, but it turns out
> to matter for a function template-id:
> 
>    template<template<class> class, class T>
>    void g(T);
> 
>    template<template<class> class TT, class T>
>    decltype(g<TT>(T{})) f(T); // #1
> 
>    template<template<class> class TT, class T>
>    decltype(g<TT>(T{})) f(T); // redeclaration of #1
> 
>    template<class T> struct A { A(T); };
> 
>    int main() {
>      f<A>(0);
>    }
> 
> Here we represent TT in g<TT> as a TEMPLATE_DECL because it's not until
> coercion that convert_template_argument turns it into a
> TEMPLATE_TEMPLATE_PARM, but of course we can't coerce until the call is
> non-dependent and we know which function we're calling.  (So TT within
> a class, variable or alias template-id would be represented as a
> TEMPLATE_TEMPLATE_PARM since we can do coercion ahead of time in that
> case.)
> 
> So indeed it seems desirable to handle this in cp_tree_equal... like so?
> Bootstrap and regtest nearly finished.

OK.

> -- >8 --
> 
> 	PR c++/112737
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (iterative_hash_template_arg) <case TEMPLATE_DECL>:
> 	Adjust hashing to match cp_tree_equal.
> 	(ctp_hasher::hash): Also hash CLASS_PLACEHOLDER_TEMPLATE.
> 	* tree.cc (cp_tree_equal) <case TEMPLATE_DECL>: Return true
> 	for ttp TEMPLATE_DECLs if their TEMPLATE_TEMPLATE_PARMs are
> 	equivalent.
> 	* typeck.cc (structural_comptypes) <case TEMPLATE_TYPE_PARM>:
> 	Use cp_tree_equal to compare CLASS_PLACEHOLDER_TEMPLATE.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/ttp42.C: New test.
> 	* g++.dg/template/ttp42a.C: New test.
> ---
>   gcc/cp/pt.cc                           |  9 +++++++++
>   gcc/cp/tree.cc                         |  6 +++++-
>   gcc/cp/typeck.cc                       |  4 ++--
>   gcc/testsuite/g++.dg/template/ttp42.C  | 14 ++++++++++++++
>   gcc/testsuite/g++.dg/template/ttp42a.C | 18 ++++++++++++++++++
>   5 files changed, 48 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/ttp42.C
>   create mode 100644 gcc/testsuite/g++.dg/template/ttp42a.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 5871cb668d0..ca454758ca7 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1816,6 +1816,13 @@ iterative_hash_template_arg (tree arg, hashval_t val)
>   	}
>         return iterative_hash_template_arg (TREE_TYPE (arg), val);
>   
> +    case TEMPLATE_DECL:
> +      if (DECL_TEMPLATE_TEMPLATE_PARM_P (arg))
> +	return iterative_hash_template_arg (TREE_TYPE (arg), val);
> +      else
> +	/* Hash it like any other declaration.  */
> +	break;
> +
>       case TARGET_EXPR:
>         return iterative_hash_template_arg (TARGET_EXPR_INITIAL (arg), val);
>   
> @@ -4499,6 +4506,8 @@ struct ctp_hasher : ggc_ptr_hash<tree_node>
>       hashval_t val = iterative_hash_object (code, 0);
>       val = iterative_hash_object (TEMPLATE_TYPE_LEVEL (t), val);
>       val = iterative_hash_object (TEMPLATE_TYPE_IDX (t), val);
> +    if (TREE_CODE (t) == TEMPLATE_TYPE_PARM)
> +      val = iterative_hash_template_arg (CLASS_PLACEHOLDER_TEMPLATE (t), val);
>       if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM)
>         val = iterative_hash_template_arg (TYPE_TI_ARGS (t), val);
>       --comparing_specializations;
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 77f57e0f9ac..5c8c05dc168 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -4084,11 +4084,15 @@ cp_tree_equal (tree t1, tree t2)
>   	}
>         return false;
>   
> +    case TEMPLATE_DECL:
> +      if (DECL_TEMPLATE_TEMPLATE_PARM_P (t1)
> +	  && DECL_TEMPLATE_TEMPLATE_PARM_P (t2))
> +	return cp_tree_equal (TREE_TYPE (t1), TREE_TYPE (t2));
> +      /* Fall through.  */
>       case VAR_DECL:
>       case CONST_DECL:
>       case FIELD_DECL:
>       case FUNCTION_DECL:
> -    case TEMPLATE_DECL:
>       case IDENTIFIER_NODE:
>       case SSA_NAME:
>       case USING_DECL:
> diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
> index a15eda3f5f8..87109e63138 100644
> --- a/gcc/cp/typeck.cc
> +++ b/gcc/cp/typeck.cc
> @@ -1573,8 +1573,8 @@ structural_comptypes (tree t1, tree t2, int strict)
>   	return false;
>         /* If T1 and T2 don't represent the same class template deduction,
>            they aren't equal.  */
> -      if (CLASS_PLACEHOLDER_TEMPLATE (t1)
> -	  != CLASS_PLACEHOLDER_TEMPLATE (t2))
> +      if (!cp_tree_equal (CLASS_PLACEHOLDER_TEMPLATE (t1),
> +			  CLASS_PLACEHOLDER_TEMPLATE (t2)))
>   	return false;
>         /* Constrained 'auto's are distinct from parms that don't have the same
>   	 constraints.  */
> diff --git a/gcc/testsuite/g++.dg/template/ttp42.C b/gcc/testsuite/g++.dg/template/ttp42.C
> new file mode 100644
> index 00000000000..da08e857fc5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp42.C
> @@ -0,0 +1,14 @@
> +// PR c++/112737
> +// { dg-do compile { target c++17 } }
> +
> +template<template<class> class TT>
> +decltype(TT{42}) f(); // #1
> +
> +template<template<class> class TT>
> +decltype(TT{42}) f(); // redeclaration of #1
> +
> +template<class T> struct A { A(T); };
> +
> +int main() {
> +  f<A>();
> +}
> diff --git a/gcc/testsuite/g++.dg/template/ttp42a.C b/gcc/testsuite/g++.dg/template/ttp42a.C
> new file mode 100644
> index 00000000000..e890deda3a3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp42a.C
> @@ -0,0 +1,18 @@
> +// PR c++/112737
> +// A version of ttp42.C using a function template-id instead of ttp CTAD.
> +// { dg-do compile { target c++11 } }
> +
> +template<template<class> class, class T>
> +void g(T);
> +
> +template<template<class> class TT, class T>
> +decltype(g<TT>(T{})) f(T); // #1
> +
> +template<template<class> class TT, class T>
> +decltype(g<TT>(T{})) f(T); // redeclaration of #1
> +
> +template<class T> struct A { A(T); };
> +
> +int main() {
> +  f<A>(0);
> +}
diff mbox series

Patch

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index a15eda3f5f8..be82877e35c 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -1573,9 +1573,23 @@  structural_comptypes (tree t1, tree t2, int strict)
 	return false;
       /* If T1 and T2 don't represent the same class template deduction,
          they aren't equal.  */
-      if (CLASS_PLACEHOLDER_TEMPLATE (t1)
-	  != CLASS_PLACEHOLDER_TEMPLATE (t2))
-	return false;
+      if (CLASS_PLACEHOLDER_TEMPLATE (t1) || CLASS_PLACEHOLDER_TEMPLATE (t2))
+	{
+	  tree tmpl1 = CLASS_PLACEHOLDER_TEMPLATE (t1);
+	  tree tmpl2 = CLASS_PLACEHOLDER_TEMPLATE (t2);
+	  if (!tmpl1 || !tmpl2)
+	    return false;
+	  if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl1)
+	      != DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl2))
+	    return false;
+	  if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl1)
+	      /* Treat two CTAD placeholders that use equivalent ttps
+		 from different scopes as equivalent by comparing their
+		 TEMPLATE_TEMPLATE_PARMs instead of their TEMPLATE_DECLs.  */
+	      ? !same_type_p (TREE_TYPE (tmpl1), TREE_TYPE (tmpl2))
+	      : tmpl1 != tmpl2)
+	    return false;
+	}
       /* Constrained 'auto's are distinct from parms that don't have the same
 	 constraints.  */
       if (!equivalent_placeholder_constraints (t1, t2))
diff --git a/gcc/testsuite/g++.dg/template/ttp42.C b/gcc/testsuite/g++.dg/template/ttp42.C
new file mode 100644
index 00000000000..da08e857fc5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ttp42.C
@@ -0,0 +1,14 @@ 
+// PR c++/112737
+// { dg-do compile { target c++17 } }
+
+template<template<class> class TT>
+decltype(TT{42}) f(); // #1
+
+template<template<class> class TT>
+decltype(TT{42}) f(); // redeclaration of #1
+
+template<class T> struct A { A(T); };
+
+int main() {
+  f<A>();
+}