Move assignment operator is not moving my shared_ptr

Question

I'm having some trouble with my move assignment operator, and maybe I'm just misunderstanding what's going on. My SignalHolder object below is meant to hold a shared pointer to a Signal object and call its detach() method as soon as the SignalHolder is destroyed. Since destroying the object automatically causes the signal to be broken, it's important that only one exists. Hence I'm using move semantics to ensure only one exists.

Anyhow, even though I've written a move assignment operator, it doesn't seem to be moving my shared_ptr. If I add break point to the end of the assignment method, both *this and c point to the same object. I though std::move was supposed to move the shared_ptr to *this and leave c with a nullptr? What am I doing wrong here?

class SignalHolder
{
    std::shared_ptr<SignalCallbackBase> _target;
    inline static int idIdx = 0;

    int id;

public:
    SignalHolder(std::shared_ptr<SignalCallbackBase> target = nullptr) :
        _target(target)
    {
        id = idIdx++;
        qDebug() << "SignalHolder construct " << id;
    }

    SignalHolder(const SignalHolder&) = delete;
    SignalHolder(const SignalHolder&& c) :
        _target(std::move(c._target))
    {
        qDebug() << "SignalHolder move construct " << id;
    }

    ~SignalHolder()
    {
        qDebug() << "SignalHolder destruct " << id;
        detach();
    }

    SignalHolder& operator=(SignalHolder const&) = delete;
    SignalHolder& operator=(SignalHolder const&& c)
    {
        qDebug() << "SignalHolder assign move " << id;
        _target = std::move(c._target);

        //Break point here
        int j = 9;
    }

    void detach() {
        qDebug() << "SignalHolder detach " << id;
        if (_target)
            _target->detach();

        _target = nullptr;
    }
};


SignalHolder makeHolder()
{
    auto s = std::make_shared<SignalCallbackBase>();
    return SignalHolder(s);
}

int main()
{
    SignalHolder a;
    a = makeHolder();

}

Note: Shared pointer moves itself. You can sit back and let the compiler do the work for you. — user4581301, Mar 10 '21 at 04:03
So if I do not define any copy or move assignments or constructors, by default I will get a move? — kitfox, Mar 10 '21 at 04:05
@kitfox, you have the same problem with your "move" constructor as you do with your move assignment operator. You have to remove the "const" from the declaration: SignalHolder(SignalHolder&& c); otherwise, the copy constructor for the shared_pointer will be called instead of the move constructor. — Luis Guzman, Mar 10 '21 at 06:14
@kitfox, also, the way you've coded your constructor, it will be creating 3 instances of shared_ptr (use shared_ptr<>::use_count() in constructor to verify). In order to only generate one copy of shared_ptr<> at any time, the declaration has to be: `SignalHolder(std::shared_ptr&& target = nullptr) : _target(std::move(target) ...`, which will force you to modify the return in makeHolder to: `return SignalHolder(std::move(s));`. — Luis Guzman, Mar 10 '21 at 06:25
@kitfox, also, the class design seems flawed. It appears to me like you're trying to construct your own `std::unique_ptr` with a custom deleter, which already exists in the standard library. Consider using it instead of your SignalHolder. See custom deleter demo in https://en.cppreference.com/w/cpp/memory/unique_ptr. I hope it helps. — Luis Guzman, Mar 10 '21 at 06:42
By default you will get a functioning move constructor. Whether the move constructor is used or not depends on how you use the object. and whether or not the compiler has a better trick up its sleeve. [See the Rule of Zero](https://en.cppreference.com/w/cpp/language/rule_of_three) — user4581301, Mar 10 '21 at 17:39

songyuanyao · Accepted Answer · 2021-03-10T06:22:24.540

Parameter c is declared as const, it can't be moved. Move operation is supposed to perform modification on the object to be moved. In _target = std::move(c._target);, the copy assignment operator but not move assignment operator of std::shared_ptr gets called. (The move assignment operator of std::shared_ptr takes rvalue-reference to non-const, which can't bind to const objects.)

You should remove the const qualifier too.

SignalHolder& operator=(SignalHolder && c)
{
    qDebug() << "SignalHolder assign move " << id;
    _target = std::move(c._target);

    //Break point here
    int j = 9;
    return *this;
}

BTW: You lost the return statement in operator=.

Move assignment operator is not moving my shared_ptr

1 Answers1