rvalue references break when deep-returning

Question

I've encountered a problem when passing returned rvalue references from a depth of more than 1.

struct Data {
 std :: vector <int> data;
 Data () {
  data .push_back (1);
 };

 Data (Data && d)
 : data (std :: move (d .data))
 {}
};

Data && foo () {
 Data d;
 return std :: move (d);
}

Data && bar () {
 return std :: move (foo ()); // Crashes in autogenerated code
}

Data && baz () {
 return foo (); // Crashes in Data move constructor.
}

Data && bop () {
 Data d = foo ();
    return std :: move (d); // Crashes in autogenerated code.
}

int main () {
 Data d_foo = foo (); // This is fine.
 Data d_bar = bar (); // Crash.
 Data d_bar = baz (); // Crash.
 Data d_bop = bop (); // Crash.
}

I think the std::vector is being double-freed. I'm using g++ (Ubuntu/Linaro 4.4.4-14ubuntu5) 4.4.5

Does the above code work for you? Am I doing something wrong or is there a bug in the library or compiler?

If (heaven forfend) it's the compiler (there are other known C++0x bugs in gcc), can someone please tell me if there's an apt-safe way to upgrade or patch gcc on ubuntu? I've tried before but got into a tangle of unsupported packages.

Thanks a lot.

You should watch [this video](http://channel9.msdn.com/Shows/Going+Deep/C9-Lectures-Stephan-T-Lavavej-Standard-Template-Library-STL-9-of-n) on rvalue references. — fredoverflow, Jan 07 '11 at 21:14

score 6 · Answer 1 · answered Jan 07 '11 at 20:27

6

It rarely makes sense to return rvalue references from the function (exception std::move), because reference was presumably bound to a temporary, or an object on stack as in your case and when you returned it, the object is gone.

Edit:

Data && foo () {
 Data d;
 return std :: move (d);
}

d is destroyed when going out of scope, so you're returning dangling reference.

answered Jan 07 '11 at 20:27

Gene Bushuyev

5,512
20
19

As a follow-up, to take advantage of rvalue references here, just have the functions return by value. The compiler will automatically use the new move constructor to move the data out rather than copying it. – templatetypedef Jan 07 '11 at 20:29
1

@templatetypedef -- you are right about returning by value, except compiler will try to elide copy before resorting to moving the object. – Gene Bushuyev Jan 07 '11 at 20:31

score 5 · Accepted Answer · answered Jan 07 '11 at 20:33

You don't return an rvalue reference, you return a value, which is then taken by the caller as by reference as an rvalue. You should just have Data foo(), not Data&& foo(). The fact that any of those works is purely coincidence, as it is undefined behaviour.

UncleBens · Answer 3 · 2011-01-08T10:36:19.117

You can't expect any good from returning a && to a local object any more than from returning a regular reference / pointer to a local object.

You should return by value. Under C++03 it is a situation where the compiler is allowed to optimize away calls to copy constructor. In C++0x this means that (if the copying can't be optimized away - e.g compilers don't manage it when returning a different object from different branches) the move constructor will be invoked instead of a copy constructor (if available) to get the value out of the function.

As far as I can understand, move semantics is supposed to kick in automatically in places where it is safe, and rvalue reference parameter types allow you to determine if the argument is a rvalue (something you can explicitly move) or not.

rvalue references break when deep-returning

3 Answers3