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

[Patrick Palka <ppalka@redhat.com>]

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