Re: [PATCH 2/2] c++: partial ordering and dep alias tmpl specs [PR90679]

[Jason Merrill <jason@redhat.com>]

On 12/15/23 14:07, Patrick Palka wrote:
> On Thu, 1 Jun 2023, Patrick Palka wrote:
> 
>> During partial ordering, we want to look through dependent alias
>> template specializations within template arguments and otherwise
>> treat them as opaque in other contexts (see e.g. r7-7116-g0c942f3edab108
>> and r11-7011-g6e0a231a4aa240).  To that end template_args_equal was
>> given a partial_order flag that controls this behavior.  This flag
>> does the right thing when a dependent alias template specialization
>> appears as template argument of the partial specialization, e.g. in
>>
>>    template<class T, class...> using first_t = T;
>>    template<class T> struct traits;
>>    template<class T> struct traits<first_t<T, T&>> { }; // #1
>>    template<class T> struct traits<first_t<const T, T&>> { }; // #2
>>
>> we correctly consider #2 to be more specialized than #1.  But if
>> the alias specialization appears as a template argument of another
>> class template specialization, e.g. in
>>
>>    template<class T> struct traits<A<first_t<T, T&>>> { }; // #1
>>    template<class T> struct traits<A<first_t<const T, T&>>> { }; // #2
>>
>> then we incorrectly consider #1 and #2 to be unordered.  This is because
>>
>>    1. we don't propagate the flag to recursive template_args_equal calls
>>    2. we don't use structural equality for class template specializations
>>       written in terms of dependent alias template specializations
>>
>> This patch fixes the first issue by turning the partial_order flag into
>> a global.  This patch fixes the second issue by making us propagate
>> structural equality appropriately when building a class template
>> specialization.  In passing this patch also improves hashing of
>> specializations that use structural equality.
>>
>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>> trunk?
>>
>> 	PR c++/90679
>>
>> gcc/cp/ChangeLog:
>>
>> 	* cp-tree.h (comp_template_args): Remove partial_order
>> 	parameter.
>> 	(template_args_equal): Likewise.
>> 	* pt.cc (iterative_hash_template_arg) <case tcc_type>: Hash
>> 	the template and arguments for specializations that use
>> 	structural equality.
>> 	(comparing_for_partial_ordering): New flag.
>> 	(template_args_equal): Remove partial order parameter and
>> 	use comparing_for_partial_ordering instead.
>> 	(comp_template_args): Likewise.
>> 	(comp_template_args_porder): Set comparing_for_partial_ordering
>> 	instead.  Make static.
>> 	(any_template_arguments_need_structural_equality_p): Return true
>> 	for an argument that's a dependent alias template specialization
>> 	or a class template specialization that itself needs structural
>> 	equality.
>> 	* tree.cc (cp_tree_equal) <case TREE_VEC>: Adjust call to
>> 	comp_template_args.
>>
>> gcc/testsuite/ChangeLog:
>>
>> 	* g++.dg/cpp0x/alias-decl-75a.C: New test.
>> 	* g++.dg/cpp0x/alias-decl-75b.C: New test.
> 
> Ping.  Here's the rebased patch:
> 
> -- >8 --
> 
> Subject: [PATCH 2/2] c++: partial ordering and dep alias tmpl specs [PR90679]
> 
> During partial ordering, we want to look through dependent alias
> template specializations within template arguments and otherwise
> treat them as opaque in other contexts (see e.g. r7-7116-g0c942f3edab108
> and r11-7011-g6e0a231a4aa240).  To that end template_args_equal was
> given a partial_order flag that controls this behavior.  This flag
> does the right thing when a dependent alias template specialization
> appears as template argument of the partial specialization, e.g. in
> 
>    template<class T, class...> using first_t = T;
>    template<class T> struct traits;
>    template<class T> struct traits<first_t<T, T&>> { }; // #1
>    template<class T> struct traits<first_t<const T, T&>> { }; // #2
> 
> we correctly consider #2 to be more specialized than #1.  But if
> the alias specialization appears as a template argument of another
> class template specialization, e.g. in
> 
>    template<class T> struct traits<A<first_t<T, T&>>> { }; // #1
>    template<class T> struct traits<A<first_t<const T, T&>>> { }; // #2
> 
> then we incorrectly consider #1 and #2 to be unordered.  This is because
> 
>    1. we don't propagate the flag to recursive template_args_equal calls
>    2. we don't use structural equality for class template specializations
>       written in terms of dependent alias template specializations
> 
> This patch fixes the first issue by turning the partial_order flag into
> a global.  This patch fixes the second issue by making us propagate
> structural equality appropriately when building a class template
> specialization.  In passing this patch also improves hashing of
> specializations that use structural equality.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/90679
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (comp_template_args): Remove partial_order
> 	parameter.
> 	(template_args_equal): Likewise.
> 	* pt.cc (iterative_hash_template_arg) <case tcc_type>: Hash
> 	the template and arguments for specializations that use
> 	structural equality.
> 	(comparing_for_partial_ordering): New flag.
> 	(template_args_equal): Remove partial order parameter and
> 	use comparing_for_partial_ordering instead.
> 	(comp_template_args): Likewise.
> 	(comp_template_args_porder): Set comparing_for_partial_ordering
> 	instead.  Make static.
> 	(any_template_arguments_need_structural_equality_p): Return true
> 	for an argument that's a dependent alias template specialization
> 	or a class template specialization that itself needs structural
> 	equality.
> 	* tree.cc (cp_tree_equal) <case TREE_VEC>: Adjust call to
> 	comp_template_args.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/alias-decl-75a.C: New test.
> 	* g++.dg/cpp0x/alias-decl-75b.C: New test.
> ---
>   gcc/cp/cp-tree.h                            |  4 +--
>   gcc/cp/pt.cc                                | 40 +++++++++++++++++----
>   gcc/cp/tree.cc                              |  2 +-
>   gcc/testsuite/g++.dg/cpp0x/alias-decl-75a.C | 26 ++++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/alias-decl-75b.C | 26 ++++++++++++++
>   5 files changed, 88 insertions(+), 10 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-75a.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/alias-decl-75b.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 85980c9ad9b..1979572c365 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7507,8 +7507,8 @@ extern int template_class_depth			(tree);
>   extern int is_specialization_of			(tree, tree);
>   extern bool is_specialization_of_friend		(tree, tree);
>   extern bool comp_template_args			(tree, tree, tree * = NULL,
> -						 tree * = NULL, bool = false);
> -extern int template_args_equal                  (tree, tree, bool = false);
> +						 tree * = NULL);
> +extern int template_args_equal                  (tree, tree);
>   extern tree maybe_process_partial_specialization (tree);
>   extern tree most_specialized_instantiation	(tree);
>   extern tree most_specialized_partial_spec       (tree, tsubst_flags_t, bool = false);
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index a3a79713236..6b0ef496dc5 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1894,6 +1894,11 @@ iterative_hash_template_arg (tree arg, hashval_t val)
>   	default:
>   	  if (tree canonical = TYPE_CANONICAL (arg))
>   	    val = iterative_hash_object (TYPE_HASH (canonical), val);
> +	  else if (tree ti = TYPE_TEMPLATE_INFO (arg))
> +	    {
> +	      val = iterative_hash_template_arg (TI_TEMPLATE (ti), val);
> +	      val = iterative_hash_template_arg (TI_ARGS (ti), val);
> +	    }
>   	  break;
>   	}
>   
> @@ -9306,6 +9311,12 @@ coerce_template_parms (tree parms,
>     return return_full_args ? new_args : new_inner_args;
>   }
>   
> +/* Whether we are comparing template arguments during partial ordering
> +   (and therefore want the comparison to look through dependent alias
> +   template specializations).  */
> +
> +static int comparing_for_partial_ordering;

How about putting this next to comparing_dependent_aliases?  OK with 
that change.

Jason