Lucretiel
Lucretiel

Reputation: 3344

Returning an rvalue reference from a nonlocal

I have a class that is queried for an internal state object:

class State {...}; //Has a copy and move constructor
class Processor
{
private:
    std::unique_ptr<State> state;

public:
    void process(...)
    {
        State newState;
        ... //create this new state
        state.reset(new State(newState));
    }

    State getState()
    {
        return std::move(*state.release());
    }
};

Is this an appropriate use of std::move? I can guarantee that getState will only be called once per call to process, but because of the design of this particular system I can't just return newState from process. Many of the other answers on Stack Overflow and elsewhere said that it's better to just return the object, because the compiler will move it or RVO it if it can anyway, but those were all in the case that the returned object was local to the function.

I don't necessarily need the state object to be behind a unique_ptr, but that seemed like the easiest way to do manage the new state objects. My actual implementation has a pointer being transferred directly to the unique_ptr at the end.

Upvotes: 2

Views: 208

Answers (3)

arabesc
arabesc

Reputation: 319

You could just return the state whose type is std::unique_ptr<State>, it would be perfectly moved with std::move(state). In this case you are free from copying the whole state object when it's returned by value. And the state object would be automaticaly destroyed if it's not captured by the caller. It's quite efficient approach for complex heap-allocated objects. But be careful, by moving state you are clearing the state of the Processor object. So, the getState isn't a best name in this case, it should be like fetchState or popState.

Upvotes: 0

Lucretiel
Lucretiel

Reputation: 3344

It turns out that the original demo code is buggy- the unique_ptr never frees the pointer. The answer involves moving onto the local function space and then returning normally.

class State {...}; //Has a copy and move constructor
class Processor
{
private:
    std::unique_ptr<State> state;

public:
    void process(...)
    {
        State* newState;
        ... //newState is allocated on the heap somehow
        state.reset(newState);
    }

    State getState()
    {
        State _state(std::move(*state));
        //Optionally: state.reset();
        return _state;
    }
};

Upvotes: 1

pmr
pmr

Reputation: 59841

What's wrong with just returning state

struct state {};
class processor {
public:
  void process() {
    state_ = State();
  }

  state get_state() {
    return std::move(state_);
  }

private:
  state state_;
};

This will default-construct state_ when a processor is constructed but you can prevent that with an optional wrapper. You still have to guarantee that get_state is only called after process, which outright sucks.

Upvotes: 0

Related Questions