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
– Kerrek SB, Commented Sep 24, 2017 at 16:48
@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
– Curious, Commented Sep 24, 2017 at 16:56
@KerrekSB also will that new expansion not cause a copy when the expression is an lvalue? — Curious
– Curious, Commented Sep 24, 2017 at 17:09
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
– Kerrek SB, Commented Sep 24, 2017 at 17:26
@KerrekSB How does changing that to a auto&& solve the problem here? Won't lifetime extension still not apply in that case> — Curious
– Curious, Commented Sep 24, 2017 at 23:03

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

6

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.

answered Sep 24, 2017 at 18:18

Nicol Bolas

481k66 gold badges861 silver badges1.1k bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

Curious Over a year ago

Sorry. A little confused. Why will that be a reference to a copy of the original? The internal vector will be moved.

Nicol Bolas Over a year ago

The contents of the vector will be moved, but the vector itself won't be the same object. Also, not every object has cheaper moves than copies.

Curious Over a year ago

But where is the copy? And so if I understand correctly, you are saying returning an rvalue reference is more efficient when you don't actually need the move, is that correct?

Nicol Bolas Over a year ago

@Curious: When you don't need the move yet. That is, as part of an expression. For member subobjects, temporary.member_name is an xvalue. Therefore, if you're using something like get or optional::value or similar things that get a conceptual member of the object, then you should get an xvalue too if the object is a temporary.

Collectives™ on Stack Overflow

Returning by rvalue reference vs returning by value

1 Answer 1

4 Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

4 Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related