c++ swapping unique_ptr's

Question

New to this kind of stuff, probably doing something wrong, but -

I have 3 members

std::unique_ptr<Gun> currentWeapon;
std::unique_ptr<Gun> weaponSlotOne;
std::unique_ptr<Gun> weaponSlotTwo;

Gun is a base class that has other derived classes such as Pistol and SMG.

What i'm doing is setting weaponSlotOne and weaponSlotTwo to two different guns, then setting currentWeapon to the first weapon.

weaponSlotOne.reset(new DevPistol());
weaponSlotTwo.reset(new AutoDevPistol());
currentWeapon = std::move(weaponSlotOne);

and i have a switchWeapons method, that does this:

void Player::switchWeapons() {
    if(currentWeapon == weaponSlotOne) {
        currentWeapon = std::move(weaponSlotTwo);
    }
    else {
        currentWeapon = std::move(weaponSlotOne);
    }
}

which seems to destroy/deallocate both guns for some reason. i'm not quite sure what's going wrong.

Perhaps: http://en.cppreference.com/w/cpp/memory/unique_ptr/release ? — Galik, Nov 25 '14 at 01:53

score 2 · Answer 1 · answered Nov 25 '14 at 03:02

The problem is that after calling std::move on an object, the object is in an indeterminate state, and you can't safely do anything with the object other than destroy it or assign to it.

In your case, after doing currentWeapon = std::move(weaponSlotOne);, weaponSlotOne is indeterminate, so when you test currentWeapon == weaponSlotOne you might get any result. Probably, this will be false (weaponSlotOne will be null), so you'll just copy it to currentWeapon, dropping whatever was there (deleting it).

The question is, what are you trying to do? If you want two weapons, and want to keep track of which one is current, it might make more sense to do:

std::unique_ptr<Gun> *currentWeapon;
std::unique_ptr<Gun> weaponSlotOne;
std::unique_ptr<Gun> weaponSlotTwo;

weaponSlotOne.reset(new DevPistol());
weaponSlotTwo.reset(new AutoDevPistol());
currentWeapon = &weaponSlotOne;

void Player::switchWeapons() {
    if(currentWeapon == &weaponSlotOne) {
        currentWeapon = &weaponSlotTwo;
    }
    else {
        currentWeapon = &weaponSlotOne;
    }
}

Or even more simply:

std::unique_ptr<Gun> weaponSlot[2];
int currentWeapon = 0;

void Player::switchWeapons() {
    currentWeapon ^= 1;
}

I'll upvote this, but i've solved it by using swap, and changing the members to `currentWeapon` and `storedWeapon`. I think it's doing what i've wanted it to, it seems to be working. — Evan Ward, Nov 25 '14 at 03:03

Howard Hinnant · Answer 2 · 2014-11-26T03:04:42.780

In general, after moving from an object the moved-from object is in a valid but unspecified state. This means that you can only safely call those functions on the moved-from object which have no preconditions. For example destruction typically has no precondition. Typically neither does assigning to an object. And typically neither do const observers such as equality comparison with a non-moved from value.

In the case of std::unique_ptr, you can safely compare a moved-from value. But also note that unique_ptr has unique ownership semantics. I.e. two non-null unique_ptrs should never compare equal, because if they did, they would own the same pointer, and thus be in violation of the basic tenant of unique_ptr. But it often makes sense to compare unique_ptr with nullptr to find out if it owns a non-null pointer:

#include <cassert>
#include <memory>

int
main()
{
    std::unique_ptr<int> p(new int(3));
    auto p2 = std::move(p);
    assert(p == nullptr); // perfectly legal & practical use of moved-from value
}

I suspect the problem with your code is/was that you were mistakenly expecting copy semantics from the unique_ptr move assignment: i.e. that the source of the assignment would be left unchanged. However from my code snippet above it can be shown that the moved-from unique_ptr will be reliably left equal to nullptr. There simply is no other way to implement the operation while still satisfying all unique_ptr specifications.

score 0 · Answer 3 · answered Sep 18 '19 at 12:46

Just in case anyone ever makes it to this post. You can use the std::swap on shared or unique pointer.

http://www.cplusplus.com/reference/memory/shared_ptr/swap/

edit:
try to avoid code like:

weaponSlotOne.reset(new DevPistol());

It can cause memory leaks. Better would be:

weaponSlotOne = std::make_unique<DevPistol>();

c++ swapping unique_ptr's

3 Answers3