Re: [PATCH] c++: ttp CTAD equivalence [PR112737]

[Patrick Palka <ppalka@redhat.com>]

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);
+}
-- 
2.43.0.493.gbc7ee2e5e1