5

Let's suppose that I have struct Foo with move constructor and operator=(Foo&&), and I used it as data member:

Foo f()
{
  Foo foo;
  //code
  return foo; 
}
struct Boo {
Foo foo;
Boo() {
  foo = f();//1
  foo = std::move(f());//2
}
};

In case (2) I actually not need std::move, but what if I used it here, does this make something bad, like preventing optimization?

I read this: Why does std::move prevent RVO?

and find out that changing return foo; to return std::move(foo); cause disabling of RVO, but what about (2) does it cause the similar situation? And if so, why?

Community
  • 1
  • 1
user1244932
  • 7,352
  • 5
  • 46
  • 103
  • 7
    Copy elision wouldn't apply in this case anyway because you are calling `foo.operator=` . It'd be relevant if you had `Foo foo = std::move(f());` which is initialization. – M.M Nov 20 '15 at 02:19
  • 1
    @M.M But `clang 3.7` warn about this, so I wonder, is it bug in warning generation, or I missed something – user1244932 Nov 20 '15 at 02:25
  • 1
    It can be bad for reasons that aren't performance reasons too. In your case for #2, you care calling std::move(f()) on something that is already a rhr, so the move is wasted characters. My rule of thumb is that you should avoid std::move unless you have to, and you only have to when you are transferring ownership in a non-trivial way. – IdeaHat Nov 20 '15 at 02:27
  • So far as I know the proper approach to `std::move` is using it when transferring data to a subroutine, not from. But I am also waiting for the answer ;) – Dean Seo Nov 20 '15 at 02:40
  • @Galik You're right, but in case 2 is it a 'bad' approach to explicitly tell the compiler to move? (although it is already ready to be moved), if so why would it be bad? – Dean Seo Nov 20 '15 at 02:47

1 Answers1

6

It's redundant and confusing. Just because I can write std::add_pointer_t<void> instead of void*, or std::add_lvalue_reference_t<Foo> (or Foo bitand) instead of Foo&, doesn't mean I should.

It also matters in other contexts:

auto&& a = f(); // OK, reference binding to a temporary extends its lifetime
auto&& b = std::move(f()); // dangling

and so, if Foo is something that can be iterated over,

for(const auto& p : f()) {} // OK
for(const auto& p : std::move(f())) {} // UB

And in your example, if the assignment operator is implemented as copy-and-swap (operator=(Foo)), then foo = std::move(f()) forces a non-elidable move, while foo = f() can elide the move from f()'s return value to the argument of operator=.

T.C.
  • 133,968
  • 17
  • 288
  • 421
  • In that first example, `b` does NOT seem dangling when compiled by `VS2013`. (Haven't tested with any other compiler yet). Could you please provide a little more information on that? – Dean Seo Nov 20 '15 at 06:01
  • @DeanSeo Neither [clang](http://coliru.stacked-crooked.com/a/607b0d720af30b9e) nor [gcc](http://coliru.stacked-crooked.com/a/698d5c5271c8f1c8) will extend the lifetime of the temporary. The former will even emit some nice performance warnings. – Brandlingo Feb 17 '16 at 12:17
  • @MatthäusBrandl I don't think `std::move(f())` is the temporary yet, but it'll likely be moved very soon as an xvalue. So it's not yet moved ... ? (meaning not dangling). I posted this a long time ago so maybe I need some time to catch up. – Dean Seo Feb 17 '16 at 12:37
  • @DeanSeo I realized but figured you might be interested. I guess after the call to `std::move()` the result of `f()` may be destructed legally before passing the reference on to `b`. Seems like an evaluation order problem. – Brandlingo Feb 17 '16 at 12:51
  • @MatthäusBrandl Woah, didn't think of that! I will get a little deeper about that later. – Dean Seo Feb 17 '16 at 13:37