[PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]

[PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]

[Patrick Palka]

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

Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]

[Jason Merrill]

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" }
> 

Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]

[Patrick Palka]

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

Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]

[Patrick Palka]

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

Re: [PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]

[Jason Merrill]

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