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