]> git.ipfire.org Git - thirdparty/gcc.git/commitdiff
c++: Extend -Wpessimizing-move to other contexts
authorMarek Polacek <polacek@redhat.com>
Mon, 1 Aug 2022 21:02:23 +0000 (17:02 -0400)
committerMarek Polacek <polacek@redhat.com>
Wed, 17 Aug 2022 16:35:45 +0000 (12:35 -0400)
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
gcc/cp/cp-tree.h
gcc/cp/decl.cc
gcc/cp/typeck.cc
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move7.C
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move8.C [new file with mode: 0644]

index 01a7be100778d0255d2f00ee0c9d96e5e4eb68d2..370137ebd6d39f069f98f987e1362f6b521dfe57 100644 (file)
@@ -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
index cb23975484fad72f85b8a284a82b33fa25d50ea7..9f2ff3728b417116812b1a987f535ff6747fa7ae 100644 (file)
@@ -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);
index 031be45e0be27b80c7dcdcc4b051fdb60a9d295d..84a1a01134116474245d73deeb0d9938ca3bdd3e 100644 (file)
@@ -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);
 }
 
index 6b38c8c7a0053b3cf833fde3b5805d51bbfaef6c..19bb7f74321db4eaf755cdec1b93544973eda8a8 100644 (file)
@@ -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&> ((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 %<std::move%> 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 %<std::move%> 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))
index cd4eaa09aae56bfdb38f1d67d69841eb3d158484..a17c7a87b7dd236ea6a8745283d971bed037e4de 100644 (file)
@@ -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 <typename T1, typename T2>
 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 (file)
index 0000000..51406c8
--- /dev/null
@@ -0,0 +1,65 @@
+// 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); };
+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));
+}