Re: [PATCH] c++: Implement C++23 P2266R1, Simpler implicit move [PR101165]
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 (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_retur
Re: [PATCH] c++: Implement C++23 P2266R1, Simpler implicit move [PR101165]
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 (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
Re: [PATCH] c++: Implement C++23 P2266R1, Simpler implicit move [PR101165]
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 (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? Marek
Re: [PATCH] c++: Implement C++23 P2266R1, Simpler implicit move [PR101165]
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. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/101165 gcc/c-family/ChangeLog: * c-cppbuiltin.cc (c_cpp_builtins): Define __cpp_implicit_move. gcc/cp/ChangeLog: * call.cc (reference_binding): Check clk_implicit_rval in C++20 only. * cp-tree.h (unparenthesized_id_or_class_member_access_p): Declare. * pt.cc (unparenthesized_id_or_class_member_access_p): New function, broken out of... (do_auto_deduction): ...here. Use it. * tree.cc (xvalue_p): Check & clk_rvalueref, not == clk_rvalueref. * typeck.cc (check_return_expr): In C++23, maybe call treat_lvalue_as_rvalue_p before do_auto_deduction. Allow implicit move for functions returning a reference as well. gcc/testsuite/ChangeLog: * g++.dg/conversion/pr41426.C: Add dg-error for C++23. * g++.dg/cpp0x/elision_weak.C: Likewise. * g++.dg/cpp0x/move-return3.C: Only link in c++20_down. * g++.dg/cpp1y/decltype-auto2.C: Add dg-error for C++23. * g++.dg/cpp1y/lambda-generic-89419.C: Likewise. * g++.dg/cpp23/feat-cxx2b.C: Test __cpp_implicit_move. * g++.dg/gomp/pr56217.C: Only compile in c++20_down. * g++.dg/warn/Wno-return-local-addr.C: Add dg-error for C++23. * g++.dg/warn/Wreturn-local-addr.C: Adjust dg-error. * g++.old-deja/g++.brendan/crash55.C: Add dg-error for C++23. * g++.old-deja/g++.jason/temporary2.C: Likewise. * g++.old-deja/g++.mike/p2846b.C: Only run in c++20_down. * g++.dg/cpp1y/decltype-auto6.C: New test. * g++.dg/cpp23/decltype1.C: New test. * g++.dg/cpp23/decltype2.C: New test. * g++.dg/cpp23/elision1.C: New test. * g++.dg/cpp23/elision2.C: New test. * g++.dg/cpp23/elision3.C: New test. * g++.dg/cpp23/elision4.C: New test. * g++.dg/cpp23/elision5.C: New test. * g++.dg/cpp23/elision6.C: New test. * g++.dg/cpp23/elision7.C: New test. --- gcc/c-family/c-cppbuiltin.cc | 1 + gcc/cp/call.cc| 2 +- gcc/cp/cp-tree.h | 1 + gcc/cp/pt.cc | 33 +++-- gcc/cp/tree.cc| 2 +- gcc/cp/typeck.cc | 28 - gcc/testsuite/g++.dg/conversion/pr41426.C | 10 +- gcc/testsuite/g++.dg/cpp0x/elision_weak.C | 4 +- gcc/testsuite/g++.dg/cpp0x/move-return3.C | 3 +- gcc/testsuite/g++.dg/cpp1y/decl
[PATCH] c++: Implement C++23 P2266R1, Simpler implicit move [PR101165]
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. 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; } 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. Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk? PR c++/101165 gcc/c-family/ChangeLog: * c-cppbuiltin.cc (c_cpp_builtins): Define __cpp_implicit_move. gcc/cp/ChangeLog: * call.cc (reference_binding): Check clk_implicit_rval in C++20 only. * cp-tree.h (unparenthesized_id_or_class_member_access_p): Declare. * pt.cc (unparenthesized_id_or_class_member_access_p): New function, broken out of... (do_auto_deduction): ...here. Use it. * tree.cc (xvalue_p): Check & clk_rvalueref, not == clk_rvalueref. * typeck.cc (check_return_expr): In C++23, maybe call treat_lvalue_as_rvalue_p before do_auto_deduction. Allow implicit move for functions returning a reference as well. gcc/testsuite/ChangeLog: * g++.dg/conversion/pr41426.C: Add dg-error for C++23. * g++.dg/cpp0x/elision_weak.C: Likewise. * g++.dg/cpp0x/move-return3.C: Only link in c++20_down. * g++.dg/cpp1y/decltype-auto2.C: Add dg-error for C++23. * g++.dg/cpp1y/lambda-generic-89419.C: Likewise. * g++.dg/cpp23/feat-cxx2b.C: Test __cpp_implicit_move. * g++.dg/gomp/pr56217.C: Only compile in c++20_down. * g++.dg/warn/Wno-return-local-addr.C: Add dg-error for C++23. * g++.dg/warn/Wreturn-local-addr.C: Adjust dg-error. * g++.old-deja/g++.brendan/crash55.C: Add dg-error for C++23. * g++.old-deja/g++.jason/temporary2.C: Likewise. * g++.old-deja/g++.mike/p2846b.C: Only run in c++20_down. * g++.dg/cpp1y/decltype-auto6.C: New test. * g++.dg/cpp23/decltype1.C: New test. * g++.dg/cpp23/decltype2.C: New test. * g++.dg/cpp23/elision1.C: New test. * g++.dg/cpp23/elision2.C: New test. * g++.dg/cpp23/elision3.C: New test. * g++.dg/cpp23/elision4.C: New test. * g++.dg/cpp23/elision5.C: New test. * g++.dg/cpp23/elision6.C: New test. * g++.dg/cpp23/elision7.C: New test. --- gcc/c-family/c-cppbuiltin.cc | 1 + gcc/cp/call.cc| 2 +- gcc/cp/cp-tree.h | 1 + gcc/cp/pt.cc | 33 +++-- gcc/cp/tree.cc| 2 +- gcc/cp/typeck.cc | 28 - gcc/testsuite/g++.dg/conversion/pr41426.C | 10 +- gcc/testsuite/g++.dg/cpp0x/elision_weak.C | 4 +- gcc/testsuite/g++.dg/cpp0x/move-return3.C | 3 +- gcc/testsuite/g++.dg/cpp1y/decltype-auto2.C | 2 +- gcc/testsuite/g++.dg/cpp1y/decltype-auto6.C | 19 +++ .../g++.dg/cpp1y/lambda-generic-89419.C | 6 +- gcc/testsuite/g++.dg/cpp23/decltype1.C| 113 ++ gcc/testsuite/g++.dg/cpp23/decltype2.C| 49 gc