6

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

Curious
  • 20,870
  • 8
  • 61
  • 146
  • 2
    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 Sep 24 '17 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 Sep 24 '17 at 16:56
  • @KerrekSB also will that new expansion not cause a copy when the expression is an lvalue? – Curious Sep 24 '17 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 Sep 24 '17 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 Sep 24 '17 at 23:03
  • I meant that you can use `for (auto&& x = f(); auto el : *x)` and not worry whether `f` returns an lvalue or a prvalue. – Kerrek SB Sep 24 '17 at 23:36

1 Answers1

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.

Nicol Bolas
  • 449,505
  • 63
  • 781
  • 982
  • Sorry. A little confused. Why will that be a reference to a copy of the original? The internal vector will be moved. – Curious Sep 24 '17 at 18:25
  • 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. – Nicol Bolas Sep 24 '17 at 18:28
  • 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? – Curious Sep 24 '17 at 18:30
  • @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. – Nicol Bolas Sep 24 '17 at 18:41