1

I am experimenting with the concept of CRTP and how it can be used to approximate mixins in C++.

I've developed the following code to illustrate the idea, but came across a problem when the vector shapeVec tries to delete the smart pointers, it makes a segmentation fault.

Can someone explain what is wrong with this approach? Thanks.

#include <memory>
#include <vector>
#include <iostream>
using namespace std;

struct Shape
{
    virtual unique_ptr<Shape> clone ()= 0;
    virtual int getX() = 0;
    virtual ~Shape()=default;
};

template <typename T>
struct CloneMixin
{
    unique_ptr<Shape> clone () 
    {
        return unique_ptr<T>(static_cast<T*>(this));
    }
};

template <typename T>
struct GetterMixin
{
    int getX()
    {
        return static_cast<T*>(this)->x;
    }
};

struct Square : public CloneMixin<Square>, public GetterMixin<Square>, public Shape
{
    int x=1;

    virtual unique_ptr<Shape> clone() override { return CloneMixin::clone();}
    virtual int getX() override { return GetterMixin::getX();}

    virtual ~Square()=default;
};


struct Rectangle : public CloneMixin<Rectangle>, public GetterMixin<Rectangle>, public Shape
{
    int x=2;
    int y=3;

    virtual unique_ptr<Shape> clone() override { return CloneMixin::clone();}
    virtual int getX() override { return GetterMixin::getX();}

    virtual ~Rectangle()=default;
};


int main()
{
    vector < unique_ptr<Shape>> shapeVec;
    shapeVec.push_back(make_unique< Square>());
    shapeVec.push_back(make_unique<Rectangle>());

    for (auto &i : shapeVec)
    {
       unique_ptr<Shape> ss = i->clone();
       cout << ss->getX()<<endl;
    }
    return 0;
}

Edit: I had a mistake in my clone function. It was returning the same pointer of the objects in shapeVec. Such objects were deleted once when deleting the (ss) unique pointer in the main function. When the shapeVec tried to delete its own pointers, they already deleted before. This caused the exception.

I changed the clone function inside CloneMixin to the following, and it worked as expected:

 unique_ptr<T> clone () 
    {
        return make_unique<T>(*static_cast<T*>(this));
    }

1 Answers1

3

The trouble is that you are not actually cloning in CloneMixin::clone. You are returning a new unique_ptr that manages the same object as the object you are supposed to be making a clone of. This is obviously an error. The solution is to actually make a copy.

Say you have a copy constructor for each of the shapes, then you could do something like

template <typename T>
struct CloneMixin
{
    unique_ptr<Shape> clone()
    {
        T* this_shape = static_cast<T*>(this);
        return unique_ptr<T>( new T(*this_shape) );
    }
};

You could also use make_unique<T> there, and the member function could be const, given suitable const ref copy constructors.

jwezorek
  • 8,592
  • 1
  • 29
  • 46