public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: bound ttp level lowering [PR109531]
@ 2023-04-17 18:44 Patrick Palka
  2023-04-17 22:11 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2023-04-17 18:44 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, Patrick Palka

Here when level lowering the bound ttp TT<typename T::type> via the
substitution T=C, we're neglecting to canonicalize (and therefore strip
of simple typedefs) the resulting template arguments {A<int>} before
determining its new canonical type via hash table lookup.  This leads to
an a hash mismatch ICE for the two equivalent types TT<int> and TT<A<int>>
since iterative_hash_template_arg misbehaves for a non-canonicalized type
argument.

We can fix this by canonicalizing or coercing the substituted arguments
directly, but given that creation and ordinary substitution of bound
ttps both go through lookup_template_class, which in turn performs
coercion/canonicalization, it seems best to make this code path go
through lookup_template_class as well.

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

	PR c++/109531

gcc/cp/ChangeLog:

	* pt.cc (tsubst) <case BOUND_TEMPLATE_TEMPLATE_PARM>:
	In the level-lowering case just use lookup_template_class
	to rebuild the bound ttp.

gcc/testsuite/ChangeLog:

	* g++.dg/template/canon-type-20.C: New test.
	* g++.dg/template/ttp36.C: New test.
---
 gcc/cp/pt.cc                                  | 39 ++++++++++---------
 gcc/testsuite/g++.dg/template/canon-type-20.C | 18 +++++++++
 gcc/testsuite/g++.dg/template/ttp36.C         | 12 ++++++
 3 files changed, 50 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/template/canon-type-20.C
 create mode 100644 gcc/testsuite/g++.dg/template/ttp36.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index fcc8e0d1d57..e065ace5c55 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -16232,7 +16232,6 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	  {
 	  case TEMPLATE_TYPE_PARM:
 	  case TEMPLATE_TEMPLATE_PARM:
-	  case BOUND_TEMPLATE_TEMPLATE_PARM:
 	    if (cp_type_quals (t))
 	      {
 		r = tsubst (TYPE_MAIN_VARIANT (t), args, complain, in_decl);
@@ -16274,24 +16273,6 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 		       only instantiated during satisfaction.  */
 		    PLACEHOLDER_TYPE_CONSTRAINTS_INFO (r) = ci;
 
-		if (code == BOUND_TEMPLATE_TEMPLATE_PARM)
-		  {
-		    tree tinfo = TYPE_TEMPLATE_INFO (t);
-		    /* We might need to substitute into the types of non-type
-		       template parameters.  */
-		    tree tmpl = tsubst (TI_TEMPLATE (tinfo), args,
-					complain, in_decl);
-		    if (tmpl == error_mark_node)
-		      return error_mark_node;
-		    tree argvec = tsubst (TI_ARGS (tinfo), args,
-					  complain, in_decl);
-		    if (argvec == error_mark_node)
-		      return error_mark_node;
-
-		    TEMPLATE_TEMPLATE_PARM_TEMPLATE_INFO (r)
-		      = build_template_info (tmpl, argvec);
-		  }
-
 		if (TYPE_STRUCTURAL_EQUALITY_P (t))
 		  SET_TYPE_STRUCTURAL_EQUALITY (r);
 		else
@@ -16299,6 +16280,26 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 	      }
 	    break;
 
+	  case BOUND_TEMPLATE_TEMPLATE_PARM:
+	    {
+	      tree tinfo = TYPE_TEMPLATE_INFO (t);
+	      /* We might need to substitute into the types of non-type
+		 template parameters.  This also lowers the level of
+		 the ttp appropriately.  */
+	      tree tmpl = tsubst (TI_TEMPLATE (tinfo), args,
+				  complain, in_decl);
+	      if (tmpl == error_mark_node)
+		return error_mark_node;
+	      tree argvec = tsubst (TI_ARGS (tinfo), args,
+				    complain, in_decl);
+	      if (argvec == error_mark_node)
+		return error_mark_node;
+	      r = lookup_template_class (tmpl, argvec, in_decl, NULL_TREE,
+					 /*entering_scope=*/false, complain);
+	      r = cp_build_qualified_type (r, cp_type_quals (t), complain);
+	      break;
+	    }
+
 	  case TEMPLATE_PARM_INDEX:
 	    /* OK, now substitute the type of the non-type parameter.  We
 	       couldn't do it earlier because it might be an auto parameter,
diff --git a/gcc/testsuite/g++.dg/template/canon-type-20.C b/gcc/testsuite/g++.dg/template/canon-type-20.C
new file mode 100644
index 00000000000..e40261037e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/canon-type-20.C
@@ -0,0 +1,18 @@
+// PR c++/109531
+// { dg-do compile { target c++11 } }
+// { dg-additional-options "--param=hash-table-verification-limit=1000" }
+
+template<class T>
+using A = int;
+
+struct B { using type = int; };
+struct C { using type = A<int>; };
+
+template<class T>
+struct D {
+  template<template<class> class TT>
+  TT<typename T::type> f ();
+};
+
+template struct D<B>;
+template struct D<C>;
diff --git a/gcc/testsuite/g++.dg/template/ttp36.C b/gcc/testsuite/g++.dg/template/ttp36.C
new file mode 100644
index 00000000000..2df00855cf7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ttp36.C
@@ -0,0 +1,12 @@
+// Verify we propagate cv-quals when level-lowering a bound ttp.
+
+template<class T>
+struct B {
+  template<template<class> class TT>
+  void f(const TT<T> *);
+
+  template<template<class> class TT>
+  void f(TT<T> *); // { dg-bogus "cannot be overloaded" }
+};
+
+template struct B<int>;
-- 
2.40.0.335.g9857273be0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] c++: bound ttp level lowering [PR109531]
  2023-04-17 18:44 [PATCH] c++: bound ttp level lowering [PR109531] Patrick Palka
@ 2023-04-17 22:11 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2023-04-17 22:11 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 4/17/23 14:44, Patrick Palka wrote:
> Here when level lowering the bound ttp TT<typename T::type> via the
> substitution T=C, we're neglecting to canonicalize (and therefore strip
> of simple typedefs) the resulting template arguments {A<int>} before
> determining its new canonical type via hash table lookup.  This leads to
> an a hash mismatch ICE for the two equivalent types TT<int> and TT<A<int>>
> since iterative_hash_template_arg misbehaves for a non-canonicalized type
> argument.
> 
> We can fix this by canonicalizing or coercing the substituted arguments
> directly, but given that creation and ordinary substitution of bound
> ttps both go through lookup_template_class, which in turn performs
> coercion/canonicalization, it seems best to make this code path go
> through lookup_template_class as well.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk/13?

OK.

> 	PR c++/109531
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (tsubst) <case BOUND_TEMPLATE_TEMPLATE_PARM>:
> 	In the level-lowering case just use lookup_template_class
> 	to rebuild the bound ttp.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/canon-type-20.C: New test.
> 	* g++.dg/template/ttp36.C: New test.
> ---
>   gcc/cp/pt.cc                                  | 39 ++++++++++---------
>   gcc/testsuite/g++.dg/template/canon-type-20.C | 18 +++++++++
>   gcc/testsuite/g++.dg/template/ttp36.C         | 12 ++++++
>   3 files changed, 50 insertions(+), 19 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/template/canon-type-20.C
>   create mode 100644 gcc/testsuite/g++.dg/template/ttp36.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index fcc8e0d1d57..e065ace5c55 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -16232,7 +16232,6 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   	  {
>   	  case TEMPLATE_TYPE_PARM:
>   	  case TEMPLATE_TEMPLATE_PARM:
> -	  case BOUND_TEMPLATE_TEMPLATE_PARM:
>   	    if (cp_type_quals (t))
>   	      {
>   		r = tsubst (TYPE_MAIN_VARIANT (t), args, complain, in_decl);
> @@ -16274,24 +16273,6 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   		       only instantiated during satisfaction.  */
>   		    PLACEHOLDER_TYPE_CONSTRAINTS_INFO (r) = ci;
>   
> -		if (code == BOUND_TEMPLATE_TEMPLATE_PARM)
> -		  {
> -		    tree tinfo = TYPE_TEMPLATE_INFO (t);
> -		    /* We might need to substitute into the types of non-type
> -		       template parameters.  */
> -		    tree tmpl = tsubst (TI_TEMPLATE (tinfo), args,
> -					complain, in_decl);
> -		    if (tmpl == error_mark_node)
> -		      return error_mark_node;
> -		    tree argvec = tsubst (TI_ARGS (tinfo), args,
> -					  complain, in_decl);
> -		    if (argvec == error_mark_node)
> -		      return error_mark_node;
> -
> -		    TEMPLATE_TEMPLATE_PARM_TEMPLATE_INFO (r)
> -		      = build_template_info (tmpl, argvec);
> -		  }
> -
>   		if (TYPE_STRUCTURAL_EQUALITY_P (t))
>   		  SET_TYPE_STRUCTURAL_EQUALITY (r);
>   		else
> @@ -16299,6 +16280,26 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   	      }
>   	    break;
>   
> +	  case BOUND_TEMPLATE_TEMPLATE_PARM:
> +	    {
> +	      tree tinfo = TYPE_TEMPLATE_INFO (t);
> +	      /* We might need to substitute into the types of non-type
> +		 template parameters.  This also lowers the level of
> +		 the ttp appropriately.  */
> +	      tree tmpl = tsubst (TI_TEMPLATE (tinfo), args,
> +				  complain, in_decl);
> +	      if (tmpl == error_mark_node)
> +		return error_mark_node;
> +	      tree argvec = tsubst (TI_ARGS (tinfo), args,
> +				    complain, in_decl);
> +	      if (argvec == error_mark_node)
> +		return error_mark_node;
> +	      r = lookup_template_class (tmpl, argvec, in_decl, NULL_TREE,
> +					 /*entering_scope=*/false, complain);
> +	      r = cp_build_qualified_type (r, cp_type_quals (t), complain);
> +	      break;
> +	    }
> +
>   	  case TEMPLATE_PARM_INDEX:
>   	    /* OK, now substitute the type of the non-type parameter.  We
>   	       couldn't do it earlier because it might be an auto parameter,
> diff --git a/gcc/testsuite/g++.dg/template/canon-type-20.C b/gcc/testsuite/g++.dg/template/canon-type-20.C
> new file mode 100644
> index 00000000000..e40261037e5
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/canon-type-20.C
> @@ -0,0 +1,18 @@
> +// PR c++/109531
> +// { dg-do compile { target c++11 } }
> +// { dg-additional-options "--param=hash-table-verification-limit=1000" }
> +
> +template<class T>
> +using A = int;
> +
> +struct B { using type = int; };
> +struct C { using type = A<int>; };
> +
> +template<class T>
> +struct D {
> +  template<template<class> class TT>
> +  TT<typename T::type> f ();
> +};
> +
> +template struct D<B>;
> +template struct D<C>;
> diff --git a/gcc/testsuite/g++.dg/template/ttp36.C b/gcc/testsuite/g++.dg/template/ttp36.C
> new file mode 100644
> index 00000000000..2df00855cf7
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp36.C
> @@ -0,0 +1,12 @@
> +// Verify we propagate cv-quals when level-lowering a bound ttp.
> +
> +template<class T>
> +struct B {
> +  template<template<class> class TT>
> +  void f(const TT<T> *);
> +
> +  template<template<class> class TT>
> +  void f(TT<T> *); // { dg-bogus "cannot be overloaded" }
> +};
> +
> +template struct B<int>;


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-04-17 22:11 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-04-17 18:44 [PATCH] c++: bound ttp level lowering [PR109531] Patrick Palka
2023-04-17 22:11 ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).