Re: [PATCH] c++: Implement C++23 P2266R1, Simpler implicit move [PR101165]

[Marek Polacek <polacek@redhat.com>]

On Mon, Sep 12, 2022 at 04:27:27PM -0400, Jason Merrill wrote:
> On 9/8/22 18:54, Marek Polacek wrote:
> > On Tue, Sep 06, 2022 at 10:38:12PM -0400, Jason Merrill wrote:
> > > On 9/3/22 12:42, Marek Polacek wrote:
> > > > This patch implements https://wg21.link/p2266, which, once again,
> > > > changes the implicit move rules.  Here's a brief summary of various
> > > > changes in this area:
> > > > 
> > > > r125211: Introduced moving from certain lvalues when returning them
> > > > r171071: CWG 1148, enable move from value parameter on return
> > > > r212099: CWG 1579, it's OK to call a converting ctor taking an rvalue
> > > > r251035: CWG 1579, do maybe-rvalue overload resolution twice
> > > > r11-2411: Avoid calling const copy ctor on implicit move
> > > > r11-2412: C++20 implicit move changes, remove the fallback overload
> > > >             resolution, allow move on throw of parameters and implicit
> > > > 	  move of rvalue references
> > > > 
> > > > P2266 enables the implicit move for functions that return references.  This
> > > > was a one-line change: check TYPE_REF_P.  That is, we will now perform
> > > > a move in
> > > > 
> > > >     X&& foo (X&& x) {
> > > >       return x;
> > > >     }
> > > > 
> > > > P2266 also removes the fallback overload resolution, but this was
> > > > resolved by r11-2412: we only do convert_for_initialization with
> > > > LOOKUP_PREFER_RVALUE in C++17 and older.
> > > 
> > > I wonder if we want to extend the current C++20 handling to the older modes
> > > for GCC 13?  Not in this patch, but as a followup.
> > > 
> > > > P2266 also says that a returned move-eligible id-expression is always an
> > > > xvalue.  This required some further short, but nontrivial changes,
> > > > especially when it comes to deduction, because we have to pay attention
> > > > to whether we have auto, auto&& (which is like T&&), or decltype(auto)
> > > > with (un)parenthesized argument.  In C++23,
> > > > 
> > > >     decltype(auto) f(int&& x) { return (x); }
> > > >     auto&& f(int x) { return x; }
> > > > 
> > > > both should deduce to 'int&&' but
> > > > 
> > > >     decltype(auto) f(int x) { return x; }
> > > > 
> > > > should deduce to 'int'.  A cornucopia of tests attached.  I've also
> > > > verified that we behave like clang++.
> > > > 
> > > > xvalue_p seemed to be broken: since the introduction of clk_implicit_rval,
> > > > it cannot use '==' when checking for clk_rvalueref.
> > > > 
> > > > Since this change breaks code, it's only enabled in C++23.  In
> > > > particular, this code will not compile in C++23:
> > > > 
> > > >     int& g(int&& x) { return x; }
> > > 
> > > Nice that the C++20 compatibility is so simple!
> > > 
> > > > because x is now treated as an rvalue, and you can't bind a non-const lvalue
> > > > reference to an rvalue.
> > > > 
> > > > There's one FIXME in elision1.C:five, which we should compile but reject
> > > > with "passing 'Mutt' as 'this' argument discards qualifiers".  That
> > > > looks bogus to me, I think I'll open a PR for it.
> > > 
> > > Let's fix that now, I think.
> > 
> > Can of worms.   The test is
> > 
> >    struct Mutt {
> >        operator int*() &&;
> >    };
> > 
> >    int* five(Mutt x) {
> >        return x;  // OK since C++20 because P1155
> >    }
> > 
> > 'x' should be treated as an rvalue, therefore the operator fn taking
> > an rvalue ref to Mutt should be used to convert 'x' to int*.  We fail
> > because we don't treat 'x' as an rvalue because the function doesn't
> > return a class.  So the patch should be just
> > 
> > --- a/gcc/cp/typeck.cc
> > +++ b/gcc/cp/typeck.cc
> > @@ -10875,10 +10875,7 @@ check_return_expr (tree retval, bool *no_warning)
> >            Note that these conditions are similar to, but not as strict as,
> >       the conditions for the named return value optimization.  */
> >         bool converted = false;
> > -      tree moved;
> > -      /* This is only interesting for class type.  */
> > -      if (CLASS_TYPE_P (functype)
> > -     && (moved = treat_lvalue_as_rvalue_p (retval, /*return*/true)))
> > +      if (tree moved = treat_lvalue_as_rvalue_p (retval, /*return*/true))
> >      {
> >        if (cxx_dialect < cxx20)
> >          {
> > 
> > which fixes the test, but breaks a lot of middle-end warnings.  For instance
> > g++.dg/warn/nonnull3.C, where the patch above changes .gimple:
> > 
> >   bool A::foo<B> (struct A * const this, <<< Unknown tree: offset_type >>> p)
> >   {
> > -  bool D.2146;
> > +  bool D.2150;
> > -  D.2146 = p != -1;
> > -  return D.2146;
> > +  p.0_1 = p;
> > +  D.2150 = p.0_1 != -1;
> > +  return D.2150;
> >   }
> > 
> > and we no longer get the warning.  I thought maybe I could undo the implicit
> > rvalue conversion in cp_fold, when it sees implicit_rvalue_p, but that didn't
> > work.  So currently I'm stuck.  Should we try to figure this out or push aside?
> 
> Can you undo the implicit rvalue conversion within check_return_expr, where
> we can still refer back to the original expression?
> 
> Or avoid the rvalue conversion if the return type is scalar?
> 
> Did you see my comments in the body of the patch?

I responded in the email I just sent so that we can have a single thread.

Marek