8

I'm trying to use an union (C++) that has some non-primitive variables, but I'm stuck trying to create the destructor for that class. As I have read, it is not possible to guess what variable of the union is being used so there is no implicit destructor, and as I'm using this union on the stack, the compiler errors that the destructor is deleted. The union is as follows:

struct LuaVariant {
    LuaVariant() : type(VARIANT_NONE) { }

    LuaVariantType_t type;
    union {
        std::string text;
        Position pos;
        uint32_t number;
    };
};

The type variable holds what field of the union is being used (chosen from an enum), for the purpose of reading from the union and it could be used to guess what value should be deleted. I tried some different approaches but none of them worked. First of all, just tried the default destructor:

~LuaVariant() = default;

It did not work, as the default is... deleted. So, I tried swapping the value with an empty one, so that the contents would be erased and there would be no problem "leaking" an empty value:

~LuaVariant() {
    switch (type) {
        case VARIANT_POSITION:
        case VARIANT_TARGETPOSITION: {
            Position p;
            std::swap(p, pos);
            break;
        }
        case VARIANT_STRING: {
            std::string s;
            std::swap(s, text);
            break;
        }
        default:
            number = 0;
            break;
    }
};

But as I am not a master of the unions, I don't know if that can cause other problems, such as allocated memory that never gets deallocated, or something like that. Can this swap strategy be used without flaws and issues?

M.M
  • 138,810
  • 21
  • 208
  • 365
ranieri
  • 2,030
  • 2
  • 21
  • 39
  • Instead of your swap trick, you need to explicitly call destructor. Your code does not invoke destructors for the object in the union. This is probably just a resource leak rather than UB, but the cleanest and correct solution is to call destructors. – M.M Jul 29 '15 at 03:58
  • *union-like class* is the Standard term for either a `union`, or a class/struct containing an anonymous union. – M.M Jul 29 '15 at 04:06

2 Answers2

9

This grouping (union + enum value for discriminating type) is called a discriminated union.

It will be up to you to call any construction/destruction, because the union itself cannot (if it could, it would also be able to discriminate for initialized/non-initialized types within the union, and you would not need the enum).

Code:

class LuaVariant // no public access to the raw union
{
public:
    LuaVariant() : type(VARIANT_NONE) { }
    ~LuaVariant() { destroy_value(); }

    void text(std::string value) // here's a setter example
    {
        using std::string;
        destroy_value();
        type = VARIANT_TEXT;
        new (&value.text) string{ std::move(value) };
    }
private:

    void destroy_value()
    {
        using std::string;
        switch(type)
        {
        case VARIANT_TEXT:
            (&value.text)->string::~string(); 
            break;
        case VARIANT_POSITION:
            (&value.pos)->Position::~Position(); 
            break;
        case VARIANT_NUMBER:
            value.number = 0;
            break;
        default:
            break;
        }
    }

    LuaVariantType_t type;
    union {
        std::string text;
        Position pos;
        uint32_t number;
    } value;
};
utnapistim
  • 26,809
  • 3
  • 46
  • 82
  • 2
    I needed to add an empty destructor in my union to get it to work. Am I missing something? I had a "value::~value() is a deleted function" otherwise – Julien Apr 03 '17 at 08:27
  • @Julien Were you using a named union or an anonymous one like the one in the example? – John Gowers Oct 26 '21 at 16:45
3

If you want to use std::string in a union in C++11, you have to explicitly call its destructor, and placement new to construct it. Example from cppreference.com:

#include <iostream>
#include <string>
#include <vector>
union S {
    std::string str;
    std::vector<int> vec;
    ~S() {} // needs to know which member is active, only possible in union-like class 
}; // the whole union occupies max(sizeof(string), sizeof(vector<int>))

int main()
{
    S s = {"Hello, world"};
    // at this point, reading from s.vec is UB
    std::cout << "s.str = " << s.str << '\n';
    s.str.~basic_string<char>();
    new (&s.vec) std::vector<int>;
    // now, s.vec is the active member of the union
    s.vec.push_back(10);
    std::cout << s.vec.size() << '\n';
    s.vec.~vector<int>();
}
Emil Laine
  • 41,598
  • 9
  • 101
  • 157
  • Good, but that is a big hassle. Sometimes I don't know which of the fields is being used (I just push and pull to/from the Lua stack), so I would need a switch anyway to destroy them, right? Also, is there any cons to my approach? – ranieri Jul 29 '15 at 03:38
  • 3
    @ranisalt You _have_ to know which of the fields you're using, to know which one you can access. Accessing any other field than the active one is UB. – Emil Laine Jul 29 '15 at 04:43