C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Marek Polacek]

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.

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

2019-01-07  Marek Polacek  <polacek@redhat.com>

	PR c++/88692, c++/87882 - -Wredundant-move false positive with *this.
	* typeck.c (treat_lvalue_as_rvalue_p): Return false for 'this'.

	* 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..c6908d23a11 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9371,6 +9371,9 @@ bool
 treat_lvalue_as_rvalue_p (tree retval, bool parm_ok)
 {
   STRIP_ANY_LOCATION_WRAPPER (retval);
+  /* *this remains an lvalue expression.  */
+  if (is_this_parameter (retval))
+    return false;
   return ((cxx_dialect != cxx98)
 	  && ((VAR_P (retval) && !DECL_HAS_VALUE_EXPR_P (retval))
 	      || (parm_ok && TREE_CODE (retval) == PARM_DECL))
diff --git gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
new file mode 100644
index 00000000000..b6a3b2296a8
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp0x/Wredundant-move5.C
@@ -0,0 +1,37 @@
+// 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" }
+    }
+};
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();
+}

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Jason Merrill]

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.

Jason

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Marek Polacek]

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  <polacek@redhat.com>

	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();
+}

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Jason Merrill]

On 1/11/19 11:09 AM, Marek Polacek wrote:
> 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?

std::move is certainly redundant for parms of pointer type (or other 
scalar type), so we might still want to warn about 'return 
std::move(this)'.  The problem here is that we're discarding the 
indirection, so we aren't actually considering the returned expression.

Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a 
NOP_EXPR from pointer to reference.

Jason

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Marek Polacek]

On Fri, Jan 11, 2019 at 01:55:09PM -0500, Jason Merrill wrote:
> On 1/11/19 11:09 AM, Marek Polacek wrote:
> > 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?
> 
> std::move is certainly redundant for parms of pointer type (or other scalar
> type), so we might still want to warn about 'return std::move(this)'.  The
> problem here is that we're discarding the indirection, so we aren't actually
> considering the returned expression.

We won't warn for 'return std::move(this)' in any case becase
maybe_warn_pessimizing_move returns for non-class types.  This is in line with
what clang does.  I'm not sure if we should change that; I'd rather not.
 
> Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a NOP_EXPR
> from pointer to reference.

I think we need that in order to make can_do_nrvo_p/treat_lvalue_as_rvalue_p
work.  Given the outermost NOP_EXPR will always be of reference type, how about
just returning if, after STRIP_NOPS, we don't see an ADDR_EXPR?  That fixes the
testcases I've been meaning to fix and doesn't regress anything.  I.e., this:

--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
    {
      tree arg = CALL_EXPR_ARG (fn, 0);
      STRIP_NOPS (arg);
-     if (TREE_CODE (arg) == ADDR_EXPR)
-       arg = TREE_OPERAND (arg, 0);
+     if (TREE_CODE (arg) != ADDR_EXPR)
+       return;
+     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))

I can test/post the complete patch.  Or am I again missing something obvious? :)

Marek

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Jason Merrill]

On 1/11/19 4:21 PM, Marek Polacek wrote:
> On Fri, Jan 11, 2019 at 01:55:09PM -0500, Jason Merrill wrote:
>> On 1/11/19 11:09 AM, Marek Polacek wrote:
>>> 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?
>>
>> std::move is certainly redundant for parms of pointer type (or other scalar
>> type), so we might still want to warn about 'return std::move(this)'.  The
>> problem here is that we're discarding the indirection, so we aren't actually
>> considering the returned expression.
> 
> We won't warn for 'return std::move(this)' in any case becase
> maybe_warn_pessimizing_move returns for non-class types.  This is in line with
> what clang does.  I'm not sure if we should change that; I'd rather not.
>   
>> Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a NOP_EXPR
>> from pointer to reference.
> 
> I think we need that in order to make can_do_nrvo_p/treat_lvalue_as_rvalue_p
> work.  Given the outermost NOP_EXPR will always be of reference type, how about
> just returning if, after STRIP_NOPS, we don't see an ADDR_EXPR?  That fixes the
> testcases I've been meaning to fix and doesn't regress anything.  I.e., this:
> 
> --- a/gcc/cp/typeck.c
> +++ b/gcc/cp/typeck.c
> @@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
>      {
>        tree arg = CALL_EXPR_ARG (fn, 0);
>        STRIP_NOPS (arg);
> -     if (TREE_CODE (arg) == ADDR_EXPR)
> -       arg = TREE_OPERAND (arg, 0);
> +     if (TREE_CODE (arg) != ADDR_EXPR)
> +       return;
> +     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))
> 
> I can test/post the complete patch.  Or am I again missing something obvious? :)

That looks good.  OK if it passes.

Jason

Re: C++ PATCH for c++/88692 - -Wredundant-move false positive with *this

[Marek Polacek]

On Fri, Jan 11, 2019 at 04:23:23PM -0500, Jason Merrill wrote:
> On 1/11/19 4:21 PM, Marek Polacek wrote:
> > On Fri, Jan 11, 2019 at 01:55:09PM -0500, Jason Merrill wrote:
> > > On 1/11/19 11:09 AM, Marek Polacek wrote:
> > > > 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?
> > > 
> > > std::move is certainly redundant for parms of pointer type (or other scalar
> > > type), so we might still want to warn about 'return std::move(this)'.  The
> > > problem here is that we're discarding the indirection, so we aren't actually
> > > considering the returned expression.
> > 
> > We won't warn for 'return std::move(this)' in any case becase
> > maybe_warn_pessimizing_move returns for non-class types.  This is in line with
> > what clang does.  I'm not sure if we should change that; I'd rather not.
> > > Is the STRIP_NOPS really necessary?  It seems we shouldn't remove a NOP_EXPR
> > > from pointer to reference.
> > 
> > I think we need that in order to make can_do_nrvo_p/treat_lvalue_as_rvalue_p
> > work.  Given the outermost NOP_EXPR will always be of reference type, how about
> > just returning if, after STRIP_NOPS, we don't see an ADDR_EXPR?  That fixes the
> > testcases I've been meaning to fix and doesn't regress anything.  I.e., this:
> > 
> > --- a/gcc/cp/typeck.c
> > +++ b/gcc/cp/typeck.c
> > @@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
> >      {
> >        tree arg = CALL_EXPR_ARG (fn, 0);
> >        STRIP_NOPS (arg);
> > -     if (TREE_CODE (arg) == ADDR_EXPR)
> > -       arg = TREE_OPERAND (arg, 0);
> > +     if (TREE_CODE (arg) != ADDR_EXPR)
> > +       return;
> > +     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))
> > 
> > I can test/post the complete patch.  Or am I again missing something obvious? :)
> 
> That looks good.  OK if it passes.

Which it did, so I'm installing the following.  Thanks!

2019-01-11  Marek Polacek  <polacek@redhat.com>

	PR c++/88692, c++/87882 - -Wredundant-move false positive with *this.
	* typeck.c (maybe_warn_pessimizing_move): Return if ARG isn't
	ADDR_EXPR.

	* 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..43d2899a3c4 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9412,8 +9412,9 @@ maybe_warn_pessimizing_move (tree retval, tree functype)
 	{
 	  tree arg = CALL_EXPR_ARG (fn, 0);
 	  STRIP_NOPS (arg);
-	  if (TREE_CODE (arg) == ADDR_EXPR)
-	    arg = TREE_OPERAND (arg, 0);
+	  if (TREE_CODE (arg) != ADDR_EXPR)
+	    return;
+	  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))
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();
+}