From b5be6d0c166ff1fd3cf90eb846fa24453ee0fc10 Mon Sep 17 00:00:00 2001 From: Marek Polacek Date: Sun, 26 Aug 2018 16:45:51 +0000 Subject: [PATCH] PR c++/87029, Implement -Wredundant-move. * c.opt (Wredundant-move): New option. * typeck.c (treat_lvalue_as_rvalue_p): New function. (maybe_warn_pessimizing_move): Call convert_from_reference. Warn about redundant moves. * doc/invoke.texi: Document -Wredundant-move. * g++.dg/cpp0x/Wredundant-move1.C: New test. * g++.dg/cpp0x/Wredundant-move2.C: New test. * g++.dg/cpp0x/Wredundant-move3.C: New test. * g++.dg/cpp0x/Wredundant-move4.C: New test. From-SVN: r263863 --- gcc/ChangeLog | 5 + gcc/c-family/ChangeLog | 5 + gcc/c-family/c.opt | 4 + gcc/cp/ChangeLog | 5 + gcc/cp/typeck.c | 37 ++++-- gcc/doc/invoke.texi | 46 +++++++- gcc/testsuite/ChangeLog | 6 + gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C | 106 ++++++++++++++++++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C | 57 ++++++++++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C | 43 +++++++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move4.C | 86 ++++++++++++++ 11 files changed, 391 insertions(+), 9 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wredundant-move4.C diff --git a/gcc/ChangeLog b/gcc/ChangeLog index b183818918e..386dc8e886a 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,8 @@ +2018-08-26 Marek Polacek + + PR c++/87029, Implement -Wredundant-move. + * doc/invoke.texi: Document -Wredundant-move. + 2018-08-25 Martin Sebor PR tree-optimization/87059 diff --git a/gcc/c-family/ChangeLog b/gcc/c-family/ChangeLog index 790df09eb59..f833d0a1722 100644 --- a/gcc/c-family/ChangeLog +++ b/gcc/c-family/ChangeLog @@ -1,3 +1,8 @@ +2018-08-26 Marek Polacek + + PR c++/87029, Implement -Wredundant-move. + * c.opt (Wredundant-move): New option. + 2018-08-21 Marek Polacek PR c++/86981, Implement -Wpessimizing-move. diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 76840dd77ad..31a2b972919 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -985,6 +985,10 @@ Wredundant-decls C ObjC C++ ObjC++ Var(warn_redundant_decls) Warning Warn about multiple declarations of the same object. +Wredundant-move +C++ ObjC++ Var(warn_redundant_move) Warning LangEnabledBy(C++ ObjC++,Wextra) +Warn about redundant calls to std::move. + Wregister C++ ObjC++ Var(warn_register) Warning Warn about uses of register storage specifier. diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog index 258c6a9b763..f1ae08e6f7d 100644 --- a/gcc/cp/ChangeLog +++ b/gcc/cp/ChangeLog @@ -3,6 +3,11 @@ PR c++/87080 * typeck.c (maybe_warn_pessimizing_move): Do nothing in a template. + PR c++/87029, Implement -Wredundant-move. + * typeck.c (treat_lvalue_as_rvalue_p): New function. + (maybe_warn_pessimizing_move): Call convert_from_reference. + Warn about redundant moves. + 2018-08-24 Marek Polacek PR c++/67012 diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index 24647e29a55..fa3ba715698 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -9178,6 +9178,19 @@ can_do_nrvo_p (tree retval, tree functype) && !TYPE_VOLATILE (TREE_TYPE (retval))); } +/* Returns true if we should treat RETVAL, an expression being returned, + as if it were designated by an rvalue. See [class.copy.elision]. */ + +static bool +treat_lvalue_as_rvalue_p (tree retval) +{ + return ((cxx_dialect != cxx98) + && ((VAR_P (retval) && !DECL_HAS_VALUE_EXPR_P (retval)) + || TREE_CODE (retval) == PARM_DECL) + && DECL_CONTEXT (retval) == current_function_decl + && !TREE_STATIC (retval)); +} + /* Warn about wrong usage of std::move in a return statement. RETVAL is the expression we are returning; FUNCTYPE is the type the function is declared to return. */ @@ -9185,9 +9198,11 @@ can_do_nrvo_p (tree retval, tree functype) static void maybe_warn_pessimizing_move (tree retval, tree functype) { - if (!warn_pessimizing_move) + if (!(warn_pessimizing_move || warn_redundant_move)) return; + location_t loc = cp_expr_loc_or_loc (retval, input_location); + /* C++98 doesn't know move. */ if (cxx_dialect < cxx11) return; @@ -9212,14 +9227,24 @@ maybe_warn_pessimizing_move (tree retval, tree functype) STRIP_NOPS (arg); if (TREE_CODE (arg) == ADDR_EXPR) arg = TREE_OPERAND (arg, 0); + arg = convert_from_reference (arg); /* Warn if we could do copy elision were it not for the move. */ if (can_do_nrvo_p (arg, functype)) { auto_diagnostic_group d; - if (warning_at (location_of (retval), OPT_Wpessimizing_move, + if (warning_at (loc, OPT_Wpessimizing_move, "moving a local object in a return statement " "prevents copy elision")) - inform (location_of (retval), "remove % call"); + inform (loc, "remove % call"); + } + /* Warn if the move is redundant. It is redundant when we would + do maybe-rvalue overload resolution even without std::move. */ + else if (treat_lvalue_as_rvalue_p (arg)) + { + auto_diagnostic_group d; + if (warning_at (loc, OPT_Wredundant_move, + "redundant move in return statement")) + inform (loc, "remove % call"); } } } @@ -9499,11 +9524,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; - if ((cxx_dialect != cxx98) - && ((VAR_P (retval) && !DECL_HAS_VALUE_EXPR_P (retval)) - || TREE_CODE (retval) == PARM_DECL) - && DECL_CONTEXT (retval) == current_function_decl - && !TREE_STATIC (retval) + if (treat_lvalue_as_rvalue_p (retval) /* This is only interesting for class type. */ && CLASS_TYPE_P (functype)) { diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi index e4148297a87..985ef9f3510 100644 --- a/gcc/doc/invoke.texi +++ b/gcc/doc/invoke.texi @@ -231,7 +231,7 @@ in the following sections. -Wdelete-non-virtual-dtor -Wdeprecated-copy -Wliteral-suffix @gol -Wmultiple-inheritance @gol -Wnamespaces -Wnarrowing @gol --Wpessimizing-move @gol +-Wpessimizing-move -Wredundant-move @gol -Wnoexcept -Wnoexcept-type -Wclass-memaccess @gol -Wnon-virtual-dtor -Wreorder -Wregister @gol -Weffc++ -Wstrict-null-sentinel -Wtemplates @gol @@ -3158,6 +3158,49 @@ But in this example, the @code{std::move} call prevents copy elision. This warning is enabled by @option{-Wall}. +@item -Wno-redundant-move @r{(C++ and Objective-C++ only)} +@opindex Wredundant-move +@opindex Wno-redundant-move +This warning warns about redundant calls to @code{std::move}; that is, when +a move operation would have been performed even without the @code{std::move} +call. This happens because the compiler is forced to treat the object as if +it were an rvalue in certain situations such as returning a local variable, +where copy elision isn't applicable. Consider: + +@smallexample +struct T @{ +@dots{} +@}; +T fn(T t) +@{ + @dots{} + return std::move (t); +@} +@end smallexample + +Here, the @code{std::move} call is redundant. Because G++ implements Core +Issue 1579, another example is: + +@smallexample +struct T @{ // convertible to U +@dots{} +@}; +struct U @{ +@dots{} +@}; +U fn() +@{ + T t; + @dots{} + return std::move (t); +@} +@end smallexample +In this example, copy elision isn't applicable because the type of the +expression being returned and the function return type differ, yet G++ +treats the return value as if it were designated by an rvalue. + +This warning is enabled by @option{-Wextra}. + @item -fext-numeric-literals @r{(C++ and Objective-C++ only)} @opindex fext-numeric-literals @opindex fno-ext-numeric-literals @@ -4112,6 +4155,7 @@ name is still supported, but the newer name is more descriptive.) -Wold-style-declaration @r{(C only)} @gol -Woverride-init @gol -Wsign-compare @r{(C only)} @gol +-Wredundant-move @r{(only for C++)} @gol -Wtype-limits @gol -Wuninitialized @gol -Wshift-negative-value @r{(in C++03 and in C99 and newer)} @gol diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 3bd02e71eed..69bc12da284 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -3,6 +3,12 @@ PR c++/87080 * g++.dg/cpp0x/Wpessimizing-move5.C: New test. + PR c++/87029, Implement -Wredundant-move. + * g++.dg/cpp0x/Wredundant-move1.C: New test. + * g++.dg/cpp0x/Wredundant-move2.C: New test. + * g++.dg/cpp0x/Wredundant-move3.C: New test. + * g++.dg/cpp0x/Wredundant-move4.C: New test. + 2018-08-25 Thomas Koenig PR libfortran/86704 diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C new file mode 100644 index 00000000000..5d4a25dbc3b --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C @@ -0,0 +1,106 @@ +// PR c++/87029 +// { dg-do compile { target c++11 } } +// { dg-options "-Wredundant-move" } + +// Define std::move. +namespace std { + template + struct remove_reference + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&> + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&&> + { typedef _Tp type; }; + + template + constexpr typename std::remove_reference<_Tp>::type&& + move(_Tp&& __t) noexcept + { return static_cast::type&&>(__t); } +} + +struct T { + T() { } + T(const T&) { } + T(T&&) { } +}; + +struct U { + U() { } + U(const U&) { } + U(U&&) { } + U(T) { } +}; + +T +fn1 (T t) +{ + return t; +} + +T +fn2 (T t) +{ + // Will use move even without std::move. + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +T +fn3 (const T t) +{ + // t is const: will decay into copy. + return t; +} + +T +fn4 (const T t) +{ + // t is const: will decay into copy despite std::move, so it's redundant. + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +int +fn5 (int i) +{ + // Not a class type. + return std::move (i); +} + +T +fn6 (T t, bool b) +{ + if (b) + throw std::move (t); + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +U +fn7 (T t) +{ + // Core 1579 means we'll get a move here. + return t; +} + +U +fn8 (T t) +{ + // Core 1579 means we'll get a move here. Even without std::move. + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +T +fn9 (T& t) +{ + // T is a reference and the move isn't redundant. + return std::move (t); +} + +T +fn10 (T&& t) +{ + // T is a reference and the move isn't redundant. + return std::move (t); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C new file mode 100644 index 00000000000..f181afeeb84 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C @@ -0,0 +1,57 @@ +// PR c++/87029 +// { dg-do compile { target c++11 } } +// { dg-options "-Wredundant-move" } + +// Define std::move. +namespace std { + template + struct remove_reference + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&> + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&&> + { typedef _Tp type; }; + + template + constexpr typename std::remove_reference<_Tp>::type&& + move(_Tp&& __t) noexcept + { return static_cast::type&&>(__t); } +} + +struct T { }; +struct U { U(T); }; + +template +T +fn1 (T t) +{ + // Non-dependent type. + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +template +Tp1 +fn2 (Tp2 t) +{ + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +template +Tp1 +fn3 (Tp2 t) +{ + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +int +main () +{ + T t; + fn1(t); + fn2(t); + fn3(t); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C new file mode 100644 index 00000000000..7084134e370 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C @@ -0,0 +1,43 @@ +// PR c++/87029 +// { dg-do compile { target c++11 } } +// { dg-options "-Wredundant-move" } + +// Define std::move. +namespace std { + template + struct remove_reference + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&> + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&&> + { typedef _Tp type; }; + + template + constexpr typename std::remove_reference<_Tp>::type&& + move(_Tp&& __t) noexcept + { return static_cast::type&&>(__t); } +} + +struct T { }; + +T +fn1 (T t) +{ + return (1, std::move (t)); +} + +T +fn2 (T t) +{ + return [&](){ return std::move (t); }(); +} + +T +fn3 (T t) +{ + return [=](){ return std::move (t); }(); +} diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move4.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move4.C new file mode 100644 index 00000000000..aa89e46de99 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move4.C @@ -0,0 +1,86 @@ +// PR c++/87029 +// { dg-do compile { target c++11 } } +// { dg-options "-Wredundant-move" } + +// Define std::move. +namespace std { + template + struct remove_reference + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&> + { typedef _Tp type; }; + + template + struct remove_reference<_Tp&&> + { typedef _Tp type; }; + + template + constexpr typename std::remove_reference<_Tp>::type&& + move(_Tp&& __t) noexcept + { return static_cast::type&&>(__t); } +} + +struct T { + T() { } + T(const T&) { } + T(T&&) { } +}; + +struct U { + U() { } + U(const U&) { } + U(U&&) { } + U(T) { } +}; + +U +fn1 (T t, bool b) +{ + if (b) + return t; + else + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +U +fn2 (bool b) +{ + T t; + if (b) + return t; + else + return std::move (t); // { dg-warning "redundant move in return statement" } +} + +U +fn3 (bool b) +{ + static T t; + if (b) + return t; + else + return std::move (t); +} + +T g; + +U +fn4 (bool b) +{ + if (b) + return g; + else + return std::move (g); +} + +long int +fn5 (bool b) +{ + int i = 42; + if (b) + return i; + else + return std::move (i); +} -- 2.30.2