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

[Jason Merrill <jason@redhat.com>]

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