* [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] @ 2020-09-19 19:49 Patrick Palka 2020-09-21 21:42 ` Jason Merrill 0 siblings, 1 reply; 5+ messages in thread From: Patrick Palka @ 2020-09-19 19:49 UTC (permalink / raw) To: gcc-patches In the testcase below, the dependent specializations iter_reference_t<F> and iter_reference_t<Out> share the same tree due to specialization caching. So when find_template_parameters walks through the requires-expression (as part of normalization), it sees and includes the out-of-scope template parameter F in the list of template parameters it found within the requires-expression (along with Out and N). From a correctness perspective this is harmless since the parameter mapping routines only care about the level and index of each parameter, so F is no different from Out in this sense. (And it's also harmless that two parameters in the parameter mapping have the same level and index.) But having both Out and F in the parameter mapping is extra work for hash_atomic_constrant, tsubst_parameter_mapping and get_mapped_args; and it also means we print this irrelevant template parameter in the testcase's diagnostics (via pp_cxx_parameter_mapping): in requirements with ‘Out o’ [with N = (const int&)&a; F = const int*; Out = const int*] This patch makes keep_template_parm return only in-scope template parameters by looking into ctx_parms for the corresponding in-scope one. (That we sometimes print irrelevant template parameters in diagnostics is also the subject of PR99 and PR66968, so the above diagnostic issue could likely be fixed in a more general way, but this targeted fix to keep_template_parm is perhaps worthwhile on its own.) Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on cmcstl2 and range-v3. Does this look OK for trunk? gcc/cp/ChangeLog: PR c++/95310 * pt.c (keep_template_parm): Adjust the given template parameter to the corresponding in-scope one from ctx_parms. gcc/testsuite/ChangeLog: PR c++/95310 * g++.dg/concepts/diagnostic15.C: New test. * g++.dg/cpp2a/concepts-ttp2.C: New test. --- gcc/cp/pt.c | 19 +++++++++++++++++++ gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 ++++++++++++++++ 2 files changed, 35 insertions(+) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index fe45de8d796..c2c70ff02b9 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -10550,6 +10550,25 @@ keep_template_parm (tree t, void* data) BOUND_TEMPLATE_TEMPLATE_PARM itself. */ t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); + /* This template parameter might be an argument to a cached dependent + specalization that was formed earlier inside some other template, in which + case the parameter is not among the ones that are in-scope. Look in + CTX_PARMS to find the corresponding in-scope template parameter and + always return that instead. */ + tree cparms = ftpi->ctx_parms; + while (TMPL_PARMS_DEPTH (cparms) > level) + cparms = TREE_CHAIN (cparms); + gcc_assert (TMPL_PARMS_DEPTH (cparms) == level); + if (TREE_VEC_LENGTH (TREE_VALUE (cparms))) + { + t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (cparms), index)); + /* As in template_parm_to_arg. */ + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL) + t = TREE_TYPE (t); + else + t = DECL_INITIAL (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/concepts/diagnostic15.C b/gcc/testsuite/g++.dg/concepts/diagnostic15.C new file mode 100644 index 00000000000..3acd9f67968 --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic15.C @@ -0,0 +1,16 @@ +// PR c++/95310 +// { dg-do compile { target concepts } } + +template <class T> +using iter_reference_t = decltype(*T{}); + +template <typename F> +struct result { using type = iter_reference_t<F>; }; + +template <class Out, const int& N> +concept indirectly_writable = requires(Out o) { // { dg-bogus "F =" } + iter_reference_t<Out>(*o) = N; +}; + +const int a = 0; +static_assert(indirectly_writable<const int*, a>); // { dg-error "assert" } -- 2.28.0.497.g54e85e7af1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] 2020-09-19 19:49 [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] Patrick Palka @ 2020-09-21 21:42 ` Jason Merrill 2020-09-22 18:28 ` Patrick Palka 0 siblings, 1 reply; 5+ messages in thread From: Jason Merrill @ 2020-09-21 21:42 UTC (permalink / raw) To: Patrick Palka, gcc-patches On 9/19/20 3:49 PM, Patrick Palka wrote: > In the testcase below, the dependent specializations iter_reference_t<F> > and iter_reference_t<Out> share the same tree due to specialization > caching. So when find_template_parameters walks through the > requires-expression (as part of normalization), it sees and includes the > out-of-scope template parameter F in the list of template parameters > it found within the requires-expression (along with Out and N). > > From a correctness perspective this is harmless since the parameter mapping > routines only care about the level and index of each parameter, so F is > no different from Out in this sense. (And it's also harmless that two > parameters in the parameter mapping have the same level and index.) > > But having both Out and F in the parameter mapping is extra work for > hash_atomic_constrant, tsubst_parameter_mapping and get_mapped_args; and > it also means we print this irrelevant template parameter in the > testcase's diagnostics (via pp_cxx_parameter_mapping): > > in requirements with ‘Out o’ [with N = (const int&)&a; F = const int*; Out = const int*] > > This patch makes keep_template_parm return only in-scope template > parameters by looking into ctx_parms for the corresponding in-scope one. > > (That we sometimes print irrelevant template parameters in diagnostics is > also the subject of PR99 and PR66968, so the above diagnostic issue > could likely be fixed in a more general way, but this targeted fix to > keep_template_parm is perhaps worthwhile on its own.) > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on > cmcstl2 and range-v3. Does this look OK for trunk? > > gcc/cp/ChangeLog: > > PR c++/95310 > * pt.c (keep_template_parm): Adjust the given template parameter > to the corresponding in-scope one from ctx_parms. > > gcc/testsuite/ChangeLog: > > PR c++/95310 > * g++.dg/concepts/diagnostic15.C: New test. > * g++.dg/cpp2a/concepts-ttp2.C: New test. > --- > gcc/cp/pt.c | 19 +++++++++++++++++++ > gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 ++++++++++++++++ > 2 files changed, 35 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index fe45de8d796..c2c70ff02b9 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -10550,6 +10550,25 @@ keep_template_parm (tree t, void* data) > BOUND_TEMPLATE_TEMPLATE_PARM itself. */ > t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); > > + /* This template parameter might be an argument to a cached dependent > + specalization that was formed earlier inside some other template, in which > + case the parameter is not among the ones that are in-scope. Look in > + CTX_PARMS to find the corresponding in-scope template parameter and > + always return that instead. */ > + tree cparms = ftpi->ctx_parms; > + while (TMPL_PARMS_DEPTH (cparms) > level) > + cparms = TREE_CHAIN (cparms); > + gcc_assert (TMPL_PARMS_DEPTH (cparms) == level); > + if (TREE_VEC_LENGTH (TREE_VALUE (cparms))) > + { > + t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (cparms), index)); > + /* As in template_parm_to_arg. */ > + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL) > + t = TREE_TYPE (t); > + else > + t = DECL_INITIAL (t); > + } This seems like a useful separate function: given a parmlist and a single template parm (or index+level), return the corresponding parm from the parmlist. Basically the reverse of canonical_type_parameter. Jason > /* 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/concepts/diagnostic15.C b/gcc/testsuite/g++.dg/concepts/diagnostic15.C > new file mode 100644 > index 00000000000..3acd9f67968 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic15.C > @@ -0,0 +1,16 @@ > +// PR c++/95310 > +// { dg-do compile { target concepts } } > + > +template <class T> > +using iter_reference_t = decltype(*T{}); > + > +template <typename F> > +struct result { using type = iter_reference_t<F>; }; > + > +template <class Out, const int& N> > +concept indirectly_writable = requires(Out o) { // { dg-bogus "F =" } > + iter_reference_t<Out>(*o) = N; > +}; > + > +const int a = 0; > +static_assert(indirectly_writable<const int*, a>); // { dg-error "assert" } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] 2020-09-21 21:42 ` Jason Merrill @ 2020-09-22 18:28 ` Patrick Palka 2020-09-22 18:41 ` Patrick Palka 0 siblings, 1 reply; 5+ messages in thread From: Patrick Palka @ 2020-09-22 18:28 UTC (permalink / raw) To: Jason Merrill; +Cc: Patrick Palka, gcc-patches On Mon, 21 Sep 2020, Jason Merrill wrote: > On 9/19/20 3:49 PM, Patrick Palka wrote: > > In the testcase below, the dependent specializations iter_reference_t<F> > > and iter_reference_t<Out> share the same tree due to specialization > > caching. So when find_template_parameters walks through the > > requires-expression (as part of normalization), it sees and includes the > > out-of-scope template parameter F in the list of template parameters > > it found within the requires-expression (along with Out and N). > > > > From a correctness perspective this is harmless since the parameter mapping > > routines only care about the level and index of each parameter, so F is > > no different from Out in this sense. (And it's also harmless that two > > parameters in the parameter mapping have the same level and index.) > > > > But having both Out and F in the parameter mapping is extra work for > > hash_atomic_constrant, tsubst_parameter_mapping and get_mapped_args; and > > it also means we print this irrelevant template parameter in the > > testcase's diagnostics (via pp_cxx_parameter_mapping): > > > > in requirements with ‘Out o’ [with N = (const int&)&a; F = const int*; > > Out = const int*] > > > > This patch makes keep_template_parm return only in-scope template > > parameters by looking into ctx_parms for the corresponding in-scope one. > > > > (That we sometimes print irrelevant template parameters in diagnostics is > > also the subject of PR99 and PR66968, so the above diagnostic issue > > could likely be fixed in a more general way, but this targeted fix to > > keep_template_parm is perhaps worthwhile on its own.) > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on > > cmcstl2 and range-v3. Does this look OK for trunk? > > > > gcc/cp/ChangeLog: > > > > PR c++/95310 > > * pt.c (keep_template_parm): Adjust the given template parameter > > to the corresponding in-scope one from ctx_parms. > > > > gcc/testsuite/ChangeLog: > > > > PR c++/95310 > > * g++.dg/concepts/diagnostic15.C: New test. > > * g++.dg/cpp2a/concepts-ttp2.C: New test. > > --- > > gcc/cp/pt.c | 19 +++++++++++++++++++ > > gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 ++++++++++++++++ > > 2 files changed, 35 insertions(+) > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > index fe45de8d796..c2c70ff02b9 100644 > > --- a/gcc/cp/pt.c > > +++ b/gcc/cp/pt.c > > @@ -10550,6 +10550,25 @@ keep_template_parm (tree t, void* data) > > BOUND_TEMPLATE_TEMPLATE_PARM itself. */ > > t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); > > + /* This template parameter might be an argument to a cached dependent > > + specalization that was formed earlier inside some other template, in > > which > > + case the parameter is not among the ones that are in-scope. Look in > > + CTX_PARMS to find the corresponding in-scope template parameter and > > + always return that instead. */ > > + tree cparms = ftpi->ctx_parms; > > + while (TMPL_PARMS_DEPTH (cparms) > level) > > + cparms = TREE_CHAIN (cparms); > > + gcc_assert (TMPL_PARMS_DEPTH (cparms) == level); > > + if (TREE_VEC_LENGTH (TREE_VALUE (cparms))) > > + { > > + t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (cparms), index)); > > + /* As in template_parm_to_arg. */ > > + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL) > > + t = TREE_TYPE (t); > > + else > > + t = DECL_INITIAL (t); > > + } > > This seems like a useful separate function: given a parmlist and a single > template parm (or index+level), return the corresponding parm from the > parmlist. Basically the reverse of canonical_type_parameter. Sounds good. Like this? -- >8 -- gcc/cp/ChangeLog: PR c++/95310 * pt.c (corresponding_template_parameter): Define. (keep_template_parm): Use it to adjust the given template parameter to the corresponding in-scope one from ctx_parms. gcc/testsuite/ChangeLog: PR c++/95310 * g++.dg/concepts/diagnostic15.C: New test. * g++.dg/cpp2a/concepts-ttp2.C: New test. --- gcc/cp/pt.c | 44 ++++++++++++++++++++ gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 +++++++ 2 files changed, 60 insertions(+) create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c index 44ca14afc4e..bec8396f9f4 100644 --- a/gcc/cp/pt.c +++ b/gcc/cp/pt.c @@ -10244,6 +10244,42 @@ lookup_and_finish_template_variable (tree templ, tree targs, return convert_from_reference (templ); } +/* If the set of template parameters PARMS contains a template with + the given LEVEL and INDEX, then return this parameter. Otherwise + return NULL_TREE. */ + +static tree +corresponding_template_parameter (tree parms, int level, int index) +{ + while (TMPL_PARMS_DEPTH (parms) > level) + parms = TREE_CHAIN (parms); + + if (TMPL_PARMS_DEPTH (parms) != level + || TREE_VEC_LENGTH (TREE_VALUE (parms)) <= index) + return NULL_TREE; + + tree t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (parms), index)); + /* As in template_parm_to_arg. */ + 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)); + return t; +} + +/* Return the template parameter from PARMS that positionally corresponds + to the template parameter PARM, or else return NULL_TREE. */ + +static tree +corresponding_template_parameter (tree parms, tree parm) +{ + int level, index; + template_parm_level_and_index (parm, &level, &index); + return corresponding_template_parameter (parms, level, index); +} + \f struct pair_fn_data { @@ -10550,6 +10586,14 @@ keep_template_parm (tree t, void* data) BOUND_TEMPLATE_TEMPLATE_PARM itself. */ t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); + /* This template parameter might be an argument to a cached dependent + specalization that was formed earlier inside some other template, in + which case the parameter is not among the ones that are in-scope. + Look in CTX_PARMS to find the corresponding in-scope template + parameter and use it instead. */ + if (tree in_scope = corresponding_template_parameter (ftpi->ctx_parms, t)) + t = in_scope; + /* 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/concepts/diagnostic15.C b/gcc/testsuite/g++.dg/concepts/diagnostic15.C new file mode 100644 index 00000000000..3acd9f67968 --- /dev/null +++ b/gcc/testsuite/g++.dg/concepts/diagnostic15.C @@ -0,0 +1,16 @@ +// PR c++/95310 +// { dg-do compile { target concepts } } + +template <class T> +using iter_reference_t = decltype(*T{}); + +template <typename F> +struct result { using type = iter_reference_t<F>; }; + +template <class Out, const int& N> +concept indirectly_writable = requires(Out o) { // { dg-bogus "F =" } + iter_reference_t<Out>(*o) = N; +}; + +const int a = 0; +static_assert(indirectly_writable<const int*, a>); // { dg-error "assert" } -- 2.28.0.497.g54e85e7af1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] 2020-09-22 18:28 ` Patrick Palka @ 2020-09-22 18:41 ` Patrick Palka 2020-09-22 20:06 ` Jason Merrill 0 siblings, 1 reply; 5+ messages in thread From: Patrick Palka @ 2020-09-22 18:41 UTC (permalink / raw) To: Patrick Palka; +Cc: Jason Merrill, gcc-patches On Tue, 22 Sep 2020, Patrick Palka wrote: > On Mon, 21 Sep 2020, Jason Merrill wrote: > > > On 9/19/20 3:49 PM, Patrick Palka wrote: > > > In the testcase below, the dependent specializations iter_reference_t<F> > > > and iter_reference_t<Out> share the same tree due to specialization > > > caching. So when find_template_parameters walks through the > > > requires-expression (as part of normalization), it sees and includes the > > > out-of-scope template parameter F in the list of template parameters > > > it found within the requires-expression (along with Out and N). > > > > > > From a correctness perspective this is harmless since the parameter mapping > > > routines only care about the level and index of each parameter, so F is > > > no different from Out in this sense. (And it's also harmless that two > > > parameters in the parameter mapping have the same level and index.) > > > > > > But having both Out and F in the parameter mapping is extra work for > > > hash_atomic_constrant, tsubst_parameter_mapping and get_mapped_args; and > > > it also means we print this irrelevant template parameter in the > > > testcase's diagnostics (via pp_cxx_parameter_mapping): > > > > > > in requirements with ‘Out o’ [with N = (const int&)&a; F = const int*; > > > Out = const int*] > > > > > > This patch makes keep_template_parm return only in-scope template > > > parameters by looking into ctx_parms for the corresponding in-scope one. > > > > > > (That we sometimes print irrelevant template parameters in diagnostics is > > > also the subject of PR99 and PR66968, so the above diagnostic issue > > > could likely be fixed in a more general way, but this targeted fix to > > > keep_template_parm is perhaps worthwhile on its own.) > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on > > > cmcstl2 and range-v3. Does this look OK for trunk? > > > > > > gcc/cp/ChangeLog: > > > > > > PR c++/95310 > > > * pt.c (keep_template_parm): Adjust the given template parameter > > > to the corresponding in-scope one from ctx_parms. > > > > > > gcc/testsuite/ChangeLog: > > > > > > PR c++/95310 > > > * g++.dg/concepts/diagnostic15.C: New test. > > > * g++.dg/cpp2a/concepts-ttp2.C: New test. > > > --- > > > gcc/cp/pt.c | 19 +++++++++++++++++++ > > > gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 ++++++++++++++++ > > > 2 files changed, 35 insertions(+) > > > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C > > > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > > > index fe45de8d796..c2c70ff02b9 100644 > > > --- a/gcc/cp/pt.c > > > +++ b/gcc/cp/pt.c > > > @@ -10550,6 +10550,25 @@ keep_template_parm (tree t, void* data) > > > BOUND_TEMPLATE_TEMPLATE_PARM itself. */ > > > t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); > > > + /* This template parameter might be an argument to a cached dependent > > > + specalization that was formed earlier inside some other template, in > > > which > > > + case the parameter is not among the ones that are in-scope. Look in > > > + CTX_PARMS to find the corresponding in-scope template parameter and > > > + always return that instead. */ > > > + tree cparms = ftpi->ctx_parms; > > > + while (TMPL_PARMS_DEPTH (cparms) > level) > > > + cparms = TREE_CHAIN (cparms); > > > + gcc_assert (TMPL_PARMS_DEPTH (cparms) == level); > > > + if (TREE_VEC_LENGTH (TREE_VALUE (cparms))) > > > + { > > > + t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (cparms), index)); > > > + /* As in template_parm_to_arg. */ > > > + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL) > > > + t = TREE_TYPE (t); > > > + else > > > + t = DECL_INITIAL (t); > > > + } > > > > This seems like a useful separate function: given a parmlist and a single > > template parm (or index+level), return the corresponding parm from the > > parmlist. Basically the reverse of canonical_type_parameter. > > Sounds good. Like this? > > -- >8 -- > > gcc/cp/ChangeLog: > > PR c++/95310 > * pt.c (corresponding_template_parameter): Define. > (keep_template_parm): Use it to adjust the given template > parameter to the corresponding in-scope one from ctx_parms. > > gcc/testsuite/ChangeLog: > > PR c++/95310 > * g++.dg/concepts/diagnostic15.C: New test. > * g++.dg/cpp2a/concepts-ttp2.C: New test. Whoops, consider this stray ChangeLog line removed. diagnostic15.C is the only new test. > --- > gcc/cp/pt.c | 44 ++++++++++++++++++++ > gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 +++++++ > 2 files changed, 60 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c > index 44ca14afc4e..bec8396f9f4 100644 > --- a/gcc/cp/pt.c > +++ b/gcc/cp/pt.c > @@ -10244,6 +10244,42 @@ lookup_and_finish_template_variable (tree templ, tree targs, > return convert_from_reference (templ); > } > > +/* If the set of template parameters PARMS contains a template with s/template with/template parameter at/ > + the given LEVEL and INDEX, then return this parameter. Otherwise > + return NULL_TREE. */ > + > +static tree > +corresponding_template_parameter (tree parms, int level, int index) > +{ > + while (TMPL_PARMS_DEPTH (parms) > level) > + parms = TREE_CHAIN (parms); > + > + if (TMPL_PARMS_DEPTH (parms) != level > + || TREE_VEC_LENGTH (TREE_VALUE (parms)) <= index) > + return NULL_TREE; > + > + tree t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (parms), index)); > + /* As in template_parm_to_arg. */ > + 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)); > + return t; > +} > + > +/* Return the template parameter from PARMS that positionally corresponds > + to the template parameter PARM, or else return NULL_TREE. */ > + > +static tree > +corresponding_template_parameter (tree parms, tree parm) > +{ > + int level, index; > + template_parm_level_and_index (parm, &level, &index); > + return corresponding_template_parameter (parms, level, index); > +} > + > \f > struct pair_fn_data > { > @@ -10550,6 +10586,14 @@ keep_template_parm (tree t, void* data) > BOUND_TEMPLATE_TEMPLATE_PARM itself. */ > t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); > > + /* This template parameter might be an argument to a cached dependent > + specalization that was formed earlier inside some other template, in > + which case the parameter is not among the ones that are in-scope. > + Look in CTX_PARMS to find the corresponding in-scope template > + parameter and use it instead. */ > + if (tree in_scope = corresponding_template_parameter (ftpi->ctx_parms, t)) > + t = in_scope; > + > /* 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/concepts/diagnostic15.C b/gcc/testsuite/g++.dg/concepts/diagnostic15.C > new file mode 100644 > index 00000000000..3acd9f67968 > --- /dev/null > +++ b/gcc/testsuite/g++.dg/concepts/diagnostic15.C > @@ -0,0 +1,16 @@ > +// PR c++/95310 > +// { dg-do compile { target concepts } } > + > +template <class T> > +using iter_reference_t = decltype(*T{}); > + > +template <typename F> > +struct result { using type = iter_reference_t<F>; }; > + > +template <class Out, const int& N> > +concept indirectly_writable = requires(Out o) { // { dg-bogus "F =" } > + iter_reference_t<Out>(*o) = N; > +}; > + > +const int a = 0; > +static_assert(indirectly_writable<const int*, a>); // { dg-error "assert" } > -- > 2.28.0.497.g54e85e7af1 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] 2020-09-22 18:41 ` Patrick Palka @ 2020-09-22 20:06 ` Jason Merrill 0 siblings, 0 replies; 5+ messages in thread From: Jason Merrill @ 2020-09-22 20:06 UTC (permalink / raw) To: Patrick Palka; +Cc: gcc-patches On 9/22/20 2:41 PM, Patrick Palka wrote: > On Tue, 22 Sep 2020, Patrick Palka wrote: > >> On Mon, 21 Sep 2020, Jason Merrill wrote: >> >>> On 9/19/20 3:49 PM, Patrick Palka wrote: >>>> In the testcase below, the dependent specializations iter_reference_t<F> >>>> and iter_reference_t<Out> share the same tree due to specialization >>>> caching. So when find_template_parameters walks through the >>>> requires-expression (as part of normalization), it sees and includes the >>>> out-of-scope template parameter F in the list of template parameters >>>> it found within the requires-expression (along with Out and N). >>>> >>>> From a correctness perspective this is harmless since the parameter mapping >>>> routines only care about the level and index of each parameter, so F is >>>> no different from Out in this sense. (And it's also harmless that two >>>> parameters in the parameter mapping have the same level and index.) >>>> >>>> But having both Out and F in the parameter mapping is extra work for >>>> hash_atomic_constrant, tsubst_parameter_mapping and get_mapped_args; and >>>> it also means we print this irrelevant template parameter in the >>>> testcase's diagnostics (via pp_cxx_parameter_mapping): >>>> >>>> in requirements with ‘Out o’ [with N = (const int&)&a; F = const int*; >>>> Out = const int*] >>>> >>>> This patch makes keep_template_parm return only in-scope template >>>> parameters by looking into ctx_parms for the corresponding in-scope one. >>>> >>>> (That we sometimes print irrelevant template parameters in diagnostics is >>>> also the subject of PR99 and PR66968, so the above diagnostic issue >>>> could likely be fixed in a more general way, but this targeted fix to >>>> keep_template_parm is perhaps worthwhile on its own.) >>>> >>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, and also tested on >>>> cmcstl2 and range-v3. Does this look OK for trunk? >>>> >>>> gcc/cp/ChangeLog: >>>> >>>> PR c++/95310 >>>> * pt.c (keep_template_parm): Adjust the given template parameter >>>> to the corresponding in-scope one from ctx_parms. >>>> >>>> gcc/testsuite/ChangeLog: >>>> >>>> PR c++/95310 >>>> * g++.dg/concepts/diagnostic15.C: New test. >>>> * g++.dg/cpp2a/concepts-ttp2.C: New test. >>>> --- >>>> gcc/cp/pt.c | 19 +++++++++++++++++++ >>>> gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 ++++++++++++++++ >>>> 2 files changed, 35 insertions(+) >>>> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C >>>> >>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c >>>> index fe45de8d796..c2c70ff02b9 100644 >>>> --- a/gcc/cp/pt.c >>>> +++ b/gcc/cp/pt.c >>>> @@ -10550,6 +10550,25 @@ keep_template_parm (tree t, void* data) >>>> BOUND_TEMPLATE_TEMPLATE_PARM itself. */ >>>> t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); >>>> + /* This template parameter might be an argument to a cached dependent >>>> + specalization that was formed earlier inside some other template, in >>>> which >>>> + case the parameter is not among the ones that are in-scope. Look in >>>> + CTX_PARMS to find the corresponding in-scope template parameter and >>>> + always return that instead. */ >>>> + tree cparms = ftpi->ctx_parms; >>>> + while (TMPL_PARMS_DEPTH (cparms) > level) >>>> + cparms = TREE_CHAIN (cparms); >>>> + gcc_assert (TMPL_PARMS_DEPTH (cparms) == level); >>>> + if (TREE_VEC_LENGTH (TREE_VALUE (cparms))) >>>> + { >>>> + t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (cparms), index)); >>>> + /* As in template_parm_to_arg. */ >>>> + if (TREE_CODE (t) == TYPE_DECL || TREE_CODE (t) == TEMPLATE_DECL) >>>> + t = TREE_TYPE (t); >>>> + else >>>> + t = DECL_INITIAL (t); >>>> + } >>> >>> This seems like a useful separate function: given a parmlist and a single >>> template parm (or index+level), return the corresponding parm from the >>> parmlist. Basically the reverse of canonical_type_parameter. >> >> Sounds good. Like this? >> >> -- >8 -- >> >> gcc/cp/ChangeLog: >> >> PR c++/95310 >> * pt.c (corresponding_template_parameter): Define. >> (keep_template_parm): Use it to adjust the given template >> parameter to the corresponding in-scope one from ctx_parms. >> >> gcc/testsuite/ChangeLog: >> >> PR c++/95310 >> * g++.dg/concepts/diagnostic15.C: New test. >> * g++.dg/cpp2a/concepts-ttp2.C: New test. > > Whoops, consider this stray ChangeLog line removed. diagnostic15.C is > the only new test. OK. >> --- >> gcc/cp/pt.c | 44 ++++++++++++++++++++ >> gcc/testsuite/g++.dg/concepts/diagnostic15.C | 16 +++++++ >> 2 files changed, 60 insertions(+) >> create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic15.C >> >> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c >> index 44ca14afc4e..bec8396f9f4 100644 >> --- a/gcc/cp/pt.c >> +++ b/gcc/cp/pt.c >> @@ -10244,6 +10244,42 @@ lookup_and_finish_template_variable (tree templ, tree targs, >> return convert_from_reference (templ); >> } >> >> +/* If the set of template parameters PARMS contains a template with > > s/template with/template parameter at/ > >> + the given LEVEL and INDEX, then return this parameter. Otherwise >> + return NULL_TREE. */ >> + >> +static tree >> +corresponding_template_parameter (tree parms, int level, int index) >> +{ >> + while (TMPL_PARMS_DEPTH (parms) > level) >> + parms = TREE_CHAIN (parms); >> + >> + if (TMPL_PARMS_DEPTH (parms) != level >> + || TREE_VEC_LENGTH (TREE_VALUE (parms)) <= index) >> + return NULL_TREE; >> + >> + tree t = TREE_VALUE (TREE_VEC_ELT (TREE_VALUE (parms), index)); >> + /* As in template_parm_to_arg. */ >> + 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)); >> + return t; >> +} >> + >> +/* Return the template parameter from PARMS that positionally corresponds >> + to the template parameter PARM, or else return NULL_TREE. */ >> + >> +static tree >> +corresponding_template_parameter (tree parms, tree parm) >> +{ >> + int level, index; >> + template_parm_level_and_index (parm, &level, &index); >> + return corresponding_template_parameter (parms, level, index); >> +} >> + >> \f >> struct pair_fn_data >> { >> @@ -10550,6 +10586,14 @@ keep_template_parm (tree t, void* data) >> BOUND_TEMPLATE_TEMPLATE_PARM itself. */ >> t = TREE_TYPE (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t)); >> >> + /* This template parameter might be an argument to a cached dependent >> + specalization that was formed earlier inside some other template, in >> + which case the parameter is not among the ones that are in-scope. >> + Look in CTX_PARMS to find the corresponding in-scope template >> + parameter and use it instead. */ >> + if (tree in_scope = corresponding_template_parameter (ftpi->ctx_parms, t)) >> + t = in_scope; >> + >> /* 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/concepts/diagnostic15.C b/gcc/testsuite/g++.dg/concepts/diagnostic15.C >> new file mode 100644 >> index 00000000000..3acd9f67968 >> --- /dev/null >> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic15.C >> @@ -0,0 +1,16 @@ >> +// PR c++/95310 >> +// { dg-do compile { target concepts } } >> + >> +template <class T> >> +using iter_reference_t = decltype(*T{}); >> + >> +template <typename F> >> +struct result { using type = iter_reference_t<F>; }; >> + >> +template <class Out, const int& N> >> +concept indirectly_writable = requires(Out o) { // { dg-bogus "F =" } >> + iter_reference_t<Out>(*o) = N; >> +}; >> + >> +const int a = 0; >> +static_assert(indirectly_writable<const int*, a>); // { dg-error "assert" } >> -- >> 2.28.0.497.g54e85e7af1 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-22 20:06 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-09-19 19:49 [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310] Patrick Palka 2020-09-21 21:42 ` Jason Merrill 2020-09-22 18:28 ` Patrick Palka 2020-09-22 18:41 ` Patrick Palka 2020-09-22 20:06 ` 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).