[PATCH] c++: Return only in-scope tparms in keep_template_parm [PR95310]
Jason Merrill
jason@redhat.com
Tue Sep 22 20:06:06 GMT 2020
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);
>> +}
>> +
>>
>> 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