[PATCH] c++: Fix self-mapping in map_arguments [PR96531, PR97103]

[Patrick Palka]

On Fri, 18 Sep 2020, Jason Merrill wrote:

> On 9/18/20 4:07 PM, Patrick Palka wrote:
> > With r10-8077 we stopped passing the argified current_template_parms to
> > normalize_constraint_expression from finish_nested_requirement, and
> > instead tweaked map_arguments to perform a self-mapping of parameters
> > when args is NULL.  We're currently not handling parameter packs and
> > BOUND_TEMPLATE_TEMPLATE_PARMs properly during this self-mapping, which
> > leads to ICEs later during satisfaction.
> > 
> > To fix the self-mapping of a parameter pack, this patch makes
> > map_arguments use template_parm_to_arg which already does the right
> > thing for parameter packs.
> > 
> > Before r10-8077, a BOUND_TEMPLATE_TEMPLATE_PARM would get mapped to the
> > corresponding TEMPLATE_TEMPLATE_PARM.  We could restore this behavior in
> > map_arguments, but since a BOUND_TEMPLATE_TEMPLATE_PARM is not really a
> > template parameter it seems better to make keep_template_parm not give
> > us a BOUND_TEMPLATE_TEMPLATE_PARM in the first place.  I think what we
> > actually want is to map the TEMPLATE_TEMPLATE_PARM to itself, so this
> > patch adjusts keep_template_parm to give us the corresponding
> > TEMPLATE_TEMPLATE_PARM of a BOUND_TEMPLATE_TEMPLATE_PARM instead.
> > 
> > Tested on x86_64-pc-linux-gnu, and also tested with the cmcstl2 library.
> > Does this look OK for trunk/10?
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	PR c++/96531
> > 	PR c++/97103
> > 	* constraint.cc (map_arguments): Call template_parm_to_arg
> > 	appropriately when doing a self-mapping.
> > 	* pt.c (keep_template_parm): Don't record a
> > 	BOUND_TEMPLATE_TEMPLATE_PARM, instead record its corresponding
> > 	TEMPLATE_TEMPLATE_PARM.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	PR c++/96531
> > 	PR c++/97103
> > 	* g++.dg/cpp2a/concepts-ttp2.C: New test.
> > 	* g++.dg/cpp2a/concepts-variadic1.C: New test.
> > ---
> >   gcc/cp/constraint.cc                          | 27 ++++++++++++------
> >   gcc/cp/pt.c                                   |  5 ++++
> >   gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C    | 11 ++++++++
> >   .../g++.dg/cpp2a/concepts-variadic1.C         | 28 +++++++++++++++++++
> >   4 files changed, 62 insertions(+), 9 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C
> > 
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index 0aab3073cc1..43336d191d9 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -546,15 +546,24 @@ static tree
> >   map_arguments (tree parms, tree args)
> >   {
> >     for (tree p = parms; p; p = TREE_CHAIN (p))
> > -    if (args)
> > -      {
> > -	int level;
> > -	int index;
> > -	template_parm_level_and_index (TREE_VALUE (p), &level, &index);
> > -	TREE_PURPOSE (p) = TMPL_ARG (args, level, index);
> > -      }
> > -    else
> > -      TREE_PURPOSE (p) = TREE_VALUE (p);
> > +    {
> > +      tree parm = TREE_VALUE (p);
> > +      if (args)
> > +	{
> > +	  int level;
> > +	  int index;
> > +	  template_parm_level_and_index (parm, &level, &index);
> > +	  TREE_PURPOSE (p) = TMPL_ARG (args, level, index);
> > +	}
> > +      else
> > +	{
> > +	  tree tpi = (TYPE_P (parm)
> > +		      ? TEMPLATE_TYPE_PARM_INDEX (parm) : parm);
> > +	  TREE_PURPOSE (p)
> > +	    = template_parm_to_arg (build_tree_list (NULL_TREE,
> > +						     TEMPLATE_PARM_DECL
> > (tpi)));
> 
> Doesn't passing 'p' to template_parm_to_arg work?

Unfortunately not, template_parm_to_arg expects a TREE_LIST node whose
TREE_VALUE is the corresponding *_DECL for the template parm (i.e. it
expects node from current_template_parms), and 'p' is a
TEMPLATE_TYPE_PARM, TEMPLATE_TEMPLATE PARM or TEMPLATE_PARM_INDEX (given
to us by find_template_parameters).

Would it be appropriate to adjust template_parm_to_arg to accept the
latter inputs as well?

> 
> > +	}
> > +    }
> >       return parms;
> >   }
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index cfe5ff4a94f..55d8060b911 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -10539,6 +10539,11 @@ keep_template_parm (tree t, void* data)
> >     if (level > ftpi->max_depth)
> >       return 0;
> >   +  if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM)
> > +    /* A BOUND_TEMPLATE_TEMPLATE_PARM isn't a template parameter.  What we
> > +       really want is the corresponding TEMPLATE_TEMPLATE_PARM.  */
> > +    t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t));
> > +
> >     /* Arguments like const T yield parameters like const T. This means that
> >        a template-id like X<T, const T> would yield two distinct parameters:
> >        T and const T. Adjust types to their unqualified versions.  */
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C
> > new file mode 100644
> > index 00000000000..7f4883754dd
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C
> > @@ -0,0 +1,11 @@
> > +// PR c++/97103
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<typename R, typename Rep>
> > +class quantity {};
> > +
> > +template<template<typename, typename> typename Q>
> > +inline constexpr bool valid_template_arguments = requires {
> > +  requires requires { typename Q<int, int>; };
> > +};
> > +static_assert(valid_template_arguments<quantity>);
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C
> > new file mode 100644
> > index 00000000000..deab028ca3c
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-variadic1.C
> > @@ -0,0 +1,28 @@
> > +// PR c++/96531
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<typename T>
> > +concept is_bool = __is_same(bool, T);
> > +
> > +template <typename... Ts>
> > +concept C = requires {
> > +  requires (is_bool<Ts> || ...);
> > +};
> > +
> > +template <bool... Bs>
> > +concept D = requires {
> > +  requires (Bs || ...);
> > +};
> > +
> > +template <typename... Ts>
> > +requires C<Ts...>
> > +void bar() {}
> > +
> > +template <bool... Bs>
> > +requires D<Bs...>
> > +void baz() {}
> > +
> > +int main() {
> > +  bar<int, char, bool>();
> > +  baz<false, true, false>();
> > +}
> > 
> 
>