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

[Patrick Palka <ppalka@redhat.com>]

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?

Ping.

> 
> 	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 5330d1e1f62..f08e5630a5c 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7381,8 +7381,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);
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 1b28195e10d..1a32f10b22b 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1913,6 +1913,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;
>  	}
>  
> @@ -9296,6 +9301,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;
> +
>  /* Returns true if T is a wrapper to make a C++20 template parameter
>     object const.  */
>  
> @@ -9312,7 +9323,7 @@ class_nttp_const_wrapper_p (tree t)
>  /* Returns 1 if template args OT and NT are equivalent.  */
>  
>  int
> -template_args_equal (tree ot, tree nt, bool partial_order /* = false */)
> +template_args_equal (tree ot, tree nt)
>  {
>    if (nt == ot)
>      return 1;
> @@ -9335,7 +9346,7 @@ template_args_equal (tree ot, tree nt, bool partial_order /* = false */)
>       During partial ordering, however, we need to treat them normally so we can
>       order uses of the same alias with different cv-qualification (79960).  */
>    auto cso = make_temp_override (comparing_dependent_aliases);
> -  if (!partial_order)
> +  if (!comparing_for_partial_ordering)
>      ++comparing_dependent_aliases;
>  
>    if (TREE_CODE (nt) == TREE_VEC || TREE_CODE (ot) == TREE_VEC)
> @@ -9383,8 +9394,7 @@ template_args_equal (tree ot, tree nt, bool partial_order /* = false */)
>  
>  bool
>  comp_template_args (tree oldargs, tree newargs,
> -		    tree *oldarg_ptr /* = NULL */, tree *newarg_ptr /* = NULL */,
> -		    bool partial_order /* = false */)
> +		    tree *oldarg_ptr /* = NULL */, tree *newarg_ptr /* = NULL */)
>  {
>    if (oldargs == newargs)
>      return true;
> @@ -9400,7 +9410,7 @@ comp_template_args (tree oldargs, tree newargs,
>        tree nt = TREE_VEC_ELT (newargs, i);
>        tree ot = TREE_VEC_ELT (oldargs, i);
>  
> -      if (! template_args_equal (ot, nt, partial_order))
> +      if (! template_args_equal (ot, nt))
>  	{
>  	  if (oldarg_ptr != NULL)
>  	    *oldarg_ptr = ot;
> @@ -9412,10 +9422,13 @@ comp_template_args (tree oldargs, tree newargs,
>    return true;
>  }
>  
> -inline bool
> +static bool
>  comp_template_args_porder (tree oargs, tree nargs)
>  {
> -  return comp_template_args (oargs, nargs, NULL, NULL, true);
> +  ++comparing_for_partial_ordering;
> +  bool equal = comp_template_args (oargs, nargs);
> +  --comparing_for_partial_ordering;
> +  return equal;
>  }
>  
>  /* Implement a freelist interface for objects of type T.
> @@ -28839,6 +28852,19 @@ any_template_arguments_need_structural_equality_p (tree args)
>  		   mutated after the fact by duplicate_decls), so just require
>  		   structural equality in this case (PR52830).  */
>  		return true;
> +	      else if (TYPE_P (arg)
> +		       && TYPE_STRUCTURAL_EQUALITY_P (arg)
> +		       && dependent_alias_template_spec_p (arg, nt_transparent))
> +		/* Require structural equality for specializations written
> +		   in terms of a dependent alias template specialization.  */
> +		return true;
> +	      else if (CLASS_TYPE_P (arg)
> +		       && TYPE_TEMPLATE_INFO (arg)
> +		       && TYPE_STRUCTURAL_EQUALITY_P (arg))
> +		/* Require structural equality for specializations written
> +		   in terms of a class template specialization that itself
> +		   needs structural equality.  */
> +		return true;
>  	    }
>  	}
>      }
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 19dfb3ed782..775c0c9cd57 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -4136,7 +4136,7 @@ cp_tree_equal (tree t1, tree t2)
>      case TREE_VEC:
>        /* These are template args.  Really we should be getting the
>  	 caller to do this as it knows it to be true.  */
> -      if (!comp_template_args (t1, t2, NULL, NULL, false))
> +      if (!comp_template_args (t1, t2))
>  	return false;
>        return true;
>  
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-75a.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-75a.C
> new file mode 100644
> index 00000000000..8adbd6f65ac
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-75a.C
> @@ -0,0 +1,26 @@
> +// PR c++/90679
> +// A version of alias-decl-75.C where the specializations of the
> +// complex alias template first_t are dependent.
> +// { dg-do compile { target c++11 } }
> +
> +template<class T, class...>
> +using first_t = T;
> +
> +template<class T>
> +struct A;
> +
> +template<class T>
> +struct traits;
> +
> +template<class T>
> +struct traits<A<first_t<T, T&>>> {
> +  static constexpr int value = 1;
> +};
> +
> +template<class T>
> +struct traits<A<first_t<const T, T&>>> {
> +  static constexpr int value = 2;
> +};
> +
> +static_assert(traits<A<int>>::value == 1, "");
> +static_assert(traits<A<const int>>::value == 2, ""); // { dg-bogus "ambiguous" }
> diff --git a/gcc/testsuite/g++.dg/cpp0x/alias-decl-75b.C b/gcc/testsuite/g++.dg/cpp0x/alias-decl-75b.C
> new file mode 100644
> index 00000000000..b89ea5cad1e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/alias-decl-75b.C
> @@ -0,0 +1,26 @@
> +// PR c++/90679
> +// A version of alias-decl-75a.C where the alias template specialization
> +// appears as a more deeply nested template argument.
> +// { dg-do compile { target c++11 } }
> +
> +template<class T, class...>
> +using first_t = T;
> +
> +template<class T>
> +struct A;
> +
> +template<class T>
> +struct traits;
> +
> +template<class T>
> +struct traits<A<A<first_t<T, T&>>>> {
> +  static constexpr int value = 1;
> +};
> +
> +template<class T>
> +struct traits<A<A<first_t<const T, T&>>>> {
> +  static constexpr int value = 2;
> +};
> +
> +static_assert(traits<A<A<int>>>::value == 1, "");
> +static_assert(traits<A<A<const int>>>::value == 2, ""); // { dg-bogus "ambiguous" }
> -- 
> 2.41.0.rc1.10.g9e49351c30
> 
>