[PATCH] c++: implicit dummy object in requires clause [PR103198]

[Jason Merrill]

On 11/18/21 14:49, Patrick Palka wrote:
> On Thu, 18 Nov 2021, Jason Merrill wrote:
> 
>> On 11/17/21 14:52, Patrick Palka wrote:
>>> On Wed, 17 Nov 2021, Jason Merrill wrote:
>>>
>>>> On 11/11/21 20:25, Patrick Palka wrote:
>>>>> In the testcase below satisfaction misbehaves for f and g ultimately
>>>>> because find_template_parameters fails to notice that the constraint
>>>>> 'val.x' depends on the template parameters of the class template.
>>>>> In contrast, satisfaction works just fine for h.
>>>>>
>>>>> The problem seems to come down to a difference in how
>>>>> any_template_parm_r
>>>>> handles 'this' vs a dummy object: we walk TREE_TYPE of the former but
>>>>> not the latter, and this causes us to miss the tparm dependencies in
>>>>> f/g's constraints since in their case the implicit object parameter
>>>>> through which we access 'val' is a dummy object.  (For h, since we know
>>>>> it's a non-static member function when parsing its trailing constraints,
>>>>> the implicit object parameter is 'this' instead of a dummy object.)
>>>>>
>>>>> This patch fixes this inconsistency by making any_template_parm_r also
>>>>> walk into the TREE_TYPE of a dummy object, as is already done for
>>>>> 'this'.
>>>>>
>>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, also tested on
>>>>> cmcstl2 and range-v3, does this look OK for trunk and 11?
>>>>>
>>>>> 	PR c++/103198
>>>>>
>>>>> gcc/cp/ChangeLog:
>>>>>
>>>>> 	* pt.c (any_template_parm_r): Walk the TREE_TYPE of a dummy
>>>>> 	object.
>>>>
>>>> Should we handle CONVERT_EXPR with the various casts in cp_walk_subtrees?
>>>
>>> This seems to work well too.  But I'm not sure about doing this since
>>> IIUC cp_walk_subtrees is generally supposed to walk subtrees that are
>>> explicitly written in the source code, but when a CONVERT_EXPR
>>> corresponds to an implicit conversion then the target type doesn't
>>> explicitly appear anywhere.
>>
>> We could check is_dummy_object there as well?
> 
> Ah I see, sorry for the misunderstanding.  So wouldn't that mean
> cp_walk_subtrees will wal the TREE_TYPE of a dummy object but not the
> TREE_TYPE of 'this'?  That seems like a weird inconsistency at first
> glance.

Good point.  Go ahead with your original patch, thanks.

>>>>
>>>>> gcc/testsuite/ChangeLog:
>>>>>
>>>>> 	* g++.dg/cpp2a/concepts-this1.C: New test.
>>>>> ---
>>>>>     gcc/cp/pt.c                                 |  5 ++++
>>>>>     gcc/testsuite/g++.dg/cpp2a/concepts-this1.C | 30
>>>>> +++++++++++++++++++++
>>>>>     2 files changed, 35 insertions(+)
>>>>>     create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-this1.C
>>>>>
>>>>> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
>>>>> index 82bf7dc26f6..fa55857d783 100644
>>>>> --- a/gcc/cp/pt.c
>>>>> +++ b/gcc/cp/pt.c
>>>>> @@ -10766,6 +10766,11 @@ any_template_parm_r (tree t, void *data)
>>>>>     	WALK_SUBTREE (TREE_TYPE (t));
>>>>>           break;
>>>>>     +    case CONVERT_EXPR:
>>>>> +      if (is_dummy_object (t))
>>>>> +	WALK_SUBTREE (TREE_TYPE (t));
>>>>> +      break;
>>>>> +
>>>>>         default:
>>>>>           break;
>>>>>         }
>>>>> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-this1.C
>>>>> b/gcc/testsuite/g++.dg/cpp2a/concepts-this1.C
>>>>> new file mode 100644
>>>>> index 00000000000..d717028201a
>>>>> --- /dev/null
>>>>> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-this1.C
>>>>> @@ -0,0 +1,30 @@
>>>>> +// PR c++/103198
>>>>> +// { dg-do compile { target c++20 } }
>>>>> +
>>>>> +template<class T, class = void>
>>>>> +struct A {
>>>>> +  T val;
>>>>> +
>>>>> +  template<class U>
>>>>> +    requires requires { val.x; }
>>>>> +  void f(U);
>>>>> +
>>>>> +  static void g(int)
>>>>> +    requires requires { val.x; };
>>>>> +
>>>>> +  void h(int)
>>>>> +    requires requires { val.x; };
>>>>> +};
>>>>> +
>>>>> +struct B { int x; };
>>>>> +struct C { };
>>>>> +
>>>>> +int main() {
>>>>> +  A<B>().f(0);
>>>>> +  A<B>().g(0);
>>>>> +  A<B>().h(0);
>>>>> +
>>>>> +  A<C>().f(0); // { dg-error "no match" }
>>>>> +  A<C>().g(0); // { dg-error "no match" }
>>>>> +  A<C>().h(0); // { dg-error "no match" }
>>>>> +}
>>>>>
>>>>
>>>>
>>>
>>
>>
>