How can classes with `std::variant` members be copied safely?

Question

The following sample builds and runs properly with the line Container container2(container1); removed. It appears the copy constructor for std::variant itself is deleted, which makes my Container's copy constructor implicitly deleted.

Effectively, I'm asking:

1. What is the proper way to store a std::variant as a member?
2. What must be implemented in Container before safe copy/move assignment is allowed?

#include <string>
#include <variant>

class A {};

class Container {
 public:
  Container(int i) : data_(i) {}
  Container(float f) : data_(f) {}
  Container(std::string s) : data_(s) {}
  Container(std::unique_ptr<A> a) : data_(std::move(a)) {}
  std::variant<int, float, std::string, std::unique_ptr<A>> data_;
};

int main() {
  Container container1{3};
  
  // error: call to implicitly-deleted copy constructor of 'Container'
  // 
  // copy constructor of 'Container' is implicitly deleted because
  // field 'data_' has a deleted copy constructor
  Container container2(container1);

  return 0;
}

Boxcar

Answer 1

cppreference has this to say about std::variant's copy constructor:

Copy constructor. [...] This constructor is defined as deleted unless std::is_copy_constructible_v<T_i> is true for all T_i in Types. [...]

In other words, it is not deleted unless one or more of the types that the std::variant can contain is not copyable for whatever reason. In your case, it's the std::unique_ptr that's causing the problem. Perhaps std::shared_ptr would be more appropriate.

Whitehall

Answer 2

Extending Paul Sanders' answer: what kind of copy do you want?

If it is a shallow copy, use shared_ptr<A>.

If it is a deep copy, why not have variant<..,A>? If the reason is that A is polymorphic, then the real problem is cloning each of the derived classes. You have to create your own mechanism for the cloning and your own copyable smart pointer to use it - as far as I know, there is nothing in the standard library to help you.

Fistulous

Answer 3

Yet another addition to what you've already gotten: You need to implement the copying yourself since no std::unique_ptr is copyable.

What must be implemented in Container before safe copy/move assignment is allowed?

For your specific case, where A is copy constructible and you'd like to support copying, it could look like this:

Container(const Container& rhs) {
    using namespace std; // to make the below less wide

    if(holds_alternative<int>(rhs.data_)) data_ = get<int>(rhs.data_);
    else if(holds_alternative<float>(rhs.data_)) data_ = get<float>(rhs.data_);
    else if(holds_alternative<string>(rhs.data_)) data_ = get<string>(rhs.data_);

    // this is the problematic one:

    else data_ = make_unique<A>( *get<unique_ptr<A>>(rhs.data_) );
}

std::get the pointer and dereference it (*) and let the copy constructor in A do its work.

There is no additional risk in doing this. Dereferencing the unique_ptr gets you an A& that you provide to make_unique which forwards that to the A being constructed. It's plain copying and there's no magic involved.

A slightly more complicated way could be to create your own unique_ptr wrapper that allows copying and use that in your variant instead.

Example:

template<typename T, typename D = std::default_delete<T>>
class copyable_obj_ptr {
public:
    template<typename... Args>
    copyable_obj_ptr(Args&&... args) : ptr(std::forward<Args>(args)...) {}
    
    // moving
    copyable_obj_ptr(copyable_obj_ptr&&) = default;
    copyable_obj_ptr& operator=(copyable_obj_ptr&&) = default;

    // copying
    copyable_obj_ptr(const copyable_obj_ptr& rhs) : ptr(new T(*rhs.ptr), D{}) {}
    copyable_obj_ptr& operator=(const copyable_obj_ptr& rhs) {
        ptr.reset(new T(*rhs.ptr));
        return *this;
    }

    // dereferencing
    T& operator*() { return *ptr; }
    const T& operator*() const { return *ptr; }

    T* operator->() { return ptr.get(); }
    const T& operator->() const { return ptr.get(); }

    // Add more proxy methods to access the unique_ptr's methods if you need them ...
private:
    std::unique_ptr<T, D> ptr;
};

Mcclees

And when he copies the contained std::unique_ptr that way what do you suppose happens now that two unique ptrs think they own the same heap-allocated object? – Chimb

I don't agree that it is useful. If you want to just copy A like this, include in variant instead of unique_ptr<a>. And if for whatever reason you need a ptr, it is easier to create a copying ptr class than implement this logic for Container. – Fistulous

No unique_ptr is copied. The object it points at is - and if that is not a valid case, the copy constructor should be left deleted and OP:s question would be sort of pointless. – Mcclees

I see. Well, if that is in fact useful to the OP. But - it certainly isn't the usual case that a unique ptr points to an object that doesn't have identity. – Chimb

@Chimb It's very useful. I've implemented copy constructors in small classes holding a resource with a unique_ptr. If the object it points at - fine. There are reasons why a unqiue_ptr doesn't do this automatically but providing copying is by no means bad. – Mcclees

@Chimb Yes, the mechansm works, so if you downvoted the answer for this reason, it is not right. But if somebody else did not downvote, I would downvote it because it is a wrong design. – Fistulous

@Fistulous I've got two objections. 1) I'm answering a question. Not designing. 2) I don't agree that it's bad design per default. Using a unique_ptr for a resource helps you and lets you only focus on the copying part. If the resource is copyable and you'd like to support that - just go ahead and implement it. – Mcclees

I'd like to hear the reason for the downvote though. I may have missed something that makes my answer not useful. – Mcclees