Re: [PATCH] c++: constraint matching, TEMPLATE_ID_EXPR, current inst

[Jason Merrill <jason@redhat.com>]

On 9/17/22 10:31, Patrick Palka wrote:
> On Sat, 17 Sep 2022, Jason Merrill wrote:
> 
>> On 9/16/22 10:59, Patrick Palka wrote:
>>> On Fri, 16 Sep 2022, Jason Merrill wrote:
>>>
>>>> On 9/15/22 11:58, Patrick Palka wrote:
>>>>> Here we're crashing during constraint matching for the instantiated
>>>>> hidden friends due to two issues with dependent substitution into a
>>>>> TEMPLATE_ID_EXPR naming a template from the current instantiation
>>>>> (as performed from maybe_substitute_reqs_for for C<3> with T=T):
>>>>>
>>>>>      * tsubst_copy substitutes into such a TEMPLATE_DECL by looking it
>>>>>        up from the substituted class scope.  But for this to not fail
>>>>> when
>>>>>        the args are dependent, we need to pass entering_scope=true for
>>>>> the
>>>>>        class scope substitution so that we obtain the primary template
>>>>> type
>>>>>        A<T> (which has TYPE_BINFO) instead of the implicit instantiation
>>>>>        A<T> (which doesn't).
>>>>>      * lookup_and_finish_template_variable shouldn't instantiate a
>>>>>        TEMPLATE_ID_EXPR that names a TEMPLATE_DECL which has more than
>>>>>        one level of (unsubstituted) parameters (such as A<T>::C).
>>>>>
>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
>>>>> trunk?
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 	* pt.cc (lookup_and_finish_template_variable): Don't
>>>>> 	instantiate if the template's scope is dependent.
>>>>> 	(tsubst_copy) <case TEMPLATE_DECL>: Pass entering_scope=true
>>>>> 	when substituting the class scope.
>>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 	* g++.dg/cpp2a/concepts-friend10.C: New test.
>>>>> ---
>>>>>     gcc/cp/pt.cc                                  | 14 +++++++------
>>>>>     .../g++.dg/cpp2a/concepts-friend10.C          | 21
>>>>> +++++++++++++++++++
>>>>>     2 files changed, 29 insertions(+), 6 deletions(-)
>>>>>     create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend10.C
>>>>>
>>>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>>>> index db4e808adec..bfcbe0b8670 100644
>>>>> --- a/gcc/cp/pt.cc
>>>>> +++ b/gcc/cp/pt.cc
>>>>> @@ -10475,14 +10475,15 @@ tree
>>>>>     lookup_and_finish_template_variable (tree templ, tree targs,
>>>>>     				     tsubst_flags_t complain)
>>>>>     {
>>>>> -  templ = lookup_template_variable (templ, targs);
>>>>> -  if (!any_dependent_template_arguments_p (targs))
>>>>> +  tree var = lookup_template_variable (templ, targs);
>>>>> +  if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
>>>>> +      && !any_dependent_template_arguments_p (targs))
>>>>
>>>> I notice that finish_id_expression_1 uses the equivalent of
>>>> type_dependent_expression_p (var).  Does that work here?
>>>
>>> Hmm, it does, but kind of by accident: type_dependent_expression_p
>>> returns true for all variable TEMPLATE_ID_EXPRs because of their empty
>>> TREE_TYPE (as set by finish_template_variable).  So testing t_d_e_p here
>>> is equivalent to testing processing_template_decl, it seems -- maximally
>>> conservative.
>>>
>>> We can improve type_dependent_expression_p for variable TEMPLATE_ID_EXPR
>>> by ignoring its (always empty) TREE_TYPE and just considering dependence
>>> of its template and args directly.

I guess the problem is that a variable template specialization is 
type-dependent until it's instantiated or specialized, and here we're 
trying to instantiate if that hasn't happened yet, so using 
type_dependent_expression_p would be wrong.

The patch is OK as is.

Jason