Is this safe? Rvalue and reference return values

Question

#include <iostream>
#include <string>

using namespace std;

class Wrapper
{
public:
    std::string&& get() &&
    {
        std::cout << "rvalue" << std::endl;
        return std::move(str);
    }

    const std::string& get() const &
    {
        std::cout << "lvalue" << std::endl;
        return str;
    }
private:
    std::string str;
};

std::string foo()
{
    Wrapper wrap;
    return wrap.get();
}

std::string bar()
{
    Wrapper wrap;
    return std::move(wrap.get());
}

std::string fooRvalue()
{
    return Wrapper().get();
}

std::string barRvalue()
{
    return std::move(Wrapper().get());
}

int main() {
    while(1)
    {
        std::string s = foo();
        std::string s2 = bar();
        std::string s3 = fooRvalue();
        std::string s4 = barRvalue();
    }
    return 0;
}

Is the return value of const std::string&, and std::string&& safe in this use case (pretend std::string is some other type that may be not copyable). I thought it would not work because the refernce should point to some local variable which would go out of scope, but it appears to work just fine? Also i've seen the && syntax used before, did i use it correctly (i restricted it to when Wrapper is itself an rvalue).

Im just a little generally confused when returning a reference is safe, i thought the rule was that the object had to outlive the value being returned but i've seen things like the standard library and boost returing reference before. If i store the reference they give me in a variable (that is not a reference) it all seems to work just fine if i then return that stored variable. Can someone fix me up, and give me some good rules to follow.

Here's the ideone i was playing around with, it all seems to work: http://ideone.com/GyWQF6

Answer 1

Yes, this is a correct and safe way of implementing a member getter. But you could make one improvement, which I'll get to. The key is that the lifetime of the std::string member is (almost) the same as the lifetime of the Wrapper object which contains it. And a temporary is only destroyed at the end of the full-expression where it appears, not immediately after it's "used". So in return std::move(Wrapper().get());, the Wrapper is created, get() is called, std::move is called, the return value is constructed, and only then the Wrapper and its string are destroyed.

The only danger is if somebody binds another reference to your member. But that would be their mistake, and ordinary const-reference getters have the same danger. This is the equivalent of the incorrect const char* ptr = "oops"s.c_str();.

std::string foo()
{
    Wrapper wrap;
    return wrap.get();
}

foo copy constructs the return value from the member. Hopefully that's not a surprise.

std::string bar()
{
    Wrapper wrap;
    return std::move(wrap.get());
}

bar also copy constructs the return value from the member. What's going on here is that since wrap is an lvalue, the lvalue get() is called, which returns a const lvalue. (Informally, "const std::string&".) Then move converts that to an xvalue, but it's still const. (Informally, "const std::string&&".) The move constructor string(string&&) cannot match here, since the argument is const. But the copy constructor string(const string&) can match, and is called.

If you expected bar to move construct the return value, you may want to add a third overload for non-const lvalues:

std::string& get() & { return str; }

(If you didn't want to allow modifying the member, note that you already sort of did. For example, somebody could do std::move(wrap).get() = "something";.)

Also, if you had return std::move(wrap).get(); instead, that would have move constructed the return value even without the third overload.

std::string fooRvalue()
{
    return Wrapper().get();
}

fooRvalue move constructs the return value, since the rvalue get() is used. As already mentioned, the Wrapper lives long enough for this construction to be safe.

std::string barRvalue()
{
    return std::move(Wrapper().get());
}

In barRvalue, the move call is absolutely useless. The expression Wrapper().get() is already an xvalue, so move just converts an xvalue of type std::string to ... an xvalue of type std::string. Still just as safe, though.

Answer 2

The return value object of a function will be initialized before the destruction of local variables and temporaries created in the return expression. As such, it is perfectly legitimate to move from locals/temporaries into function return values.

Those four global functions work because they return objects, not references to objects. And that object gets constructed before any of the locals that their constructor parameter reference are destroyed.

barRvalue is rather redundant (since the result of Wrapper().get() is already an rvalue reference), but functional.

Im just a little generally confused when returning a reference is safe

A function returning a reference to an object that has been destroyed is what is unsafe. Returning a reference to a local object, or part of a local object, is unsafe.

Returning a reference to *this or some part of *this is safe, because the this object will outlive the function that returns the reference. It's no different from returning a reference to a parameter that you take by pointer/reference. The object will outlive the function call that returns the reference.