return std::move(m_field) or return m_field?

Question

I have read in some others posts that move is unnecessary and even deoptimizes mechanisms like NRVO. But it was in most cases about a local field.

Consider this example:

• A class stores a temporary field after a computation.
• I want to take this field without copying it ("consuming" it).

In this case, is it appropriate to use std::move on return? Specifically, are these pieces of code equivalent? The second is more "generic", letting the user decide if the result should be moved or not, and the first one is more strict — any call will consume the result.

Option 1

// Option 1

#include <utility>

template<typename T>
class TemporaryResultHolder
{
public:
    void doTask() { result = T(); }

    T getResult() { return std::move(result); }
    // Or return T&& ?

private:
    T result;
};

int main() {
    TemporaryResultHolder<int> holder;
    holder.doTask();
    int res = holder.getResult();

    return 0;
}

Option 2

// Option 2

#include <utility>

template<typename T>
class TemporaryResultHolder
{
public:
    void doTask() { result = T(); }

    T getResult() { return result; }

private:
    T result;
};

int main() {
    TemporaryResultHolder<int> holder;
    holder.doTask();
    int res = std::move(holder.getResult());

    return 0;
}

Ketch

For T getResult() { return std::move(result); }, how do you guarantee that you don't use result again (except for re-assignment, or destruction)? – Dean

"read in some others posts that it's unecessarily and even deoptimize mecanisms like NRVO." That doesn't apply here. [N]RVO happens when you return a function-local variable. – Hairbrush

For int res = std::move(holder.getResult());, the move is redundant since the result is already an rvalue. – Dean

It was brief for the example but i did not included others things like a boolean value to check if a value is hold, for example – Ketch

@Dean I'm not sure that it's necessary to guarantee that. It can be a post condition of getResult(). – Poitiers

@Dean We never guarantee such a thing for moved-from objects. – Rhody

@AsteroidsWithWings • the OP could use a separate bool indicating whether the T is in a valid state, or in a valid-but-unspecified state because may have been moved from. Or to phrase it another way, example #1 is a very bad idiom. – Dean

@Dean They could, but I'm not aware of literally even one library that does that. – Rhody

@AsteroidsWithWings • I have seen a temporary result holder class for a return error code that if the result isn't looked at (and isn't a "no error" condition), the destructor will throw an exception. That's somewhat similar to having a member variable that has been moved from so it could be in a valid-but-unspecified state, such that if it member variable is accessed a second time it could throw or assert or some such. Seems like an unnecessarily brittle design though. – Dean

The reasoning was that if you want to access the value, you want to take it, letting the place for another value. The value come from anothers threads and this class would for example produce value to the chain when it's possible. – Ketch

Answer 1

If you were to return an object with automatic storage, then you should never return with std::move because former is faster in best case, and equal in worst case.

I have read in some others posts that it's unecessarily and even deoptimize mecanisms like NRVO.

And this is the reason. This doesn't however apply to the function in the question, because it doesn't return an object with automatic storage, but rather a member variable. NRVO cannot apply to a member variable, nor does the implicit move of return value.

So, in your example, return std::move(result); does a move, and return result; does a copy. You should use move if the purpose of the function is to move from the member, and copy if the purpose is to copy the member. Note however that moving from a member in a function that is not rvalue qualified is quite unconventional design, and I recommend to avoid it unless you have a good reason to do so. Also note that the copying version can be const qualified, which would make it more generally useful.

P.S. for int that is used in the example, these distinctions are irrelevant. Moving versus coping is only relevant to non-trivially copyable / -movable types.

Handel

Answer 2

Yes, std::move(result) is appropriate here, as long as the intent is exactly that: to "steal" the value from the object and pass ownership of it to the calling scope.

I'd suggest, though, either:

1. Rename the function takeResult(), to show what it's doing.

2. Or qualify the function itself with a &&, for added safety: then you can only use this function when the object itself is referred to through an rvalue

T getResult() && { return std::move(result); }
// ...
holder.doTask();
int res = std::move(holder).getResult();

Rhody

Even returning rvalue refence is not sufficient documentation in my opinion. Definitely needs a comment. Rvalue qualification approach is safe though. – Handel

@Handel This should definitely be discussed in the type's documentation, yes of course 👍 – Rhody

Answer 3

A "destructive" getResult is non-idiomatic, so you should document it. Specifying the return type as T&& is great for that! This way, even if the implementation is hidden in a .cpp-file, the developer who reads your code will understand this particular detail.

If you return by value, the value will be copied, no matter what. Even if you move it later, it already was copied, so your second option wastes resources (assuming instances of T are expensive, of course).

Another option is returning by reference (const T&). No copying here either, so you should consider that too.

Rubenrubens