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?
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" }
+}
