PR c++/86981, Implement -Wpessimizing-move.
authorMarek Polacek <polacek@redhat.com>
Tue, 21 Aug 2018 15:38:36 +0000 (15:38 +0000)
committerMarek Polacek <mpolacek@gcc.gnu.org>
Tue, 21 Aug 2018 15:38:36 +0000 (15:38 +0000)
* c.opt (Wpessimizing-move): New option.

* typeck.c (decl_in_std_namespace_p): New.
(is_std_move_p): New.
(maybe_warn_pessimizing_move): New.
(can_do_nrvo_p): New, factored out of ...
(check_return_expr): ... here.  Warn about potentially harmful
std::move in a return statement.

* doc/invoke.texi: Document -Wpessimizing-move.

* g++.dg/cpp0x/Wpessimizing-move1.C: New test.
* g++.dg/cpp0x/Wpessimizing-move2.C: New test.
* g++.dg/cpp0x/Wpessimizing-move3.C: New test.
* g++.dg/cpp0x/Wpessimizing-move4.C: New test.
* g++.dg/cpp1z/Wpessimizing-move1.C: New test.

From-SVN: r263741

12 files changed:
gcc/ChangeLog
gcc/c-family/ChangeLog
gcc/c-family/c.opt
gcc/cp/ChangeLog
gcc/cp/typeck.c
gcc/doc/invoke.texi
gcc/testsuite/ChangeLog
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move1.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move2.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move4.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp1z/Wpessimizing-move1.C [new file with mode: 0644]

index c856a935e8b6e60f5c579102c0bc5e698b3a4b9b..264d183f818ada35b45859c7149a09f4e84bdae3 100644 (file)
@@ -1,3 +1,8 @@
+2018-08-21  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/86981, Implement -Wpessimizing-move.
+       * doc/invoke.texi: Document -Wpessimizing-move.
+
 2018-08-21  Jan Hubicka  <jh@suse.cz>
 
        * tree.c (find_decls_types_r): Do not check for redundant typedefs.
index aa52815cc751b29498b9fe782ac81405a6b06287..790df09eb595f110cd20cdade4e81e750a01a09d 100644 (file)
@@ -1,3 +1,8 @@
+2018-08-21  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/86981, Implement -Wpessimizing-move.
+       * c.opt (Wpessimizing-move): New option.
+
 2018-08-20  David Malcolm  <dmalcolm@redhat.com>
 
        PR other/84889
index 9980bfac11c640b3aff9e364ab82a3b0079d73ee..76840dd77addee0f3db384e23cbc751aaf3bd6a0 100644 (file)
@@ -937,6 +937,10 @@ Wpedantic
 C ObjC C++ ObjC++ CPP(cpp_pedantic) CppReason(CPP_W_PEDANTIC) Warning
 ; Documented in common.opt
 
+Wpessimizing-move
+C++ ObjC++ Var(warn_pessimizing_move) Warning LangEnabledBy(C++ ObjC++, Wall)
+Warn about calling std::move on a local object in a return statement preventing copy elision.
+
 Wpmf-conversions
 C++ ObjC++ Var(warn_pmf2ptr) Init(1) Warning
 Warn when converting the type of pointers to member functions.
index 44be980985dc1c256fd5d9dbea2cf64193b502b6..6e6724428bf1fce33cb9a412756bce9382a1af32 100644 (file)
@@ -1,5 +1,13 @@
 2018-08-21  Marek Polacek  <polacek@redhat.com>
 
+       PR c++/86981, Implement -Wpessimizing-move.
+       * typeck.c (decl_in_std_namespace_p): New.
+       (is_std_move_p): New.
+       (maybe_warn_pessimizing_move): New.
+       (can_do_nrvo_p): New, factored out of ...
+       (check_return_expr): ... here.  Warn about potentially harmful
+       std::move in a return statement.
+
        PR c++/65043
        * call.c (standard_conversion): Set check_narrowing.
        * typeck2.c (check_narrowing): Use CP_INTEGRAL_TYPE_P rather
index 99be38ed8f862c3674ee5270411c711e33726f11..122d9dcd4b3c923976a7390c28a9b8dd43b5715a 100644 (file)
@@ -9126,6 +9126,100 @@ maybe_warn_about_returning_address_of_local (tree retval)
   return false;
 }
 
+/* Returns true if DECL is in the std namespace.  */
+
+static bool
+decl_in_std_namespace_p (tree decl)
+{
+  return (decl != NULL_TREE
+         && DECL_NAMESPACE_STD_P (decl_namespace_context (decl)));
+}
+
+/* Returns true if FN, a CALL_EXPR, is a call to std::move.  */
+
+static bool
+is_std_move_p (tree fn)
+{
+  /* std::move only takes one argument.  */
+  if (call_expr_nargs (fn) != 1)
+    return false;
+
+  tree fndecl = cp_get_callee_fndecl_nofold (fn);
+  if (!decl_in_std_namespace_p (fndecl))
+    return false;
+
+  tree name = DECL_NAME (fndecl);
+  return name && id_equal (name, "move");
+}
+
+/* Returns true if RETVAL is a good candidate for the NRVO as per
+   [class.copy.elision].  FUNCTYPE is the type the function is declared
+   to return.  */
+
+static bool
+can_do_nrvo_p (tree retval, tree functype)
+{
+  tree result = DECL_RESULT (current_function_decl);
+  return (retval != NULL_TREE
+         && !processing_template_decl
+         /* Must be a local, automatic variable.  */
+         && VAR_P (retval)
+         && DECL_CONTEXT (retval) == current_function_decl
+         && !TREE_STATIC (retval)
+         /* And not a lambda or anonymous union proxy.  */
+         && !DECL_HAS_VALUE_EXPR_P (retval)
+         && (DECL_ALIGN (retval) <= DECL_ALIGN (result))
+         /* 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)))
+         /* And the returned value must be non-volatile.  */
+         && !TYPE_VOLATILE (TREE_TYPE (retval)));
+}
+
+/* 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.  */
+
+static void
+maybe_warn_pessimizing_move (tree retval, tree functype)
+{
+  if (!warn_pessimizing_move)
+    return;
+
+  /* C++98 doesn't know move.  */
+  if (cxx_dialect < cxx11)
+    return;
+
+  /* This is only interesting for class types.  */
+  if (!CLASS_TYPE_P (functype))
+    return;
+
+  /* We're looking for *std::move<T&> (&arg).  */
+  if (REFERENCE_REF_P (retval)
+      && TREE_CODE (TREE_OPERAND (retval, 0)) == CALL_EXPR)
+    {
+      tree fn = TREE_OPERAND (retval, 0);
+      if (is_std_move_p (fn))
+       {
+         tree arg = CALL_EXPR_ARG (fn, 0);
+         STRIP_NOPS (arg);
+         if (TREE_CODE (arg) == ADDR_EXPR)
+           arg = TREE_OPERAND (arg, 0);
+         /* Warn if we could do copy elision were it not for the move.  */
+         if (can_do_nrvo_p (arg, functype))
+           {
+             auto_diagnostic_group d;
+             if (warning_at (location_of (retval), OPT_Wpessimizing_move,
+                             "moving a local object in a return statement "
+                             "prevents copy elision"))
+               inform (location_of (retval), "remove %<std::move%> call");
+           }
+       }
+    }
+}
+
 /* Check that returning RETVAL from the current function is valid.
    Return an expression explicitly showing all conversions required to
    change RETVAL into the function return type, and to assign it to
@@ -9143,7 +9237,6 @@ check_return_expr (tree retval, bool *no_warning)
      the declared type is incomplete.  */
   tree functype;
   int fn_returns_value_p;
-  bool named_return_value_okay_p;
 
   *no_warning = false;
 
@@ -9355,24 +9448,7 @@ check_return_expr (tree retval, bool *no_warning)
 
      See finish_function and finalize_nrv for the rest of this optimization.  */
 
-  named_return_value_okay_p = 
-    (retval != NULL_TREE
-     && !processing_template_decl
-     /* Must be a local, automatic variable.  */
-     && VAR_P (retval)
-     && DECL_CONTEXT (retval) == current_function_decl
-     && ! TREE_STATIC (retval)
-     /* And not a lambda or anonymous union proxy.  */
-     && !DECL_HAS_VALUE_EXPR_P (retval)
-     && (DECL_ALIGN (retval) <= DECL_ALIGN (result))
-     /* 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)))
-     /* And the returned value must be non-volatile.  */
-     && ! TYPE_VOLATILE (TREE_TYPE (retval)));
-     
+  bool named_return_value_okay_p = can_do_nrvo_p (retval, functype);
   if (fn_returns_value_p && flag_elide_constructors)
     {
       if (named_return_value_okay_p
@@ -9388,6 +9464,9 @@ check_return_expr (tree retval, bool *no_warning)
   if (!retval)
     return NULL_TREE;
 
+  if (!named_return_value_okay_p)
+    maybe_warn_pessimizing_move (retval, functype);
+
   /* Do any required conversions.  */
   if (retval == result || DECL_CONSTRUCTOR_P (current_function_decl))
     /* No conversions are required.  */
index 99849ec6467b607dd637469cc4d453a30361077f..e4148297a87ae53cda0ea8b124e40c873c539cf1 100644 (file)
@@ -231,6 +231,7 @@ in the following sections.
 -Wdelete-non-virtual-dtor -Wdeprecated-copy  -Wliteral-suffix @gol
 -Wmultiple-inheritance @gol
 -Wnamespaces  -Wnarrowing @gol
+-Wpessimizing-move @gol
 -Wnoexcept  -Wnoexcept-type  -Wclass-memaccess @gol
 -Wnon-virtual-dtor  -Wreorder  -Wregister @gol
 -Weffc++  -Wstrict-null-sentinel  -Wtemplates @gol
@@ -3132,6 +3133,31 @@ The compiler rearranges the member initializers for @code{i}
 and @code{j} to match the declaration order of the members, emitting
 a warning to that effect.  This warning is enabled by @option{-Wall}.
 
+@item -Wno-pessimizing-move @r{(C++ and Objective-C++ only)}
+@opindex Wpessimizing-move
+@opindex Wno-pessimizing-move
+This warning warns when a call to @code{std::move} prevents copy
+elision.  A typical scenario when copy elision can occur is when returning in
+a function with a class return type, when the expression being returned is the
+name of a non-volatile automatic object, and is not a function parameter, and
+has the same type as the function return type.
+
+@smallexample
+struct T @{
+@dots{}
+@};
+T fn()
+@{
+  T t;
+  @dots{}
+  return std::move (t);
+@}
+@end smallexample
+
+But in this example, the @code{std::move} call prevents copy elision.
+
+This warning is enabled by @option{-Wall}.
+
 @item -fext-numeric-literals @r{(C++ and Objective-C++ only)}
 @opindex fext-numeric-literals
 @opindex fno-ext-numeric-literals
@@ -4037,6 +4063,7 @@ Options} and @ref{Objective-C and Objective-C++ Dialect Options}.
 -Wnonnull-compare  @gol
 -Wopenmp-simd @gol
 -Wparentheses  @gol
+-Wpessimizing-move @r{(only for C++)}  @gol
 -Wpointer-sign  @gol
 -Wreorder   @gol
 -Wrestrict   @gol
index 7b54bc0a8681b9b03194de04130beb770d51d9d7..6c87f8017d3569e1784980e7d154ee97773400a7 100644 (file)
@@ -1,5 +1,12 @@
 2018-08-21  Marek Polacek  <polacek@redhat.com>
 
+       PR c++/86981, Implement -Wpessimizing-move.
+       * g++.dg/cpp0x/Wpessimizing-move1.C: New test.
+       * g++.dg/cpp0x/Wpessimizing-move2.C: New test.
+       * g++.dg/cpp0x/Wpessimizing-move3.C: New test.
+       * g++.dg/cpp0x/Wpessimizing-move4.C: New test.
+       * g++.dg/cpp1z/Wpessimizing-move1.C: New test.
+
        PR c++/65043
        * g++.dg/concepts/pr67595.C: Add dg-warning.
        * g++.dg/cpp0x/Wnarrowing11.C: New test.
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move1.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move1.C
new file mode 100644 (file)
index 0000000..858bed6
--- /dev/null
@@ -0,0 +1,132 @@
+// PR c++/86981
+// { 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 T {
+  T() { }
+  T(const T&) { }
+  T(T&&) { }
+};
+struct U {
+  U() { }
+  U(const U&) { }
+  U(U&&) { }
+  U(T) { }
+};
+
+T g;
+
+T
+fn1 ()
+{
+  T t;
+  return std::move (t); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+}
+
+T
+fn2 ()
+{
+  // Not a local variable.
+  return std::move (g);
+}
+
+int
+fn3 ()
+{
+  int i = 42;
+  // Not a class type.
+  return std::move (i);
+}
+
+T
+fn4 (bool b)
+{
+  T t;
+  if (b)
+    throw std::move (t);
+  return std::move (t); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+}
+
+T
+fn5 (T t)
+{
+  // Function parameter; std::move is redundant but not pessimizing.
+  return std::move (t);
+}
+
+U
+fn6 (T t, U u, bool b)
+{
+  if (b)
+    return std::move (t);
+  else
+    // Function parameter; std::move is redundant but not pessimizing.
+    return std::move (u);
+}
+
+U
+fn6 (bool b)
+{
+  T t;
+  U u;
+  if (b)
+    return std::move (t);
+  else
+    return std::move (u); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+}
+
+T
+fn7 ()
+{
+  static T t;
+  // Non-local; don't warn.
+  return std::move (t);
+}
+
+T
+fn8 ()
+{
+  return T();
+}
+
+T
+fn9 (int i)
+{
+  T t;
+
+  switch (i)
+    {
+    case 1:
+      return std::move ((t)); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+    case 2:
+      return (std::move (t)); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+    default:
+      return (std::move ((t))); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+    }
+}
+
+int
+fn10 ()
+{
+  return std::move (42);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move2.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move2.C
new file mode 100644 (file)
index 0000000..0ee6e05
--- /dev/null
@@ -0,0 +1,14 @@
+// PR c++/86981
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wpessimizing-move" }
+
+#include <string>
+#include <tuple>
+#include <utility>
+
+std::tuple<std::string, std::string>
+foo ()
+{
+  std::pair<std::string, std::string> p;
+  return std::move (p);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move3.C
new file mode 100644 (file)
index 0000000..a1af123
--- /dev/null
@@ -0,0 +1,59 @@
+// PR c++/86981
+// { 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 T { };
+struct U { U(T); };
+
+template<typename Tp>
+T
+fn1 ()
+{
+  T t;
+  // Non-dependent type.
+  return std::move (t); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+}
+
+template<typename Tp1, typename Tp2>
+Tp1
+fn2 ()
+{
+  Tp2 t;
+  return std::move (t); // { dg-warning "moving a local object in a return statement prevents copy elision" }
+}
+
+template<typename Tp1, typename Tp2>
+Tp1
+fn3 ()
+{
+  Tp2 t;
+  return std::move (t);
+}
+
+int
+main ()
+{
+  fn1<T>();
+  fn2<T, T>();
+  fn3<U, T>();
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move4.C b/gcc/testsuite/g++.dg/cpp0x/Wpessimizing-move4.C
new file mode 100644 (file)
index 0000000..59e148e
--- /dev/null
@@ -0,0 +1,46 @@
+// PR c++/86981
+// { 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 T { };
+
+T
+fn1 ()
+{
+  T t;
+  return (1, std::move (t));
+}
+
+T
+fn2 ()
+{
+  T t;
+  return [&](){ return std::move (t); }();
+}
+
+T
+fn3 ()
+{
+  T t;
+  return [=](){ return std::move (t); }();
+}
diff --git a/gcc/testsuite/g++.dg/cpp1z/Wpessimizing-move1.C b/gcc/testsuite/g++.dg/cpp1z/Wpessimizing-move1.C
new file mode 100644 (file)
index 0000000..5974188
--- /dev/null
@@ -0,0 +1,18 @@
+// PR c++/86981
+// { dg-options "-Wpessimizing-move -std=c++17" }
+
+#include <utility>
+#include <optional>
+
+struct T {
+  T() { }
+  T(const T&) { }
+  T(T&&) { }
+};
+
+std::optional<T>
+fn ()
+{
+  T t;
+  return std::move (t);
+}