PR c++/87029, Implement -Wredundant-move.
authorMarek Polacek <polacek@redhat.com>
Sun, 26 Aug 2018 16:45:51 +0000 (16:45 +0000)
committerMarek Polacek <mpolacek@gcc.gnu.org>
Sun, 26 Aug 2018 16:45:51 +0000 (16:45 +0000)
* c.opt (Wredundant-move): New option.

* typeck.c (treat_lvalue_as_rvalue_p): New function.
(maybe_warn_pessimizing_move): Call convert_from_reference.
Warn about redundant moves.

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

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

From-SVN: r263863

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/Wredundant-move1.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C [new file with mode: 0644]
gcc/testsuite/g++.dg/cpp0x/Wredundant-move4.C [new file with mode: 0644]

index b183818918e8d75ba793b8c7866707ea4fb3e657..386dc8e886a5631027da776414d1ac9ac4044032 100644 (file)
@@ -1,3 +1,8 @@
+2018-08-26  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/87029, Implement -Wredundant-move.
+       * doc/invoke.texi: Document -Wredundant-move.
+
 2018-08-25  Martin Sebor  <msebor@redhat.com>
 
        PR tree-optimization/87059
index 790df09eb595f110cd20cdade4e81e750a01a09d..f833d0a17221d2b0c054c5653e65f5b7590b3502 100644 (file)
@@ -1,3 +1,8 @@
+2018-08-26  Marek Polacek  <polacek@redhat.com>
+
+       PR c++/87029, Implement -Wredundant-move.
+       * c.opt (Wredundant-move): New option.
+
 2018-08-21  Marek Polacek  <polacek@redhat.com>
 
        PR c++/86981, Implement -Wpessimizing-move.
index 76840dd77addee0f3db384e23cbc751aaf3bd6a0..31a2b972919eb7e96899fc88e695f59444968875 100644 (file)
@@ -985,6 +985,10 @@ Wredundant-decls
 C ObjC C++ ObjC++ Var(warn_redundant_decls) Warning
 Warn about multiple declarations of the same object.
 
+Wredundant-move
+C++ ObjC++ Var(warn_redundant_move) Warning LangEnabledBy(C++ ObjC++,Wextra)
+Warn about redundant calls to std::move.
+
 Wregister
 C++ ObjC++ Var(warn_register) Warning
 Warn about uses of register storage specifier.
index 258c6a9b76313faf8ee55d131c44ad84b0121e3a..f1ae08e6f7da12a9206da1170c5a3e6c2e41c5af 100644 (file)
@@ -3,6 +3,11 @@
        PR c++/87080
        * typeck.c (maybe_warn_pessimizing_move): Do nothing in a template.
 
+       PR c++/87029, Implement -Wredundant-move.
+       * typeck.c (treat_lvalue_as_rvalue_p): New function.
+       (maybe_warn_pessimizing_move): Call convert_from_reference.
+       Warn about redundant moves.
+
 2018-08-24  Marek Polacek  <polacek@redhat.com>
 
        PR c++/67012
index 24647e29a5547bb4ea8b299dd25ed7604113c51e..fa3ba7156984bc333d315aa1d1925dc04db7bbaa 100644 (file)
@@ -9178,6 +9178,19 @@ can_do_nrvo_p (tree retval, tree functype)
          && !TYPE_VOLATILE (TREE_TYPE (retval)));
 }
 
+/* Returns true if we should treat RETVAL, an expression being returned,
+   as if it were designated by an rvalue.  See [class.copy.elision].  */
+
+static bool
+treat_lvalue_as_rvalue_p (tree retval)
+{
+  return ((cxx_dialect != cxx98)
+         && ((VAR_P (retval) && !DECL_HAS_VALUE_EXPR_P (retval))
+             || TREE_CODE (retval) == PARM_DECL)
+         && DECL_CONTEXT (retval) == current_function_decl
+         && !TREE_STATIC (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.  */
@@ -9185,9 +9198,11 @@ can_do_nrvo_p (tree retval, tree functype)
 static void
 maybe_warn_pessimizing_move (tree retval, tree functype)
 {
-  if (!warn_pessimizing_move)
+  if (!(warn_pessimizing_move || warn_redundant_move))
     return;
 
+  location_t loc = cp_expr_loc_or_loc (retval, input_location);
+
   /* C++98 doesn't know move.  */
   if (cxx_dialect < cxx11)
     return;
@@ -9212,14 +9227,24 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
          STRIP_NOPS (arg);
          if (TREE_CODE (arg) == ADDR_EXPR)
            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))
            {
              auto_diagnostic_group d;
-             if (warning_at (location_of (retval), OPT_Wpessimizing_move,
+             if (warning_at (loc, OPT_Wpessimizing_move,
                              "moving a local object in a return statement "
                              "prevents copy elision"))
-               inform (location_of (retval), "remove %<std::move%> call");
+               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 (treat_lvalue_as_rvalue_p (arg))
+           {
+             auto_diagnostic_group d;
+             if (warning_at (loc, OPT_Wredundant_move,
+                             "redundant move in return statement"))
+               inform (loc, "remove %<std::move%> call");
            }
        }
     }
@@ -9499,11 +9524,7 @@ check_return_expr (tree retval, bool *no_warning)
          Note that these conditions are similar to, but not as strict as,
         the conditions for the named return value optimization.  */
       bool converted = false;
-      if ((cxx_dialect != cxx98)
-          && ((VAR_P (retval) && !DECL_HAS_VALUE_EXPR_P (retval))
-             || TREE_CODE (retval) == PARM_DECL)
-         && DECL_CONTEXT (retval) == current_function_decl
-         && !TREE_STATIC (retval)
+      if (treat_lvalue_as_rvalue_p (retval)
          /* This is only interesting for class type.  */
          && CLASS_TYPE_P (functype))
        {
index e4148297a87ae53cda0ea8b124e40c873c539cf1..985ef9f3510fc61f8cfd63c1ec36d0eaa221daca 100644 (file)
@@ -231,7 +231,7 @@ in the following sections.
 -Wdelete-non-virtual-dtor -Wdeprecated-copy  -Wliteral-suffix @gol
 -Wmultiple-inheritance @gol
 -Wnamespaces  -Wnarrowing @gol
--Wpessimizing-move @gol
+-Wpessimizing-move  -Wredundant-move @gol
 -Wnoexcept  -Wnoexcept-type  -Wclass-memaccess @gol
 -Wnon-virtual-dtor  -Wreorder  -Wregister @gol
 -Weffc++  -Wstrict-null-sentinel  -Wtemplates @gol
@@ -3158,6 +3158,49 @@ But in this example, the @code{std::move} call prevents copy elision.
 
 This warning is enabled by @option{-Wall}.
 
+@item -Wno-redundant-move @r{(C++ and Objective-C++ only)}
+@opindex Wredundant-move
+@opindex Wno-redundant-move
+This warning warns about redundant calls to @code{std::move}; that is, when
+a move operation would have been performed even without the @code{std::move}
+call.  This happens because the compiler is forced to treat the object as if
+it were an rvalue in certain situations such as returning a local variable,
+where copy elision isn't applicable.  Consider:
+
+@smallexample
+struct T @{
+@dots{}
+@};
+T fn(T t)
+@{
+  @dots{}
+  return std::move (t);
+@}
+@end smallexample
+
+Here, the @code{std::move} call is redundant.  Because G++ implements Core
+Issue 1579, another example is:
+
+@smallexample
+struct T @{ // convertible to U
+@dots{}
+@};
+struct U @{
+@dots{}
+@};
+U fn()
+@{
+  T t;
+  @dots{}
+  return std::move (t);
+@}
+@end smallexample
+In this example, copy elision isn't applicable because the type of the
+expression being returned and the function return type differ, yet G++
+treats the return value as if it were designated by an rvalue.
+
+This warning is enabled by @option{-Wextra}.
+
 @item -fext-numeric-literals @r{(C++ and Objective-C++ only)}
 @opindex fext-numeric-literals
 @opindex fno-ext-numeric-literals
@@ -4112,6 +4155,7 @@ name is still supported, but the newer name is more descriptive.)
 -Wold-style-declaration @r{(C only)}  @gol
 -Woverride-init  @gol
 -Wsign-compare @r{(C only)} @gol
+-Wredundant-move @r{(only for C++)}  @gol
 -Wtype-limits  @gol
 -Wuninitialized  @gol
 -Wshift-negative-value @r{(in C++03 and in C99 and newer)}  @gol
index 3bd02e71eedda9edf9de2fdd2258f2b909003d52..69bc12da284354c04ef8e262ed8849d11c4161cd 100644 (file)
@@ -3,6 +3,12 @@
        PR c++/87080
        * g++.dg/cpp0x/Wpessimizing-move5.C: New test.
 
+       PR c++/87029, Implement -Wredundant-move.
+       * g++.dg/cpp0x/Wredundant-move1.C: New test.
+       * g++.dg/cpp0x/Wredundant-move2.C: New test.
+       * g++.dg/cpp0x/Wredundant-move3.C: New test.
+       * g++.dg/cpp0x/Wredundant-move4.C: New test.
+
 2018-08-25  Thomas Koenig  <tkoenig@gcc.gnu.org>
 
        PR libfortran/86704
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C
new file mode 100644 (file)
index 0000000..5d4a25d
--- /dev/null
@@ -0,0 +1,106 @@
+// PR c++/87029
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-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
+fn1 (T t)
+{
+  return t;
+}
+
+T
+fn2 (T t)
+{
+  // Will use move even without std::move.
+  return std::move (t); // { dg-warning "redundant move in return statement" }
+}
+
+T
+fn3 (const T t)
+{
+  // t is const: will decay into copy.
+  return t;
+}
+
+T
+fn4 (const T t)
+{
+  // t is const: will decay into copy despite std::move, so it's redundant.
+  return std::move (t); // { dg-warning "redundant move in return statement" }
+}
+
+int
+fn5 (int i)
+{
+  // Not a class type.
+  return std::move (i);
+}
+
+T
+fn6 (T t, bool b)
+{
+  if (b)
+    throw std::move (t);
+  return std::move (t); // { dg-warning "redundant move in return statement" }
+}
+
+U
+fn7 (T t)
+{
+  // Core 1579 means we'll get a move here.
+  return t;
+}
+
+U
+fn8 (T t)
+{
+  // Core 1579 means we'll get a move here.  Even without std::move.
+  return std::move (t);  // { dg-warning "redundant move in return statement" }
+}
+
+T
+fn9 (T& t)
+{
+  // T is a reference and the move isn't redundant.
+  return std::move (t);
+}
+
+T
+fn10 (T&& t)
+{
+  // T is a reference and the move isn't redundant.
+  return std::move (t);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move2.C
new file mode 100644 (file)
index 0000000..f181afe
--- /dev/null
@@ -0,0 +1,57 @@
+// PR c++/87029
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-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 "redundant move in return statement" }
+}
+
+template<typename Tp1, typename Tp2>
+Tp1
+fn2 (Tp2 t)
+{
+  return std::move (t); // { dg-warning "redundant move in return statement" }
+}
+
+template<typename Tp1, typename Tp2>
+Tp1
+fn3 (Tp2 t)
+{
+  return std::move (t); // { dg-warning "redundant move in return statement" }
+}
+
+int
+main ()
+{
+  T t;
+  fn1<T>(t);
+  fn2<T, T>(t);
+  fn3<U, T>(t);
+}
diff --git a/gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move3.C
new file mode 100644 (file)
index 0000000..7084134
--- /dev/null
@@ -0,0 +1,43 @@
+// PR c++/87029
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-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/cpp0x/Wredundant-move4.C b/gcc/testsuite/g++.dg/cpp0x/Wredundant-move4.C
new file mode 100644 (file)
index 0000000..aa89e46
--- /dev/null
@@ -0,0 +1,86 @@
+// PR c++/87029
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wredundant-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) { }
+};
+
+U
+fn1 (T t, bool b)
+{
+  if (b)
+    return t;
+  else
+    return std::move (t); // { dg-warning "redundant move in return statement" }
+}
+
+U
+fn2 (bool b)
+{
+  T t;
+  if (b)
+    return t;
+  else
+    return std::move (t); // { dg-warning "redundant move in return statement" }
+}
+
+U
+fn3 (bool b)
+{
+  static T t;
+  if (b)
+    return t;
+  else
+    return std::move (t);
+}
+
+T g;
+
+U
+fn4 (bool b)
+{
+  if (b)
+    return g;
+  else
+    return std::move (g);
+}
+
+long int
+fn5 (bool b)
+{
+  int i = 42;
+  if (b)
+    return i;
+  else
+    return std::move (i);
+}