public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Extend -Wpessimizing-move to other contexts
@ 2022-08-02 23:04 Marek Polacek
  2022-08-16 12:27 ` Marek Polacek
  2022-08-16 19:23 ` Jason Merrill
  0 siblings, 2 replies; 7+ messages in thread
From: Marek Polacek @ 2022-08-02 23:04 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill

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.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	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 ++++++++++++-----
 .../g++.dg/cpp0x/Wpessimizing-move7.C         | 16 ++---
 .../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 3278b4114bd..5a8af22b509 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -8101,6 +8101,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 70ad681467e..dc6853a7de1 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 9500c4e2fe8..2650beb780e 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -10368,17 +10368,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)
@@ -10390,14 +10390,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);
@@ -10409,20 +10427,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");
 	    }
@@ -10433,7 +10455,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
@@ -10718,7 +10740,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 <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
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<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));
+}

base-commit: 502605a277d36cee1b0570982a16d97a43eace67
prerequisite-patch-id: f4862bc588ce5fed1d21016fecc4b61feb71eba5
-- 
2.37.1


^ permalink raw reply	[flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-08-22 21:02 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-02 23:04 [PATCH] c++: Extend -Wpessimizing-move to other contexts Marek Polacek
2022-08-16 12:27 ` Marek Polacek
2022-08-22 11:48   ` Stephan Bergmann
2022-08-22 21:02     ` Marek Polacek
2022-08-16 19:23 ` Jason Merrill
2022-08-16 21:09   ` Marek Polacek
2022-08-17 16:11     ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).