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

user_5110937 0
Viewed 302

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;
}
move-semantics c++
3 Answers

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.

user_20793030

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();
user_43862780

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.

user_5098680
Related