On 3/5/19 4:53 PM, Marek Polacek wrote:
On Tue, Mar 05, 2019 at 03:50:30PM -0500, Jason Merrill wrote:
On 3/4/19 7:17 PM, Marek Polacek wrote:
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.
Indeed.
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.
So this is the case where std::move is redundant because doing overload
resolution on the lvalue would select the same constructor? I'm not sure
that's worth warning about, especially in templates where we don't know
anything about the return type.
Yes, it's about:
struct T {
T(const T&);
T(T&&);
};
T
f(const T t)
{
return t; // uses const T&
return std::move (t); // also uses const T&
}
I'm fine with dropping the warning in this (IMO fairly obscure) case; I
certainly didn't have this in mind when adding the warning.
So the patch can be simplified to the following:
Bootstrapped/regtested on x86_64-linux, ok for trunk?
OK.
2019-03-05 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-move1.C (fn4): Drop dg-warning.
* g++.dg/cpp0x/Wredundant-move7.C: New test.
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 1bf9ad88141..43ff3d63abd 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9429,10 +9429,24 @@ 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 t = convert_for_initialization (NULL_TREE, functype,
+ move (arg),
+ (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)
+ {
+ 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-move1.C
gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C
index 5d4a25dbc3b..e70f3cde625 100644
--- gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move1.C
@@ -59,7 +59,8 @@ 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" }
+ // We used to warn about this, but no longer since c++/87378.
+ return std::move (t);
}
int
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" }
+}
