On Mon, Jan 07, 2019 at 04:59:14PM -0500, Jason Merrill wrote:
> On 1/7/19 4:29 PM, Marek Polacek wrote:
> > This patch fixes bogus -Wredundant-move warnings reported in 88692 and
> > 87882.
> >
> > To quickly recap, this warning is supposed to warn for cases like
> >
> > struct T { };
> >
> > T fn(T t)
> > {
> > return std::move (t);
> > }
> >
> > where NRVO isn't applicable for T because it's a parameter, but it's
> > a local variable and we're returning, so C++11 says activate move
> > semantics, so the std::move is redundant. But, as these testcases show,
> > we're failing to realize that that is not the case when returning *this,
> > which is disguised as an ordinary PARM_DECL, and treat_lvalue_as_rvalue_p
> > was fooled by that.
>
> Hmm, the function isn't returning 'this', it's returning '*this'. I guess
> what's happening is that in order to pass *this to the reference parameter
> of move, we end up converting it from pointer to reference by NOP_EXPR, and
> the STRIP_NOPS in maybe_warn_pessimizing_move throws that away so that it
> then thinks we're returning 'this'. I expect the same thing could happen
> with any parameter of pointer-to-class type.
You're right, I didn't realize that we warned even for parameters of
pointer-to-class
types. So why don't we disable the warning for PARM_DECLs with pointer types?
Bootstrapped/regtested on x86_64-linux, ok for trunk?
2019-01-11 Marek Polacek <[email protected]>
PR c++/88692 - -Wredundant-move false positive with *this.
* typeck.c (maybe_warn_pessimizing_move): Don't issue Wredundant-move
warnings for variables of pointer types.
* g++.dg/cpp0x/Wredundant-move5.C: New test.
* g++.dg/cpp0x/Wredundant-move6.C: New test.
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index e399cd3fe45..2b26e49f676 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9426,7 +9426,8 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
}
/* 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, /*parm_ok*/true))
+ else if (!POINTER_TYPE_P (TREE_TYPE (arg))
+ && treat_lvalue_as_rvalue_p (arg, /*parm_ok*/true))
{
auto_diagnostic_group d;
if (warning_at (loc, OPT_Wredundant_move,
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
new file mode 100644
index 00000000000..0e2ec46d11e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
@@ -0,0 +1,53 @@
+// PR c++/88692
+// { 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 X {
+ X f() && {
+ return std::move(*this); // { dg-bogus "redundant move in return
statement" }
+ }
+
+ X f2() & {
+ return std::move(*this); // { dg-bogus "redundant move in return
statement" }
+ }
+
+ X f3() {
+ return std::move(*this); // { dg-bogus "redundant move in return
statement" }
+ }
+};
+
+struct S { int i; int j; };
+
+struct Y {
+ S f1 (S s) {
+ return std::move (s); // { dg-warning "redundant move in return statement"
}
+ }
+
+ S f2 (S* s) {
+ return std::move (*s); // { dg-bogus "redundant move in return statement" }
+ }
+
+ S f3 (S** s) {
+ return std::move (**s); // { dg-bogus "redundant move in return statement"
}
+ }
+};
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move6.C
gcc/testsuite/g++.dg/cpp0x/Wredundant-move6.C
new file mode 100644
index 00000000000..5808a78638e
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move6.C
@@ -0,0 +1,43 @@
+// PR c++/87882
+// { 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 Foo {
+ Foo Bar() {
+ return std::move(*this); // { dg-bogus "redundant move in return
statement" }
+ }
+ Foo Baz() {
+ return *this;
+ }
+ int i;
+};
+
+void Move(Foo & f)
+{
+ f = Foo{}.Bar();
+}
+
+void NoMove(Foo & f)
+{
+ f = Foo{}.Baz();
+}
