Writing a copyable unique_ptr for copyable classes with PIMPL idiom

Question

I want to write a class that uses the PIMPL idiom but makes the following possible:

int main() {
    MyClass myclass {12};
    std::string val = myclass.get_value();
    std::cout << val << "\n";

    MyClass copy = myclass;
    val = copy.get_value();

    return 0;
}

That is, provide copy functionality while using PIMPL. Using std::unique_ptr to implement PIMPL leads to the expected compile error:

error C2280: 'MyClass::MyClass(const MyClass &)': attempting to reference a deleted function

This is my attempt:

class.hpp

#pragma once

#include <string>

#include "copyable_unique_ptr.hpp"

class MyClass {
private:
    struct MyClassPrivate;
    copyable_unique_ptr<MyClassPrivate> _impl;

public:
    MyClass(int value);
    ~MyClass();

    std::string get_value();
};

class.cpp

#include "class.hpp"

#include <string>

struct MyClass::MyClassPrivate {
    int value;
};

MyClass::MyClass(int value) 
    : _impl(std::make_unique<MyClassPrivate>()) {
    _impl->value = value;
}

MyClass::~MyClass() = default;

std::string MyClass::get_value() {
    return std::to_string(_impl->value);
}

copyable_unique_ptr.hpp

#pragma once

#include <memory>

template <typename T>
class copyable_unique_ptr {
private:
    std::unique_ptr<T> _ptr;

public:
    copyable_unique_ptr(std::unique_ptr<T> &&ptr)
        : _ptr(std::move(ptr)) {}

    ~copyable_unique_ptr() = default;

    copyable_unique_ptr(const copyable_unique_ptr<T> &other) {
        _ptr = std::make_unique<T>(*other._ptr);
    }

    copyable_unique_ptr(copyable_unique_ptr<T> &&other) {
         _ptr = std::move(other._ptr);
    }

    copyable_unique_ptr &operator=(const copyable_unique_ptr<T> &other) {
        _ptr = std::make_unique<T>(*other._ptr);
    }

    copyable_unique_ptr &operator=(copyable_unique_ptr<T> &&other) {
         _ptr = std::move(other._ptr);
    }

    T *operator->() {
        return _ptr.get();
    }
};

which leads to the following error: warning C4150: deletion of pointer to incomplete type 'MyClass::MyClassPrivate'; no destructor called

I also had this error when first trying to use unique_ptr, and the solution was to move the destructor into class.cpp: MyClass::~MyClass() = default;.

I don't really know how to solve this problem while using the approach with copyable_unique_ptr.

I saw this question on SO. They are trying to do the same as me, but the last comment to the answer suggests that they are having exactly the same problem.

Also need to move the copy constructor and the assignment operator into the class implementation, too. P.S. Are you familiar with `std::shared_ptr`? None of this is needed, with `std::shared_ptr`. — Sam Varshavchik, Oct 18 '20 at 10:56
If a **unique** pointer is copyable, then it kind of breaks the whole "unique" part of the pointer. Or do you mean that you want to copy the object that is pointed to? You don't need a new special class for that, just something like `auto copy = std::make_unique(*original);` — Some programmer dude, Oct 18 '20 at 11:01
You could get away with using a `std::vector` with just one element. — Galik, Oct 18 '20 at 11:02
@SamVarshavchik: Would using `std::shared_ptr` be useful even when I want a 1:1 relationship between MyClass and MyClassPrivate? Btw: Moving copy constructor and assignment operators into the class implementation helped. That makes everything compile and work as expected. — masi456, Oct 18 '20 at 11:20
@SamVarshavchik I think `std::shared_ptr` would do exactly the wrong thing here. What is needed is a single dynamic object with copy semantics rather than pointer semantics. I would simply use a `std::vector` with a single element for this. — Galik, Oct 18 '20 at 11:23
@Someprogrammerdude I want to make a deep copy of the private implementation instance, not copy the pointer. All that while remaining the 1:1 relationship between MyClass and MyClassPrivate. — masi456, Oct 18 '20 at 11:24
@Galik: Using a vector with only one element is a wonderful idea and seems to work good! The only thing that bothers me a little bit is that it's not really obvious on first sight what is happening and why it's done. Still, I'm considering using that. — masi456, Oct 18 '20 at 11:31

Galik · Accepted Answer · 2020-10-18T11:44:44.223

Although you can write a custom smart pointer here, I always like to reuse as much code as possible. It seems you want a dynamically created object with copy semantics rather than pointer semantics. For that reason I would hesitate to call your copyable_unique_ptr a pointer at all. I did write a class for this once and settled on something like dynamic_object<T>.

Instead of a pointer I would do this using a std::vector that contains just a single element. Something a bit like this:

class MyClass {
private:
    struct MyClassPrivate;
    std::vector<MyClassPrivate> _impl;

public:
    MyClass(int value);
    ~MyClass();

    std::string get_value();
};

struct MyClass::MyClassPrivate {
    int value;
};

MyClass::MyClass(int value)
    : _impl(1) {
    _impl.front().value = value;
}

MyClass::~MyClass() = default;

std::string MyClass::get_value() {
    return std::to_string(_impl.front().value);
}

However if you really want to create your copyable_unique_ptr I woud still use a std::vector here to simplify its codeing and to reuse high quality Standard Library code:

Perhaps a bit like this:

template<typename T>
class copyable_unique_ptr
: private std::vector<T>
{
    using base_type = std::vector<T>;

public:
    copyable_unique_ptr(std::unique_ptr<T>&& ptr)
    : base_type(std::move(*ptr), 1) {}

    copyable_unique_ptr(std::unique_ptr<T> const& ptr)
    : base_type(*ptr, 1) {}

    T& operator*()  { return base_type::back(); }
    T* operator->() { return base_type::data(); }

    T const& operator*()  const { return base_type::back(); }
    T const* operator->() const { return base_type::data(); }
};

score 1 · Answer 2 · answered Oct 18 '20 at 13:05

Sam Varshavchik gave a hint in the comment that also moving copy constructor and assignment operator to the implementation fixes the code that I was trying to write, that means:

class.hpp

#pragma once

#include <string>

#include "copyable_unique_ptr.hpp"

class MyClass {
private:
    struct MyClassPrivate;
    copyable_unique_ptr<MyClassPrivate> _impl;

public:
    MyClass(int value);

    MyClass(const MyClass &);
    MyClass(MyClass &&);
    MyClass &operator=(const MyClass &);
    MyClass &operator=(MyClass &&);
    ~MyClass();

    std::string get_value();
};

class.cpp

#include "class.hpp"

#include <string>
#include <iostream>

struct MyClass::MyClassPrivate {
    int value;
};

MyClass::MyClass(int value)
    : _impl(std::make_unique<MyClassPrivate>()) {
    _impl->value = value;
}

MyClass::MyClass(const MyClass &) = default;
MyClass::MyClass(MyClass &&) = default;
MyClass &MyClass::operator=(const MyClass &) = default;
MyClass &MyClass::operator=(MyClass &&) = default;
MyClass::~MyClass() = default;


std::string MyClass::get_value() {
    std::cout << &_impl << "\n";
    return std::to_string(_impl->value);
}

And it works as expected.

On the other hand, Galik gave an answer that gives a much better and less complex alternative to copyable_unique_ptr. I will prefer using his way.

score 1 · Answer 3 · answered Oct 18 '20 at 13:18

Its probably simpler to stick to a normal unique_ptr and just implement copying of MyClass in your cpp file:

class MyClass {
private:
    struct MyClassPrivate;
    std::unique_ptr<MyClassPrivate> _impl;

public:
    MyClass(int value);
    MyClass(MyClass&&);
    MyClass(const MyClass&);
    ~MyClass();

    MyClass& operator = (const MyClass&);
    MyClass& operator = (MyClass&&);

    std::string get_value();
};

MyClass::MyClass(const MyClass& other)
    : _impl(std::make_unique<MyClassPrivate>(*other._impl)) {
}

MyClass& MyClass::operator = (const MyClass& other) {
    _impl = std::make_unique<MyClassPrivate>(*other._impl);
    return *this;
}

MyClass::~MyClass() = default;
MyClass::MyClass(MyClass&&) = default;
MyClass& MyClass::operator = (MyClass&&) = default;

There is a slight issue with this code (and your original code) in that the moved from objects will have a null _impl you might want to add checks for null, alternatively you could replace with a default constructed object instead of a null pointer but that might be a waste in most cases?

Writing a copyable unique_ptr for copyable classes with PIMPL idiom

3 Answers3