From 8d22c7cb8b1a6f9b67c54a798dd5504244614e51 Mon Sep 17 00:00:00 2001 From: Marek Polacek Date: Mon, 1 Aug 2022 17:02:23 -0400 Subject: [PATCH] c++: Extend -Wpessimizing-move to other contexts In my recent patch which enhanced -Wpessimizing-move so that it warns about class prvalues too I said that I'd like to extend it so that it warns in more contexts where a std::move can prevent copy elision, such as: T t = std::move(T()); T t(std::move(T())); T t{std::move(T())}; T t = {std::move(T())}; void foo (T); foo (std::move(T())); This patch does that by adding two maybe_warn_pessimizing_move calls. These must happen before we've converted the initializers otherwise the std::move will be buried in a TARGET_EXPR. PR c++/106276 gcc/cp/ChangeLog: * call.cc (build_over_call): Call maybe_warn_pessimizing_move. * cp-tree.h (maybe_warn_pessimizing_move): Declare. * decl.cc (build_aggr_init_full_exprs): Call maybe_warn_pessimizing_move. * typeck.cc (maybe_warn_pessimizing_move): Handle TREE_LIST and CONSTRUCTOR. Add a bool parameter and use it. Adjust a diagnostic message. (check_return_expr): Adjust the call to maybe_warn_pessimizing_move. gcc/testsuite/ChangeLog: * g++.dg/cpp0x/Wpessimizing-move7.C: Add dg-warning. * g++.dg/cpp0x/Wpessimizing-move8.C: New test. --- gcc/cp/call.cc | 5 +- gcc/cp/cp-tree.h | 1 + gcc/cp/decl.cc | 3 +- gcc/cp/typeck.cc | 58 +++++++++++++++------- gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C | 16 +++--- gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C | 65 +++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 28 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C diff --git a/gcc/cp/call.cc b/gcc/cp/call.cc index 01a7be10077..370137ebd6d 100644 --- a/gcc/cp/call.cc +++ b/gcc/cp/call.cc @@ -9627,10 +9627,13 @@ build_over_call (struct z_candidate *cand, int flags, tsubst_flags_t complain) if (!conversion_warning) arg_complain &= ~tf_warning; + if (arg_complain & tf_warning) + maybe_warn_pessimizing_move (arg, type, /*return_p*/false); + val = convert_like_with_context (conv, arg, fn, i - is_method, arg_complain); val = convert_for_arg_passing (type, val, arg_complain); - + if (val == error_mark_node) return error_mark_node; else diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index cb23975484f..9f2ff3728b4 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -8099,6 +8099,7 @@ extern tree finish_right_unary_fold_expr (tree, int); extern tree finish_binary_fold_expr (tree, tree, int); extern tree treat_lvalue_as_rvalue_p (tree, bool); extern bool decl_in_std_namespace_p (tree); +extern void maybe_warn_pessimizing_move (tree, tree, bool); /* in typeck2.cc */ extern void require_complete_eh_spec_types (tree, tree); diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc index 031be45e0be..84a1a011341 100644 --- a/gcc/cp/decl.cc +++ b/gcc/cp/decl.cc @@ -7220,9 +7220,10 @@ check_array_initializer (tree decl, tree type, tree init) static tree build_aggr_init_full_exprs (tree decl, tree init, int flags) - { gcc_assert (stmts_are_full_exprs_p ()); + if (init) + maybe_warn_pessimizing_move (init, TREE_TYPE (decl), /*return_p*/false); return build_aggr_init (decl, init, flags, tf_warning_or_error); } diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc index 6b38c8c7a00..19bb7f74321 100644 --- a/gcc/cp/typeck.cc +++ b/gcc/cp/typeck.cc @@ -10372,17 +10372,17 @@ treat_lvalue_as_rvalue_p (tree expr, bool return_p) } } -/* 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. */ +/* Warn about dubious usage of std::move (in a return statement, if RETURN_P + is true). EXPR is the std::move expression; TYPE is the type of the object + being initialized. */ -static void -maybe_warn_pessimizing_move (tree retval, tree functype) +void +maybe_warn_pessimizing_move (tree expr, tree type, bool return_p) { if (!(warn_pessimizing_move || warn_redundant_move)) return; - location_t loc = cp_expr_loc_or_input_loc (retval); + const location_t loc = cp_expr_loc_or_input_loc (expr); /* C++98 doesn't know move. */ if (cxx_dialect < cxx11) @@ -10394,14 +10394,32 @@ maybe_warn_pessimizing_move (tree retval, tree functype) return; /* This is only interesting for class types. */ - if (!CLASS_TYPE_P (functype)) + if (!CLASS_TYPE_P (type)) return; + /* A a = std::move (A()); */ + if (TREE_CODE (expr) == TREE_LIST) + { + if (list_length (expr) == 1) + expr = TREE_VALUE (expr); + else + return; + } + /* A a = {std::move (A())}; + A a{std::move (A())}; */ + else if (TREE_CODE (expr) == CONSTRUCTOR) + { + if (CONSTRUCTOR_NELTS (expr) == 1) + expr = CONSTRUCTOR_ELT (expr, 0)->value; + else + return; + } + /* We're looking for *std::move ((T &) &arg). */ - if (REFERENCE_REF_P (retval) - && TREE_CODE (TREE_OPERAND (retval, 0)) == CALL_EXPR) + if (REFERENCE_REF_P (expr) + && TREE_CODE (TREE_OPERAND (expr, 0)) == CALL_EXPR) { - tree fn = TREE_OPERAND (retval, 0); + tree fn = TREE_OPERAND (expr, 0); if (is_std_move_p (fn)) { tree arg = CALL_EXPR_ARG (fn, 0); @@ -10413,20 +10431,24 @@ maybe_warn_pessimizing_move (tree retval, tree functype) return; 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)) + if (can_do_rvo_p (arg, type)) { auto_diagnostic_group d; if (warning_at (loc, OPT_Wpessimizing_move, - "moving a local object in a return statement " - "prevents copy elision")) + "moving a temporary object prevents copy " + "elision")) inform (loc, "remove % call"); } - else if (can_do_rvo_p (arg, functype)) + /* The rest of the warnings is only relevant for when we are + returning from a function. */ + else if (!return_p) + return; + /* Warn if we could do copy elision were it not for the move. */ + else if (can_do_nrvo_p (arg, type)) { auto_diagnostic_group d; if (warning_at (loc, OPT_Wpessimizing_move, - "moving a temporary object in a return statement " + "moving a local object in a return statement " "prevents copy elision")) inform (loc, "remove % call"); } @@ -10437,7 +10459,7 @@ maybe_warn_pessimizing_move (tree retval, tree functype) { /* Make sure that overload resolution would actually succeed if we removed the std::move call. */ - tree t = convert_for_initialization (NULL_TREE, functype, + tree t = convert_for_initialization (NULL_TREE, type, moved, (LOOKUP_NORMAL | LOOKUP_ONLYCONVERTING @@ -10722,7 +10744,7 @@ check_return_expr (tree retval, bool *no_warning) return NULL_TREE; if (!named_return_value_okay_p) - maybe_warn_pessimizing_move (retval, functype); + maybe_warn_pessimizing_move (retval, functype, /*return_p*/true); /* Do any required conversions. */ if (bare_retval == result || DECL_CONSTRUCTOR_P (current_function_decl)) diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C index cd4eaa09aae..a17c7a87b7d 100644 --- a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C @@ -30,23 +30,23 @@ static A foo (); A fn1 () { - return std::move (A{}); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } - return std::move (A()); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } - return std::move (foo ()); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } + return std::move (A{}); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (foo ()); // { dg-warning "moving a temporary object prevents copy elision" } } B fn2 () { - return std::move (A()); - return std::move (A{}); - return std::move (foo ()); + return std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (A{}); // { dg-warning "moving a temporary object prevents copy elision" } + return std::move (foo ()); // { dg-warning "moving a temporary object prevents copy elision" } } template T1 fn3 () { - return std::move (T2{}); // { dg-warning "moving a temporary object in a return statement prevents copy elision" } + return std::move (T2{}); // { dg-warning "moving a temporary object prevents copy elision" } } void @@ -58,6 +58,6 @@ do_fn3 () char take_buffer; struct label_text { - label_text take() { return std::move(label_text(&take_buffer)); } // { dg-warning "moving a temporary object in a return statement prevents copy elision" } + label_text take() { return std::move(label_text(&take_buffer)); } // { dg-warning "moving a temporary object prevents copy elision" } label_text(char *); }; diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C new file mode 100644 index 00000000000..51406c8f97f --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C @@ -0,0 +1,65 @@ +// PR c++/106276 +// { dg-do compile { target c++11 } } +// { dg-options "-Wpessimizing-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 A { A(); A(const A&) = delete; A(A&&); }; +struct B { B(A); }; +struct X { }; + +void foo (A); +void bar (X); + +void +fn1 () +{ + A a1 = std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + A a2 = std::move (A{}); // { dg-warning "moving a temporary object prevents copy elision" } + A a3(std::move (A())); // { dg-warning "moving a temporary object prevents copy elision" } + A a4(std::move (A{})); // { dg-warning "moving a temporary object prevents copy elision" } + A a5{std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + A a6{std::move (A{})}; // { dg-warning "moving a temporary object prevents copy elision" } + A a7 = {std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + A a8 = {std::move (A{})}; // { dg-warning "moving a temporary object prevents copy elision" } + + B b1 = std::move (A()); // { dg-warning "moving a temporary object prevents copy elision" } + B b2(std::move (A())); // { dg-warning "moving a temporary object prevents copy elision" } + B b3{std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + B b4 = {std::move (A())}; // { dg-warning "moving a temporary object prevents copy elision" } + + X x1 = std::move (X()); // { dg-warning "moving a temporary object prevents copy elision" } + X x2 = std::move (X{}); // { dg-warning "moving a temporary object prevents copy elision" } + X x3(std::move (X())); // { dg-warning "moving a temporary object prevents copy elision" } + X x4(std::move (X{})); // { dg-warning "moving a temporary object prevents copy elision" } + X x5{std::move (X())}; // { dg-warning "moving a temporary object prevents copy elision" } + X x6{std::move (X{})}; // { dg-warning "moving a temporary object prevents copy elision" } + X x7 = {std::move (X())}; // { dg-warning "moving a temporary object prevents copy elision" } + X x8 = {std::move (X{})}; // { dg-warning "moving a temporary object prevents copy elision" } + + foo (std::move (A())); // { dg-warning "moving a temporary object prevents copy elision" } + foo (std::move (A{})); // { dg-warning "moving a temporary object prevents copy elision" } + bar (std::move (X())); // { dg-warning "moving a temporary object prevents copy elision" } + bar (std::move (X{})); // { dg-warning "moving a temporary object prevents copy elision" } + + foo (std::move (a1)); + bar (std::move (x1)); +} -- 2.11.4.GIT