Using a union with unique_ptr

Question

Trying to use a unique_ptr inside a union gives me a segfault when I try to std::move or std::make_unique it.

#include <iostream>
#include <memory>

union myUnion{
    struct{std::unique_ptr<float> upFloat;}structUpFloat;
    struct{std::unique_ptr<int> upInt;}structUpInt;
    myUnion(){}
    ~myUnion(){}
};
struct myStruct{
    int x;
    myUnion num;

};
int main()
{
    myStruct aStruct, bStruct;
    aStruct.x = 1;
    bStruct.x = 2;

    auto upF = std::make_unique<float>(3.14);
    auto upI = std::make_unique<int>(3);

    aStruct.num.structUpFloat.upFloat = std::move(upF);
    bStruct.num.structUpInt.upInt = std::move(upI);

    std::cout << "aStruct float = " << *aStruct.num.structUpFloat.upFloat << std::endl;
    std::cout << "bStruct int = " << *bStruct.num.structUpInt.upInt << std::endl;
    return 0;
}

However, using a normal pointer works as expected:

#include <iostream>
#include <memory>

union myUnion{
    struct{float *pFloat;}structPFloat;
    struct{int *pInt;}structPInt;
    myUnion(){}
    ~myUnion(){}
};
struct myStruct{
    int x;
    myUnion num;

};
int main()
{
    myStruct aStruct, bStruct;
    aStruct.x = 1;
    bStruct.x = 2;

    auto upF = std::make_unique<float>(3.14);
    auto upI = std::make_unique<int>(3);

    aStruct.num.structPFloat.pFloat = upF.get();
    bStruct.num.structPInt.pInt = upI.get();

    std::cout << "aStruct float = " << *aStruct.num.structPFloat.pFloat << std::endl;
    std::cout << "bStruct int = " << *bStruct.num.structPInt.pInt << std::endl;
    return 0;
}

This is using clang.3.4.2 or gcc.4.9.0. So I'm assuming that I am doing something wrong here. Any help would be appreciated.

EDIT:

Ok, so it's probably a nice thing to do to share the code I settled on. Big thanks to everyone who pointed me to managing the lifetime of my pointers in variant members using placement new.

#include <memory>
#include <iostream>
#include <vector>
struct myStruct
{
public:
    union
    {
        std::unique_ptr<float> upFloat;
        std::unique_ptr<int> upInt;
    };
    enum class unionType {f, i,none} type = unionType::none; // Keep it sane
    myStruct(){}
    myStruct(std::unique_ptr<float> p)
    {
        new (&upFloat) std::unique_ptr<float>{std::move(p)};
        type = unionType::f;
    }
    myStruct(std::unique_ptr<int> p)
    {
        new (&upInt) std::unique_ptr<int>{std::move(p)};
        type = unionType::i;
    }
    ~myStruct()
    {
        switch (type)
        {
            case unionType::f: upFloat.~unique_ptr<float>(); break;
            case unionType::i: upInt.~unique_ptr<int>(); break;
        }
    }
};

int main()
{
    std::vector<std::unique_ptr<myStruct>> structVec;
    structVec.push_back(std::make_unique<myStruct>(std::make_unique<float>(3.14f)));
    structVec.push_back(std::make_unique<myStruct>(std::make_unique<int>(739)));
    structVec.push_back(std::make_unique<myStruct>());
    structVec.push_back(std::make_unique<myStruct>(std::make_unique<float>(8.95f)));
    structVec.push_back(std::make_unique<myStruct>(std::make_unique<int>(3)));
    structVec.push_back(std::make_unique<myStruct>());

    for(auto &a: structVec)
    {
        if(a->type == myStruct::unionType::none)
        {
            std::cout << "Struct Has Unallocated Union" << std::endl;
        }
        else if(a->type == myStruct::unionType::f)
        {
            std::cout << "Struct float = " << *a->upFloat << std::endl;
        }
        else
        {
            std::cout << "Struct int = " << *a->upInt << std::endl;
        }
        std::cout << std::endl;
    }

    return 0;
}

Outputs:

Struct float = 3.14

Struct int = 739

Struct Has Unallocated Union

Struct float = 8.95

Struct int = 3

Struct Has Unallocated Union

Answer 1

Changing the active member of a union requires special care to object lifetime. The C++ Standard says (9.5p4):

Note: In general, one must use explicit destructor calls and placement new operators to change the active member of a union.

When the members are plain old data, it generally "just works", even though you aren't calling constructors (using placement new) and destructors. That's because the lifetime for objects with trivial initialization begins "when storage is obtained" of sufficient size and correct alignment, and the union provides that.

Now you've got members with non-trivial constructor and destructor. Their lifetime doesn't begin when storage is obtained, you also have to cause initialization to finish. And that means placement new. Skipping destructor calls isn't safe either, you get undefined behavior if those destructors would have had side effects your program relies on (and a unique_ptr destructor has the side effect of deallocating its target).

Thus you are calling a move-assignment operator on a member whose lifetime hasn't begun. That is undefined behavior.

Answer 2

For unrestricted union, you have to manage yourself some construct/destruction.

Following may help:

union myUnion{
    std::unique_ptr<float> upFloat;
    std::unique_ptr<int> upInt;

    myUnion(){ new (&upFloat) std::unique_ptr<float>{};}
    ~myUnion() {}
};

class myStruct
{
public:
    ~myStruct()
    {
        destroy();
    }

    void destroy()
    {
        switch (type)
        {
            case unionType::f: num.upFloat.~unique_ptr<float>(); break;
            case unionType::i: num.upInt.~unique_ptr<int>(); break;
        }
    }

    void set(std::unique_ptr<int> p)
    {
        destroy();
        new (&num.upInt) std::unique_ptr<int>{std::move(p)};
        type = unionType::i;
    }
    void set(std::unique_ptr<float> p)
    {
        destroy();
        new (&num.upFloat) std::unique_ptr<float>{std::move(p)};
        type = unionType::f;
    }

public:
    enum class unionType {f, i} type = unionType::f; // match the default constructor of enum
    myUnion num;
};

int main()
{
    myStruct aStruct, bStruct;

    aStruct.set(std::make_unique<float>(3.14f));
    bStruct.set(std::make_unique<int>(3));

    std::cout << "aStruct float = " << *aStruct.num.upFloat << std::endl;
    std::cout << "bStruct int = " << *bStruct.num.upInt << std::endl;
    return 0;
}

In C++17, you may use std::variant instead of your own struct

Answer 3

From this reference:

If a union contains a non-static data member with a non-trivial special member function (copy/move constructor, copy/move assignment, or destructor) that function is deleted by default in the union and needs to be defined explicitly by the programmer.

I assume that the reason you wrapped the pointers in simple structures is because you could not build it otherwise, due to the restrictions imposed by the above paragraph.

What you have done instead is bypassed the compilers safety-guards, and probably have undefined behavior in your code.

Answer 4

From §12.6.2[class.base.init]/p8 of the standard (emphasis added):

In a non-delegating constructor, if a given non-static data member or base class is not designated by a mem-initializer-id (including the case where there is no mem-initializer-list because the constructor has no ctor-initializer) and the entity is not a virtual base class of an abstract class (10.4), then

• if the entity is a non-static data member that has a brace-or-equal-initializer, the entity is initialized as specified in 8.5;
• otherwise, if the entity is a variant member (9.5), no initialization is performed;
• [...]

Union members are variant members, which means that the unique_ptrs are left uninitialized. In particular, no constructor, not even the default one, is called. Technically, the lifetime of these unique_ptrs never even began.

The unique_ptr move assignment operator must delete what the unique_ptr is currently holding, but you are move-assigning to an uninitialized "unique_ptr" containing garbage values. As a result, your move assignment likely caused an attempt to delete a garbage pointer, causing a segfault.