| Message ID | 20211112012529.1829478-1-ppalka@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++: implicit dummy object in requires clause [PR103198] | expand |
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? > 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" } > +} >
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. > > > 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" } > > +} > > > >
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? >> >>> 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" } >>> +} >>> >> >> >
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. > > > > > > > > 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" } > > > > +} > > > > > > > > > > > > > >
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" } >>>>> +} >>>>> >>>> >>>> >>> >> >> >
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" } +}