[gcc r13-2045] c++: Extend -Wpessimizing-move for class prvalues [PR106276]

[gcc r13-2045] c++: Extend -Wpessimizing-move for class prvalues [PR106276]

[Marek Polacek]

https://gcc.gnu.org/g:ae3459cd5956fcd29e8aa5408efb707cb7d9b14f

commit r13-2045-gae3459cd5956fcd29e8aa5408efb707cb7d9b14f
Author: Marek Polacek <polacek@redhat.com>
Date:   Fri Jul 22 18:10:30 2022 -0400

    c++: Extend -Wpessimizing-move for class prvalues [PR106276]
    
    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()));
    
            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.

Diff:
---
 gcc/cp/typeck.cc                                | 31 ++++++++++--
 gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C | 63 +++++++++++++++++++++++++
 2 files changed, 91 insertions(+), 3 deletions(-)

diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 6e4f23af982..15548e5091f 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
+	  && !glvalue_p (retval)
+	  && 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"))
+		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 *);
+};