| Message ID | 20200918200726.2123779-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: Fix self-mapping in map_arguments [PR96531, PR97103] | expand |
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? > + } > + } > > 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>(); > +} >
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>(); > > +} > > > >
On Fri, 18 Sep 2020, Patrick Palka wrote: > 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). Oops, 'p' is of course a TREE_LIST of a TEMPLATE_PARM_P node. But template_parm_to_arg wants a TREE_LIST of a DECL_TEMPLATE_PARM_P node. > > Would it be appropriate to adjust template_parm_to_arg to accept the > latter inputs as well? This version extends template_parm_to_arg to handle TEMPLATE_PARM_P nodes alongside its existing DECL_TEMPLATE_PARM_P handling, which allows us to simply pass 'p' to template_parm_to_arg from map_arguments. Bootstrapped and regtested on x86_64-pc-linux-gnu and tested on cmcstl2 and range-v3. -- >8 -- Subject: [PATCH] c++: Fix self-mapping in map_arguments [PR96531, PR97103] With r10-8077 we stopped passing the argified current_template_parms to normalize_constraint_expression from finish_nested_requirement, and instead made map_arguments perform a self-mapping of parameters when args is NULL. But 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 properly handle self-mapping of a parameter pack, this patch extends template_parm_to_arg to handle TEMPLATE_PARM_P nodes, and makes map_arguments use it. This change revealed that the call to template_parm_to_arg in convert_generic_types_to_packs is a no-op because 't' is never a TREE_LIST, so this patch additionally removes this call. As for bound ttps, map_arguments before r10-8077 would map a BOUND_TEMPLATE_TEMPLATE_PARM not to itself but to its underlying TEMPLATE_TEMPLATE_PARM. We could restore this behavior in map_arguments, but since a bound ttp is not really a template parameter it seems better to make keep_template_parm not give us a bound ttp in the first place. So this patch makes keep_template_parm return the underlying ttp instead of the bound ttp itself. gcc/cp/ChangeLog: PR c++/96531 PR c++/97103 * constraint.cc (map_arguments): Call template_parm_to_arg in the self-mapping case. (finish_shorthand_constraint): No need to build a TREE_LIST before calling template_parm_to_arg. * pt.c (template_parm_to_arg): Rewrite to handle TEMPLATE_PARM_P nodes as well as DECL_TEMPLATE_PARM_P nodes, and to make the overlying TREE_LIST node optional. (keep_template_parm): Don't record a BOUND_TEMPLATE_TEMPLATE_PARM, instead record its corresponding TEMPLATE_TEMPLATE_PARM. (convert_generic_types_to_packs): Don't call template_parm_to_arg. 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 | 4 +- gcc/cp/pt.c | 43 ++++++++++++------- gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C | 11 +++++ .../g++.dg/cpp2a/concepts-variadic1.C | 28 ++++++++++++ 4 files changed, 68 insertions(+), 18 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..2827d3f91b3 100644 --- a/gcc/cp/constraint.cc +++ b/gcc/cp/constraint.cc @@ -554,7 +554,7 @@ map_arguments (tree parms, tree args) TREE_PURPOSE (p) = TMPL_ARG (args, level, index); } else - TREE_PURPOSE (p) = TREE_VALUE (p); + TREE_PURPOSE (p) = template_parm_to_arg (p); return parms; } @@ -1492,7 +1492,7 @@ finish_shorthand_constraint (tree decl, tree constr) /* Get the argument and overload used for the requirement and adjust it if we're going to expand later. */ - tree arg = template_parm_to_arg (build_tree_list (NULL_TREE, decl)); + tree arg = template_parm_to_arg (decl); if (apply_to_each_p && declared_pack_p) arg = PACK_EXPANSION_PATTERN (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg), 0)); diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index cfe5ff4a94f..93706543d93 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -4704,29 +4704,37 @@ end_template_decl (void) current_template_parms = TREE_CHAIN (current_template_parms); } -/* Takes a TREE_LIST representing a template parameter and convert it - into an argument suitable to be passed to the type substitution - functions. Note that If the TREE_LIST contains an error_mark - node, the returned argument is error_mark_node. */ +/* Takes a TEMPLATE_PARM_P or DECL_TEMPLATE_PARM_P node or a TREE_LIST + thereof, and converts it into an argument suitable to be passed to + the type substitution functions. Note that if the TREE_LIST contains + an error_mark node, the returned argument is error_mark_node. */ tree template_parm_to_arg (tree t) { - - if (t == NULL_TREE - || TREE_CODE (t) != TREE_LIST) + if (t == NULL_TREE) return t; - if (error_operand_p (TREE_VALUE (t))) - return error_mark_node; + if (TREE_CODE (t) == TREE_LIST) + t = TREE_VALUE (t); - t = TREE_VALUE (t); + if (error_operand_p (t)) + return error_mark_node; - if (TREE_CODE (t) == TYPE_DECL - || TREE_CODE (t) == TEMPLATE_DECL) + if (DECL_P (t) && DECL_TEMPLATE_PARM_P (t)) { - t = TREE_TYPE (t); + if (TREE_CODE (t) == TYPE_DECL + || TREE_CODE (t) == TEMPLATE_DECL) + t = TREE_TYPE (t); + else + t = DECL_INITIAL (t); + } + + gcc_assert (TEMPLATE_PARM_P (t)); + if (TREE_CODE (t) == TEMPLATE_TYPE_PARM + || TREE_CODE (t) == TEMPLATE_TEMPLATE_PARM) + { if (TEMPLATE_TYPE_PARAMETER_PACK (t)) { /* Turn this argument into a TYPE_ARGUMENT_PACK @@ -4743,8 +4751,6 @@ template_parm_to_arg (tree t) } else { - t = DECL_INITIAL (t); - if (TEMPLATE_PARM_PARAMETER_PACK (t)) { /* Turn this argument into a NONTYPE_ARGUMENT_PACK @@ -10539,6 +10545,11 @@ keep_template_parm (tree t, void* data) if (level > ftpi->max_depth) return 0; + if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM) + /* We want the underlying TEMPLATE_TEMPLATE_PARM, not the + BOUND_TEMPLATE_TEMPLATE_PARM itself. */ + 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. */ @@ -29448,7 +29459,7 @@ convert_generic_types_to_packs (tree parm, int start_idx, int end_idx) if (tree constr = TEMPLATE_PARM_CONSTRAINTS (node)) { tree id = unpack_concept_check (constr); - TREE_VEC_ELT (TREE_OPERAND (id, 1), 0) = template_parm_to_arg (t); + TREE_VEC_ELT (TREE_OPERAND (id, 1), 0) = t; tree fold = finish_left_unary_fold_expr (constr, TRUTH_ANDIF_EXPR); TEMPLATE_PARM_CONSTRAINTS (node) = fold; 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>(); +}
On 9/18/20 8:42 PM, Patrick Palka wrote: > On Fri, 18 Sep 2020, Patrick Palka wrote: > >> 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). > > Oops, 'p' is of course a TREE_LIST of a TEMPLATE_PARM_P node. > But template_parm_to_arg wants a TREE_LIST of a DECL_TEMPLATE_PARM_P > node. > >> >> Would it be appropriate to adjust template_parm_to_arg to accept the >> latter inputs as well? > > This version extends template_parm_to_arg to handle TEMPLATE_PARM_P > nodes alongside its existing DECL_TEMPLATE_PARM_P handling, which allows > us to simply pass 'p' to template_parm_to_arg from map_arguments. > > Bootstrapped and regtested on x86_64-pc-linux-gnu and tested on cmcstl2 > and range-v3. OK, thanks. > -- >8 -- > > Subject: [PATCH] c++: Fix self-mapping in map_arguments [PR96531, PR97103] > > With r10-8077 we stopped passing the argified current_template_parms to > normalize_constraint_expression from finish_nested_requirement, and > instead made map_arguments perform a self-mapping of parameters when > args is NULL. But 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 properly handle self-mapping of a parameter pack, this patch > extends template_parm_to_arg to handle TEMPLATE_PARM_P nodes, and > makes map_arguments use it. This change revealed that the call to > template_parm_to_arg in convert_generic_types_to_packs is a no-op > because 't' is never a TREE_LIST, so this patch additionally removes > this call. > > As for bound ttps, map_arguments before r10-8077 would map a > BOUND_TEMPLATE_TEMPLATE_PARM not to itself but to its underlying > TEMPLATE_TEMPLATE_PARM. We could restore this behavior in > map_arguments, but since a bound ttp is not really a template parameter > it seems better to make keep_template_parm not give us a bound ttp in > the first place. So this patch makes keep_template_parm return the > underlying ttp instead of the bound ttp itself. > > gcc/cp/ChangeLog: > > PR c++/96531 > PR c++/97103 > * constraint.cc (map_arguments): Call template_parm_to_arg > in the self-mapping case. > (finish_shorthand_constraint): No need to build a TREE_LIST > before calling template_parm_to_arg. > * pt.c (template_parm_to_arg): Rewrite to handle TEMPLATE_PARM_P > nodes as well as DECL_TEMPLATE_PARM_P nodes, and to make the > overlying TREE_LIST node optional. > (keep_template_parm): Don't record a BOUND_TEMPLATE_TEMPLATE_PARM, > instead record its corresponding TEMPLATE_TEMPLATE_PARM. > (convert_generic_types_to_packs): Don't call > template_parm_to_arg. > > 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 | 4 +- > gcc/cp/pt.c | 43 ++++++++++++------- > gcc/testsuite/g++.dg/cpp2a/concepts-ttp2.C | 11 +++++ > .../g++.dg/cpp2a/concepts-variadic1.C | 28 ++++++++++++ > 4 files changed, 68 insertions(+), 18 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..2827d3f91b3 100644 > --- a/gcc/cp/constraint.cc > +++ b/gcc/cp/constraint.cc > @@ -554,7 +554,7 @@ map_arguments (tree parms, tree args) > TREE_PURPOSE (p) = TMPL_ARG (args, level, index); > } > else > - TREE_PURPOSE (p) = TREE_VALUE (p); > + TREE_PURPOSE (p) = template_parm_to_arg (p); > > return parms; > } > @@ -1492,7 +1492,7 @@ finish_shorthand_constraint (tree decl, tree constr) > > /* Get the argument and overload used for the requirement > and adjust it if we're going to expand later. */ > - tree arg = template_parm_to_arg (build_tree_list (NULL_TREE, decl)); > + tree arg = template_parm_to_arg (decl); > if (apply_to_each_p && declared_pack_p) > arg = PACK_EXPANSION_PATTERN (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg), 0)); > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index cfe5ff4a94f..93706543d93 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -4704,29 +4704,37 @@ end_template_decl (void) > current_template_parms = TREE_CHAIN (current_template_parms); > } > > -/* Takes a TREE_LIST representing a template parameter and convert it > - into an argument suitable to be passed to the type substitution > - functions. Note that If the TREE_LIST contains an error_mark > - node, the returned argument is error_mark_node. */ > +/* Takes a TEMPLATE_PARM_P or DECL_TEMPLATE_PARM_P node or a TREE_LIST > + thereof, and converts it into an argument suitable to be passed to > + the type substitution functions. Note that if the TREE_LIST contains > + an error_mark node, the returned argument is error_mark_node. */ > > tree > template_parm_to_arg (tree t) > { > - > - if (t == NULL_TREE > - || TREE_CODE (t) != TREE_LIST) > + if (t == NULL_TREE) > return t; > > - if (error_operand_p (TREE_VALUE (t))) > - return error_mark_node; > + if (TREE_CODE (t) == TREE_LIST) > + t = TREE_VALUE (t); > > - t = TREE_VALUE (t); > + if (error_operand_p (t)) > + return error_mark_node; > > - if (TREE_CODE (t) == TYPE_DECL > - || TREE_CODE (t) == TEMPLATE_DECL) > + if (DECL_P (t) && DECL_TEMPLATE_PARM_P (t)) > { > - t = TREE_TYPE (t); > + if (TREE_CODE (t) == TYPE_DECL > + || TREE_CODE (t) == TEMPLATE_DECL) > + t = TREE_TYPE (t); > + else > + t = DECL_INITIAL (t); > + } > + > + gcc_assert (TEMPLATE_PARM_P (t)); > > + if (TREE_CODE (t) == TEMPLATE_TYPE_PARM > + || TREE_CODE (t) == TEMPLATE_TEMPLATE_PARM) > + { > if (TEMPLATE_TYPE_PARAMETER_PACK (t)) > { > /* Turn this argument into a TYPE_ARGUMENT_PACK > @@ -4743,8 +4751,6 @@ template_parm_to_arg (tree t) > } > else > { > - t = DECL_INITIAL (t); > - > if (TEMPLATE_PARM_PARAMETER_PACK (t)) > { > /* Turn this argument into a NONTYPE_ARGUMENT_PACK > @@ -10539,6 +10545,11 @@ keep_template_parm (tree t, void* data) > if (level > ftpi->max_depth) > return 0; > > + if (TREE_CODE (t) == BOUND_TEMPLATE_TEMPLATE_PARM) > + /* We want the underlying TEMPLATE_TEMPLATE_PARM, not the > + BOUND_TEMPLATE_TEMPLATE_PARM itself. */ > + 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. */ > @@ -29448,7 +29459,7 @@ convert_generic_types_to_packs (tree parm, int start_idx, int end_idx) > if (tree constr = TEMPLATE_PARM_CONSTRAINTS (node)) > { > tree id = unpack_concept_check (constr); > - TREE_VEC_ELT (TREE_OPERAND (id, 1), 0) = template_parm_to_arg (t); > + TREE_VEC_ELT (TREE_OPERAND (id, 1), 0) = t; > tree fold = finish_left_unary_fold_expr (constr, TRUTH_ANDIF_EXPR); > TEMPLATE_PARM_CONSTRAINTS (node) = fold; > > 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>(); > +} >
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))); + } + } 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>(); +}