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

[Jason Merrill <jason@redhat.com>]

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?

Jason