Creating object with Copy Constructor (Simple Rule of Three Class) yields run-time error

Question

I have the simple program below:

#include <iostream>

class Counter
{
private:
    size_t m_count;
public:
    Counter() :
        m_count(0)
    {
        std::cout << "defctor" << '\n';
    }
    Counter(size_t count) :
        m_count(count)
    {
        std::cout << "ctor" << '\n';
    }
    ~Counter() {
        std::cout << "dtor" << '\n';
    }
    Counter(const Counter& rhs) :
        m_count{0}
    {
        Counter temp{rhs.m_count};
        std::cout << "cctor" << '\n';
        std::swap(*this, temp);
    }
    Counter& operator=(const Counter& rhs)
    {
        if (this == &rhs) {
            return *this;
        }
        Counter temp{rhs.m_count};
        std::swap(*this, temp);
        std::cout << "caop" << '\n';
        return *this;
    }

    constexpr int getCount() const noexcept {
        return this-> m_count;
    }
};


int main() {
    Counter x{1};
    Counter y;
    Counter z{x}; // this fails
}

I've tried to construct a simple rule of three class. I get UB on this line Counter z{x}; which should be calling the copy constructor. Output:

ctor
defctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor
cctor
ctor

Then it repeats ctor\ncctor ...

It's been a while since I worked with C++. I just cannot find the error!

Answer 1

std::swap only uses the move assignment operator if you have defined one (the compiler will not define one once you add almost any other special member function). You did not, so it falls back to copy assignment.

But your copy assignment operator is defined in terms of std::swap. That's of course an endless recursion: To swap you need to assign, and to assign you need to swap. Eventually you get a stack overflow.

You could just initialize m_count directly in the copy constructor (and modify it directly in the copy assignment operator). It looks like you are doing half a copy-and-swap idiom, but I'm not sure what you are really going for here.

And yes, in modern C++ you should also implement the move constructor where appropriate. If done correctly, this would fix the std::swap recursion. I suggest you look into some examples of how to implement these special member functions correctly.

Answer 2

In my opinion you should change your copy constructor like this:

Counter(const Counter& rhs)
{
    m_count=rhs.m_count;
}

It will be enough. The result is:

ctor
defctor
dtor
dtor
dtor
Press any key to continue ...