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
