Returning by rvalue reference vs returning by value

Question

Most C++ standard library utilities return by rvalue reference when overloaded on a rvalue qualifier for this. For example std::optional has the following overloads for the value() function

constexpr T& value() &;
constexpr const T & value() const &;
constexpr T&& value() &&;
constexpr const T&& value() const &&;

This allows the returned value to be moved from when needed, good. This is a solid optimization.

But what about the uncertainty associated with the returning of an rvalue? For example (live example here https://wandbox.org/permlink/kUqjfOWWRP6N57eS)

auto get_vector() {
    auto vector = std::vector<int>{1, 2, 3};
    return std::optional{std::move(vector)};
}

int main() {
    for (auto ele : *get_vector()) {
        cout << ele << endl;
    }
}

The code above causes undefined behavior because of how the range based for loop is expanded

{
    auto && __range = range_expression ; 
    auto __begin = begin_expr ;
    auto __end = end_expr ;
    for ( ; __begin != __end; ++__begin) { 
        range_declaration = *__begin; 
        loop_statement 
    } 
}

The forwarding reference range when binding to the return value of *get_vector() does not extend the lifetime of the xvalue. And results in binding to a destroyed value. And therefore results in UB.

Why not return by value and internally move the stored object? Especially because now C++17 has the prvalue optimization, for example

auto lck = std::lock_guard{mtx};

Note that this is not the same as this question here C++11 rvalues and move semantics confusion (return statement), this does not mention the lifetime extension problem with rvalue returns with container/holders and was asked way before C++17 had mandatory elision for prvalues

That's a general problem with range-based for loops where the expression is an xvalue. For what it's worth, there's a proposal for C++20 to allow for (auto vec_op = get_vector(); auto ele : *vec_op) that would allow a correct version of this iteration with only marginally more syntax. — Kerrek SB
@kerreksb I imagine thought that there is other code out there that is a little like the current expansion. And in that case returning be value works better. Why not change the standard library components to return by value? — Curious
@KerrekSB also will that new expansion not cause a copy when the expression is an lvalue? — Curious
To address both questions: I suppose reusable library components are composed by the user at the user's discretion, so a) why pessimize a general component unconditionally because you have found one use that you don't like, and b) you wouldn't unconditionally use the new syntax: you would use it if and when you need it (just like most code writing). (I guess you could stick auto&& into the initializer to cover both cases.) — Kerrek SB
@KerrekSB How does changing that to a auto&& solve the problem here? Won't lifetime extension still not apply in that case> — Curious

Nicol Bolas Nicol Bolas · Accepted Answer · 2017-09-24T18:18:20

Why not return by value and internally move the stored object?

Because that could be less efficient than returning a reference. Consider a case where you use the returned reference to fetch another reference from within that object. Like for example (*get_vector())[3]. With your proposed change, that is a reference to a copy of the original; the way it is currently, it is a reference to a value in the original temporary.

C++ as a language doesn't deal well effectively with the lifetime of references to temporaries. The solutions currently consist of either being careful about lifetimes, or not using references and having potentially slower/less efficient code. The standard library, in general, prefers to err on the side of performance.

Returning by rvalue reference vs returning by value

1 Answers