c++ - Constructor and copy-constructor for class containing union with non-trivial members

Question

I am trying to implement a custom variant type which uses a union to store data of various different types. In the field type_id I plan to store which type the data stored in the union is of. The union contains non-trivial members. Here is my current implementation:

struct MyVariant {
  enum { t_invalid, t_string, t_int, t_double, t_ptr, t_dictionary } type_id;
  union {
    int                             as_int;
    double                          as_double;
    std::string                     as_string;
    std::unique_ptr<int>            as_ptr;
    std::map<int, double>           as_dictionary;
  };
};

I try to create an instance of MyVariant like follows:

MyVariant v;

I get the error message: Call to implicitly-deleted default constructor of MyVariant. So, I tried to implement the constructor manually like follows:

MyVariant() : type_id{t_int}, as_int{0} {}

That gives me a similar error message: Attempt to use a deleted function. Next, I tried to implement the following constructor:

MyVariant(int value) : type_id{t_int}, as_int{value} {}

and construct my instance like follows:

MyVariant v{123};

=> same error message: Attempt to use a deleted function.

I've also started to implement a copy constructor, it looks like follows. However, of course this doesn't help with the compiler errors.

MyVariant::MyVariant(const MyVariant& other)
{
    type_id = other.type_id;
    switch (type_id) {
        case t_invalid:
            break;
        case t_string:
            new (&as_string) std::string();
            as_string = other.as_string;
            break;
        case t_int:
            as_int = other.as_int;
            break;
        case t_double:
            as_double = other.as_double;
            break;
        case t_ptr:
            new (&as_ptr) std::unique_ptr<int>(nullptr);
            as_ptr = std::make_unique<int>(*other.as_ptr);
            break;
        case t_dictionary:
            new (&as_dictionary) std::map<int, double>();
            // TODO: copy values from other
            break;
    }
}

I am using Xcode and Apple LLVM 6.1 as compiler.

The main question is: Why do I get the compiler errors which I'm getting and how do I have to modify my code to make it compile?

The additional question is: Am I on the right way with my implementations for the constructor and copy constructor?

Answer 1

Your union has data members of type string, unique_ptr and map, all of which have non-trivial default/copy/move constructors, copy/move assignment operators and destructors. Hence all of these are implicitly deleted for your union.

§9.5/2 [class.union]

... [ Note: If any non-static data member of a union has a non-trivial default constructor (12.1), copy constructor (12.8), move constructor (12.8), copy assignment operator (12.8), move assignment operator (12.8), or destructor (12.4), the corresponding member function of the union must be user-provided or it will be implicitly deleted (8.4.3) for the union. —end note ]

So you must manually implement these for your union. At a minimum, for you to be able to create an instance of MyVariant, the class needs to be constructible and destructible. So you need

MyVariant() : type_id{t_int}, as_int{0} {}
~MyVariant()
{
  switch(type_id)
  {
      case t_int:
      case t_double:
        // trivially destructible, no need to do anything
        break;
      case t_string:
        as_string.~basic_string();
        break;
      case t_ptr:
        as_ptr.~unique_ptr();
        break;
      case t_dictionary:
        as_dictionary.~map();
        break;
      case t_invalid:
        // do nothing
        break;
      default:
        throw std::runtime_error("unknown type");
  }
}

Your copy constructor implementation looks valid, but what I'd do differently is instead of first default constructing the member, and then copying from the source object, just copy construct in the placement new call itself.

MyVariant(const MyVariant& other)
{
  type_id = other.type_id;
  switch (type_id) {
      case t_invalid:
          break;
      case t_string:
          new (&as_string) auto(other.as_string);
          break;
      case t_int:
          as_int = other.as_int;
          break;
      case t_double:
          as_double = other.as_double;
          break;
      case t_ptr:
          new (&as_ptr) auto(std::make_unique<int>(*other.as_ptr));
          break;
      case t_dictionary:
          new (&as_dictionary) auto(other.as_dictionary);
          break;
  }

Live demo

Note that if the unique_ptr member is active, and is storing a pointer to some derived class instance via a base class pointer, then your copy constructor implementation will only copy the base class part.

Finally, unless you're doing this as a learning exercise, I'd strongly urge you to use Boost.Variant instead of rolling your own.