Constructor and copier constructor for a class containing a union with nontrivial members

Question

I am trying to implement a custom variant type that uses a union to store data of various types. In the type_id field type_id I plan to save that type of data stored in the union. The union contains nontrivial 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 am trying to create an instance of MyVariant as shown below:

 MyVariant v; 

I get an error: the implicitly deleted default constructor MyVariant is called. So, I tried to implement the constructor manually, as shown below:

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

This gives me a similar error message: trying to use a remote function. Then I tried to implement the following constructor:

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

and build my example as follows:

 MyVariant v{123}; 

=> same error message: attempt to delete a remote function.

I also started implementing the copy constructor, it looks like this. However, of course, this does not help with 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 a compiler.

The main question is: why do I get the compiler errors that I get, and how do I change the code so that it compiles?

Additional question: am I correct with my implementations for the copy constructor and constructor?

Answer 1

Your union has data members such as string , unique_ptr and map , all of which have non-trivial default / copy / move constructors, copy and move assignment operators, and destructors. Therefore, all of them are implicitly deleted for your association.

§9.5 / 2 [class.union]

... [Note: if any non-static data member from the 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 membership function of the union must be provided by the user or it will be implicitly removed (8.4.3) to combine. -end note]

So, you must manually implement them for your union. At a minimum, so that you can instantiate MyVariant , the class must be constructive and destructible. Therefore 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"); } } 

The implementation of your instance constructor looks correct, but what I do differently, instead of creating the default element first and then copying from the original object, just copy the construct in the newest allocation call.

 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 element unique_ptr active and stores a pointer to some instance of the derived class using the base class pointer, then your copy constructor implementation will only copy part of the base class.

Finally, if you are not doing this as an exercise for learning, I highly recommend that you use Boost.Variant instead of yourself.