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
