[PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]
Patrick Palka
ppalka@redhat.com
Tue Sep 22 18:41:07 GMT 2020
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);
> +}
> +
>
> 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
More information about the Gcc-patches
mailing list