user1725794
user1725794

Reputation: 267

Issue passing std::unique_ptr's

Been stuck on this code for the past hour, still trying to get my head around smart pointers and implementing them but this issue has had my stumped for quite a bit.

void GameState::addEntity(std::unique_ptr<Entity> gameObject)
{
    if(gameObject->isCollidable()){
        _actors.Add(gameObject);
    } else {
        _props.Add(gameObject);
    }
}

// This is the method the above function is trying to call.
void GameObjectManager::Add(std::unique_ptr<Entity> gameObject)
{
    _gameObjects.insert(std::make_pair(ID, std::move(gameObject)));
    ID++;
}

The error message I'm reciving is;

'std::unique_ptr<_Ty>::unique_ptr' : cannot access private member declared in class 'std::unique_ptr<_Ty>'  

Upvotes: 0

Views: 295

Answers (3)

Andy Prowl
Andy Prowl

Reputation: 126562

You are trying to pass a unique_ptr lvalue to a function which takes a unique_ptr by value, which would result in creating a copy of that pointer: this is intentionally forbidden. Unique pointers are meant to model unique ownership: this means that the entity that holds a unique pointer has the ownership of the pointed object.

Copying a unique pointer around would mean having several owning pointers spread all over the system: this would of course defeat the purpose itself of unique ownership, which explains why you can't copy unique pointers.

What you can do is to transfer this ownership, so if you call a function that accepts a unique pointer, you could move the unique_ptr when you pass it as an argument to that function:

_actors.Add(std::move(gameObject));
//          ^^^^^^^^^

Notice, that transferring ownership by moving away from a unique pointer means that the caller function is left with a "zombie" unique pointer object which is no longer owning the object it previously pointed to: therefore, the caller function should not try to dereference it anymore.

Upvotes: 1

meyumer
meyumer

Reputation: 5064

You cannot pass a unique_ptr by value. Following are trying to create copies of a unique_ptr, which is forbidden:

if(gameObject->isCollidable()){
    _actors.Add(gameObject);     // THIS LINE
} else {
    _props.Add(gameObject);      // and THIS LINE
}

A solution is passing with std::move():

if(gameObject->isCollidable()){
    _actors.Add(std::move(gameObject));
//              ^^^^^^^^^
} else {
    _props.Add(std::move(gameObject));
//              ^^^^^^^^^
}

This will result in two ownership transfers. One to the std::unique_ptr<Entity> which is the input parameter of GameObjectManager::Add and one to the std::pair in _gameObjects.

Another solution is modifying the signature of GameObjectManager::Add to get a reference:

void GameObjectManager::Add(std::unique_ptr<Entity> & gameObject)
//                                                 ^^^

Now, you can call the method as you are currently doing, and this will result in only a single ownership transfer (to the pair in _gameObjects).

But, as pointed out by @Xeo in the comments the second option violates the best practice rule-of-thumb which is: Ownership transfer should be explicit at all points.

Upvotes: 1

Joseph Mansfield
Joseph Mansfield

Reputation: 110768

You need to pass ownership from addEntity to Add by using std::move:

if(gameObject->isCollidable()){
    _actors.Add(std::move(gameObject));
} else {
    _props.Add(std::move(gameObject));
}

You cannot pass a unique_ptr without explicitly allowing it to be moved from. This is what makes it a unique pointer. If you could copy it, both the original and the copy would be pointing to the same object. When you move from it, the original pointer gives up ownership to the new one.

Upvotes: 3

Related Questions