Non-trivial union

Started by
4 comments, last by King Mir 7 years, 11 months ago

I am trying to get my head around the new C++11 non-trivial unions in Visual Studio 2015. Please have a look at a simplified class I created (note that this code might not compile in its current form):


class Type
{
public:
  Type() :
    mKind(eObject),
    mObject()
  {
  }

?  // move constructor and operator= removed

  Type(const Type& that) :
    mKind(eObject),
    mObject()
  {
    if ( that.mKind == eArray ) {
      mKind = eArray;
      mArray.mDimension = 1;
      mArray.mType.reset(new Type()); // <-- here I get crashes due to uninitialized mType
    }
    else {
      mObject.mName = that.eObject.mName;
    }
  }

  enum Kind { eArray, eObject } mKind;
  struct ArrayInfo {
    std::unique_ptr<Type> mType;
    int mDimension;
  };

  struct ObjectInfo {
    String mName;
  };

  union {
    ArrayInfo  mArray;
    ObjectInfo mObject;
  };
}

I read that you may only initialize one member of the union (otherwise the constructor is ill formed). In this case I choose the mObject variable. Compiling works fine and well, but at runtime when I try to assign values to the mArray part I get crashes as it seems that it has not been initialized. I have looked through numerous resources about non-trivial unions and they always only have a std::string in there. Now I wonder how I can make this work. Do I need something like:


mObject.~String();
new (&mArray) ArrayInfo(); 

Thanks for the help.

Crafter 2D: the open source 2D game framework

?Github: https://github.com/crafter2d/crafter2d
Twitter: [twitter]crafter_2d[/twitter]

Advertisement

Hi.

unique_ptr::reset() calls the destructor and bang, you're dead, because its internal members are most probably crap.

If you alias an ArrayInfo over the same bytes as ObjectInfo, it can't end well.

unique_ptr and int size is implementation defined, your String is who knows what (pointer + int or an int + pointer or possibly anything). How do you imagine it should work? What are you trying to achieve? The unique_ptr won't be in a healthy state for you to call reset.

Furthermore, the standard says it's only legal to read from the most recently written union member, although actual compilers are less strict.

What you suggest might work, just I wouldn't call a "String" dtor on an instance of a ObjectInfo class :)

But what's the point?

The Type class is used in my custom script language to specify the type of for example a variable, class, function argument, etc. To preserve memory I wanted to give this new C++11 functionality a try as that would prevent memory allocations of the structs in case a pointer is used.

Crafter 2D: the open source 2D game framework

?Github: https://github.com/crafter2d/crafter2d
Twitter: [twitter]crafter_2d[/twitter]


class Type
{
public:
  Type() :
    mKind(eObject),
    mName()
  {

  Type(const Type& that) :
    mKind(eObject),
    mName()
  {

I read that you may only initialize one member of the union (otherwise the constructor is ill formed). In this case I choose the mObject variable.

You don't initialize any of the union members (see the initializer lists quoted above). You only perform assignment to the uninitialized members within the constructor body.

Right, I fixed that in my code. I think I got it working with the proposal I made myself. It works both with GCC and visual studio. E.g.:


Type(const Type& that) :
    mKind(eObject),
    mObject()
  {
    if ( that.mKind == eArray ) {
      mKind = eArray;
      new (&mArray.mType) std::unique_ptr<Type>(new Type()));
      mArray.mDimension = 1;
    }
    else {
      mObject.mName = that.eObject.mName;
    }
 }

Crafter 2D: the open source 2D game framework

?Github: https://github.com/crafter2d/crafter2d
Twitter: [twitter]crafter_2d[/twitter]

That's still not correct.

Essentially, if you have a non-trivial type in a union, you have to explicitly call constructors and destructors. Calling mObject in the initializer list does this for object, but then you cannot store an ArrayInfo there, without first calling the destructor. So instead, you should not construct either object in the member initializer expression list, instead using placement new in the constructor body. The code looks like this:


Type(const Type& that) :
    mKind(that.mKind)
{
    if ( that.mKind == eArray ) {     
      new (&mArray) ArrayInfo{that.mArray}; 
    }
    else {
      new (&mObject) ObjectInfo{that.mObject}; 
    }
}

(I also made this follow normal copy constructor semantics, which your code didn't do)

Likewise, you need to define a destructor, that will destroy the correct union member explicitly.

This topic is closed to new replies.

Advertisement