4

In Effective Modern C++, Item 12, Scott Meyers writes the following class to show how useful overloading member functions on the reference qualifiers can be:

class Widget {
public:
    using DataType = std::vector<double>;
    …
    DataType& data() &            // for lvalue Widgets
    { return values; }            // return lvalue

    DataType data() &&            // for rvalue Widgets
    { return std::move(values); } // return rvalue
    …
private:
    DataType values;
};

This seems clear: now non_temp_obj.data() will call the first overload and return a reference to a member of an object which is still alive afterwards, whereas make_temp_obj().data() returns by value a member of an object which dies as soon as that expression is done.

Here's my first question: as regards the && overload, why return std::move(values); and not just return values;, considering we are returning by value?

In the errata, however, Meyers writes

A better way to have the rvalue reference overload of the data member function return an rvalue is to have it return an rvalue reference. That would avoid the creation of a temporary object for the return value, and it would be consistent with the by-reference return of the original data interface near the top of page 84.

which I interpret as suggesting to change

    DataType data() &&
    { return std::move(values); }

to

    DataType&& data() &&
    { return std::move(values); }

but I don't understand the reason, especially in light of this answer which pretty much convinces me that the book version is correct and the errata is wrong.

So my second question is: who's right?

Enlico
  • 23,259
  • 6
  • 48
  • 102

1 Answers1

4

values is an object member and an lvalue, so if you just return values directly, it will be copied to the return value, not moved. The point of the && ref-qualified overload is to avoid making an unnecessary copy. return std::move(values) accomplishes this by casting values to an rvalue, so that it gets moved from instead of copied.

For the second part of your question: both have their advantages and disadvantages. As the answer you linked notes, returning by value from the && overload avoids lifetime issues, since the returned object will have its lifetime extended if a reference is immediately bound to it. On the other hand, returning by value could destroy the value of values unexpectedly. For instance:

DataType Widget::data() &&
{ return std::move(values); }

void func() {
    Widget foo;
    std::move(foo).data(); // Moves-constructs a temporary from
                           // foo::value, which is then immediately
                           // destroyed.
    auto bar = foo.data(); // Oops, foo::value was already moved from
                           // above and its value is likely gone.
}
Miles Budnek
  • 28,216
  • 2
  • 35
  • 52
  • For the first point, shouldn't RVO kick in and avoid copy as well as move? – Enlico Nov 03 '20 at 22:31
  • No, RVO only applies to function local variables, not object members. Similarly, function local variables are automatically moved from when returned, even if RVO doesn't apply, but again that doesn't apply to object members. – Miles Budnek Nov 03 '20 at 22:34
  • 3
    Not sure why this is unexpected. `std::move(foo)` explicitly says that the object can be moved from, so I would expect that the state is changed after that expression, otherwise I would not `std::move`. – Jens Nov 03 '20 at 22:43
  • IMO, the main cons of returning by value is the extra move constructor (which isn't necessary cheap, as for `std::array`). – Jarod42 Nov 04 '20 at 00:22
  • @MilesBudnek, how would returning by `DataType&&` in your example change what happens? Is it that `std::move(foo).data();` would be creating and destroying a _reference_ to `foo::value`, thus leaving the latter intact when you define `bar`? – Enlico Nov 05 '20 at 22:07
  • @Enrico Correct. Returning a reference would mean that no new object gets constructed as part of that call, and so there's nothing to steal any resources owned by `foo.value`. – Miles Budnek Nov 06 '20 at 00:15
  • ```[[nodiscard]]``` attribute somewhat mitigates the issue of unexpected destroying – erzya Nov 15 '21 at 14:22