This patch fixes a bogus -Wredundant-move warning. In the test in the PR the std::move call isn't redundant; the testcase won't actually compile without that call, as per the resolution of bug 87150.
Before this patch, we'd issue the redundant-move warning anytime treat_lvalue_as_rvalue_p is true for a std::move's argument. But we also need to make sure that convert_for_initialization works even without the std::move call, if not, it's not redundant. Trouble arises when the argument is const. Then it might be the case that the implicit rvalue fails because it uses a const copy constructor, or that the type of the returned object and the type of the selected ctor's parameter aren't the same. To handle various corner cases (e.g. when the std::move actually makes a difference between const T& and const T&&) I had to use another convert_for_initialization, but the first one is the important one and should handle most of the cases. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2019-03-04 Marek Polacek <pola...@redhat.com> PR c++/87378 - bogus -Wredundant-move warning. * typeck.c (maybe_warn_pessimizing_move): See if the maybe-rvalue overload resolution would actually succeed. * g++.dg/cpp0x/Wredundant-move7.C: New test. * g++.dg/cpp0x/Wredundant-move8.C: New test. diff --git gcc/cp/typeck.c gcc/cp/typeck.c index 1bf9ad88141..7a43ba70010 100644 --- gcc/cp/typeck.c +++ gcc/cp/typeck.c @@ -9429,10 +9429,43 @@ maybe_warn_pessimizing_move (tree retval, tree functype) do maybe-rvalue overload resolution even without std::move. */ else if (treat_lvalue_as_rvalue_p (arg, /*parm_ok*/true)) { - auto_diagnostic_group d; - if (warning_at (loc, OPT_Wredundant_move, - "redundant move in return statement")) - inform (loc, "remove %<std::move%> call"); + /* Make sure that the overload resolution would actually succeed + if we removed the std::move call. */ + tree moved = move (arg); + tree t = convert_for_initialization (NULL_TREE, functype, moved, + (LOOKUP_NORMAL + | LOOKUP_ONLYCONVERTING + | LOOKUP_PREFER_RVALUE), + ICR_RETURN, NULL_TREE, 0, + tf_none); + /* If this worked, implicit rvalue would work, so the call to + std::move is redundant. */ + if (t == error_mark_node + && CP_TYPE_CONST_P (TREE_TYPE (arg))) + { + /* But if ARG is const, it may be the case of using + const T& instead of T&&. */ + t = convert_for_initialization (NULL_TREE, functype, moved, + (LOOKUP_NORMAL + | LOOKUP_ONLYCONVERTING), + ICR_RETURN, NULL_TREE, 0, + tf_none); + /* Unless, rarely, using std::move would actually choose + const T && over const T&. In that case std::move isn't + redundant. */ + if (TREE_CODE (t) == TARGET_EXPR) + t = TARGET_EXPR_INITIAL (t); + tree call = cp_get_callee_fndecl_nofold (t); + if (call && DECL_MOVE_CONSTRUCTOR_P (call)) + t = error_mark_node; + } + if (t != error_mark_node) + { + auto_diagnostic_group d; + if (warning_at (loc, OPT_Wredundant_move, + "redundant move in return statement")) + inform (loc, "remove %<std::move%> call"); + } } } } diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C new file mode 100644 index 00000000000..015d7c4f7a4 --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C @@ -0,0 +1,59 @@ +// PR c++/87378 +// { dg-do compile { target c++11 } } +// { dg-options "-Wredundant-move" } + +// Define std::move. +namespace std { + template<typename _Tp> + struct remove_reference + { typedef _Tp type; }; + + template<typename _Tp> + struct remove_reference<_Tp&> + { typedef _Tp type; }; + + template<typename _Tp> + struct remove_reference<_Tp&&> + { typedef _Tp type; }; + + template<typename _Tp> + constexpr typename std::remove_reference<_Tp>::type&& + move(_Tp&& __t) noexcept + { return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); } +} + +struct S1 { S1(S1 &&); }; +struct S2 : S1 {}; + +S1 +f (S2 s) +{ + return std::move(s); // { dg-bogus "redundant move in return statement" } +} + +struct R1 { + R1(R1 &&); + R1(const R1 &&); +}; +struct R2 : R1 {}; + +R1 +f2 (const R2 s) +{ + return std::move(s); // { dg-bogus "redundant move in return statement" } +} + +struct T1 { + T1(const T1 &); + T1(T1 &&); + T1(const T1 &&); +}; +struct T2 : T1 {}; + +T1 +f3 (const T2 s) +{ + // Without std::move: const T1 & + // With std::move: const T1 && + return std::move(s); // { dg-bogus "redundant move in return statement" } +} diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C new file mode 100644 index 00000000000..6d34e55522e --- /dev/null +++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C @@ -0,0 +1,63 @@ +// PR c++/87378 +// { dg-do compile { target c++11 } } +// { dg-options "-Wredundant-move" } + +// Define std::move. +namespace std { + template<typename _Tp> + struct remove_reference + { typedef _Tp type; }; + + template<typename _Tp> + struct remove_reference<_Tp&> + { typedef _Tp type; }; + + template<typename _Tp> + struct remove_reference<_Tp&&> + { typedef _Tp type; }; + + template<typename _Tp> + constexpr typename std::remove_reference<_Tp>::type&& + move(_Tp&& __t) noexcept + { return static_cast<typename std::remove_reference<_Tp>::type&&>(__t); } +} + +struct S1 { + S1(S1&&) { } +}; + +S1 f1(S1 s) +{ + return std::move(s); // { dg-warning "redundant move in return statement" } +} + +struct S2 { + S2(const S2&) { } + S2(S2&&) { } +}; + +S2 f2(S2 s) +{ + return std::move(s); // { dg-warning "redundant move in return statement" } +} + +S2 f2c(const S2 s) +{ + return std::move(s); // { dg-warning "redundant move in return statement" } +} + +struct S3 { + S3(const S3&) { } + S3(S3&&) { } + S3(const S3&&) { } +}; + +S3 f3(S3 s) +{ + return std::move(s); // { dg-warning "redundant move in return statement" } +} + +S3 f3c(const S3 s) +{ + return std::move(s); // { dg-warning "redundant move in return statement" } +}