This patch fixes a bogus -Wredundant-move warning.  In the test in the PR
the std::move call isn't redundant; the testcase won't actually compile
without that call, as per the resolution of bug 87150.

Before this patch, we'd issue the redundant-move warning anytime
treat_lvalue_as_rvalue_p is true for a std::move's argument.  But we also
need to make sure that convert_for_initialization works even without the
std::move call, if not, it's not redundant.

Trouble arises when the argument is const.  Then it might be the case that
the implicit rvalue fails because it uses a const copy constructor, or
that the type of the returned object and the type of the selected ctor's
parameter aren't the same.  To handle various corner cases (e.g. when the
std::move actually makes a difference between const T& and const T&&) I had
to use another convert_for_initialization, but the first one is the important
one and should handle most of the cases.

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

2019-03-04  Marek Polacek  <pola...@redhat.com>

        PR c++/87378 - bogus -Wredundant-move warning.
        * typeck.c (maybe_warn_pessimizing_move): See if the maybe-rvalue
        overload resolution would actually succeed.

        * g++.dg/cpp0x/Wredundant-move7.C: New test.
        * g++.dg/cpp0x/Wredundant-move8.C: New test.

diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 1bf9ad88141..7a43ba70010 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9429,10 +9429,43 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
             do maybe-rvalue overload resolution even without std::move.  */
          else if (treat_lvalue_as_rvalue_p (arg, /*parm_ok*/true))
            {
-             auto_diagnostic_group d;
-             if (warning_at (loc, OPT_Wredundant_move,
-                             "redundant move in return statement"))
-               inform (loc, "remove %<std::move%> call");
+             /* Make sure that the overload resolution would actually succeed
+                if we removed the std::move call.  */
+             tree moved = move (arg);
+             tree t = convert_for_initialization (NULL_TREE, functype, moved,
+                                                  (LOOKUP_NORMAL
+                                                   | LOOKUP_ONLYCONVERTING
+                                                   | LOOKUP_PREFER_RVALUE),
+                                                  ICR_RETURN, NULL_TREE, 0,
+                                                  tf_none);
+             /* If this worked, implicit rvalue would work, so the call to
+                std::move is redundant.  */
+             if (t == error_mark_node
+                 && CP_TYPE_CONST_P (TREE_TYPE (arg)))
+               {
+                 /* But if ARG is const, it may be the case of using
+                    const T& instead of T&&.  */
+                 t = convert_for_initialization (NULL_TREE, functype, moved,
+                                                 (LOOKUP_NORMAL
+                                                  | LOOKUP_ONLYCONVERTING),
+                                                 ICR_RETURN, NULL_TREE, 0,
+                                                 tf_none);
+                 /* Unless, rarely, using std::move would actually choose
+                    const T && over const T&.  In that case std::move isn't
+                    redundant.  */
+                 if (TREE_CODE (t) == TARGET_EXPR)
+                   t = TARGET_EXPR_INITIAL (t);
+                 tree call = cp_get_callee_fndecl_nofold (t);
+                 if (call && DECL_MOVE_CONSTRUCTOR_P (call))
+                   t = error_mark_node;
+               }
+             if (t != error_mark_node)
+               {
+                 auto_diagnostic_group d;
+                 if (warning_at (loc, OPT_Wredundant_move,
+                                 "redundant move in return statement"))
+                   inform (loc, "remove %<std::move%> call");
+               }
            }
        }
     }
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C 
gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
new file mode 100644
index 00000000000..015d7c4f7a4
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move7.C
@@ -0,0 +1,59 @@
+// PR c++/87378
+// { 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 S1 { S1(S1 &&); };
+struct S2 : S1 {};
+
+S1
+f (S2 s)
+{
+  return std::move(s); // { dg-bogus "redundant move in return statement" }
+}
+
+struct R1 {
+  R1(R1 &&);
+  R1(const R1 &&);
+};
+struct R2 : R1 {};
+
+R1
+f2 (const R2 s)
+{
+  return std::move(s); // { dg-bogus "redundant move in return statement" }
+}
+
+struct T1 {
+  T1(const T1 &);
+  T1(T1 &&);
+  T1(const T1 &&);
+};
+struct T2 : T1 {};
+
+T1
+f3 (const T2 s)
+{
+  // Without std::move: const T1 &
+  // With std::move: const T1 &&
+  return std::move(s); // { dg-bogus "redundant move in return statement" }
+}
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C 
gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C
new file mode 100644
index 00000000000..6d34e55522e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move8.C
@@ -0,0 +1,63 @@
+// PR c++/87378
+// { 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 S1 {
+  S1(S1&&) { }
+};
+
+S1 f1(S1 s)
+{
+  return std::move(s); // { dg-warning "redundant move in return statement" }
+}
+
+struct S2 {
+  S2(const S2&) { }
+  S2(S2&&) { }
+};
+
+S2 f2(S2 s)
+{
+  return std::move(s); // { dg-warning "redundant move in return statement" }
+}
+
+S2 f2c(const S2 s)
+{
+  return std::move(s); // { dg-warning "redundant move in return statement" }
+}
+
+struct S3 {
+  S3(const S3&) { }
+  S3(S3&&) { }
+  S3(const S3&&) { }
+};
+
+S3 f3(S3 s)
+{
+  return std::move(s); // { dg-warning "redundant move in return statement" }
+}
+
+S3 f3c(const S3 s)
+{
+  return std::move(s); // { dg-warning "redundant move in return statement" }
+}

Reply via email to