• Advertisement
Sign in to follow this  

Non-trivial union

This topic is 709 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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.

Edited by jeroenb

Share this post


Link to post
Share on other sites
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?

Share this post


Link to post
Share on other sites

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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites

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;
    }
 }

Share this post


Link to post
Share on other sites

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.

Edited by King Mir

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement