On 7/27/22 17:14, Marek Polacek wrote:
We already have a warning that warns about pessimizing std::move
in a return statement, when it prevents the NRVO:
T fn()
{
T t;
return std::move (t); // warning \o/
}
However, the warning doesn't warn when what we are returning is a class
prvalue, that is, when std::move prevents the RVO:
T fn()
{
T t;
return std::move (T{}); // no warning :-(
}
This came up recently in GCC:
<https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598177.html>.
This patch fixes that. I would like to extend the warning further, so
that it warns in more contexts, e.g.:
T t = std::move(T());
or
void foo (T);
foo (std::move(T()));
Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
PR c++/106276
gcc/cp/ChangeLog:
* typeck.cc (can_do_rvo_p): New.
(maybe_warn_pessimizing_move): Warn when moving a temporary object
in a return statement prevents copy elision.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/Wpessimizing-move7.C: New test.
---
gcc/cp/typeck.cc | 31 ++++++++-
.../g++.dg/cpp0x/Wpessimizing-move7.C | 63 +++++++++++++++++++
2 files changed, 91 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 6e4f23af982..9500c4e2fe8 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10287,12 +10287,29 @@ can_do_nrvo_p (tree retval, tree functype)
/* The cv-unqualified type of the returned value must be the
same as the cv-unqualified return type of the
function. */
- && same_type_p ((TYPE_MAIN_VARIANT (TREE_TYPE (retval))),
- (TYPE_MAIN_VARIANT (functype)))
+ && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (retval)),
+ TYPE_MAIN_VARIANT (functype))
/* And the returned value must be non-volatile. */
&& !TYPE_VOLATILE (TREE_TYPE (retval)));
}
+/* Like can_do_nrvo_p, but we check if we're trying to move a class
+ prvalue. */
+
+static bool
+can_do_rvo_p (tree retval, tree functype)
+{
+ if (functype == error_mark_node)
+ return false;
+ if (retval)
+ STRIP_ANY_LOCATION_WRAPPER (retval);
+ return (retval != NULL_TREE
+ && TREE_CODE (retval) == TARGET_EXPR
Maybe use !glvalue_p instead of specifically checking for TARGET_EXPR?
I don't feel strongly about this.
+ && same_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (retval)),
+ TYPE_MAIN_VARIANT (functype))
+ && !TYPE_VOLATILE (TREE_TYPE (retval)));
+}
+
/* If we should treat RETVAL, an expression being returned, as if it were
designated by an rvalue, returns it adjusted accordingly; otherwise,
returns
NULL_TREE. See [class.copy.elision]. RETURN_P is true if this is a return
@@ -10401,12 +10418,20 @@ maybe_warn_pessimizing_move (tree retval, tree
functype)
"prevents copy elision"))
inform (loc, "remove %<std::move%> call");
}
+ else if (can_do_rvo_p (arg, functype))
+ {
+ auto_diagnostic_group d;
+ if (warning_at (loc, OPT_Wpessimizing_move,
+ "moving a temporary object in a return statement "
+ "prevents copy elision"))
It doesn't just prevent copy elision, it produces a dangling reference.
This is a special case of the warning we talked about passing a
temporary to a function that returns a reference argument unchanged, and
should probably use a different warning flag.
+ inform (loc, "remove %<std::move%> call");
+ }
/* Warn if the move is redundant. It is redundant when we would
do maybe-rvalue overload resolution even without std::move. */
else if (warn_redundant_move
&& (moved = treat_lvalue_as_rvalue_p (arg, /*return*/true)))
{
- /* Make sure that the overload resolution would actually succeed
+ /* Make sure that overload resolution would actually succeed
if we removed the std::move call. */
tree t = convert_for_initialization (NULL_TREE, functype,
moved,
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
new file mode 100644
index 00000000000..cd4eaa09aae
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
@@ -0,0 +1,63 @@
+// PR c++/106276
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wpessimizing-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 A { A(); A(const A&) = delete; A(A&&); };
+struct B { B(A); };
+
+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" }
+}
+
+B fn2 ()
+{
+ return std::move (A());
+ return std::move (A{});
+ return std::move (foo ());
+}
+
+template <typename T1, typename T2>
+T1
+fn3 ()
+{
+ return std::move (T2{}); // { dg-warning "moving a temporary object in a return
statement prevents copy elision" }
+}
+
+void
+do_fn3 ()
+{
+ fn3<A, A>();
+ fn3<B, A>();
+}
+
+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(char *);
+};
base-commit: 219f86495791fd27b012678f13e10cada211daab
