Assigning Union's fields

Question

i have created a structure uses an union as its field. Here is a small code example:

#include <iostream>
#include <string>

enum Type
{
    STR, 
    INT
};

struct MyStruct
{
    Type type;
    union Value
    {
        std::string str;
        int i;
        Value(){}
        ~Value(){};

    } value;
    void setType(Type type)
    {
        this->type = type;
    }
    void setValue(const std::string& data)
    {
        this->value.str = data;
    }
    MyStruct(){}
    ~MyStruct(){}
};

int main()
{
    MyStruct my;
    my.setType(Type::STR);
    my.setValue("Hallo");

   std::cout << my.value.str << std::endl; 

   return 0;
}

Setting Value i got an error (Segmentation fault (core dumped)) What is the right way to do that? Thanks!

Answer 1

Be careful. Non-POD types in unions are asking for trouble. Saying that, this is supported in C++11, provided you only initialise at most one member. That means you'll have to change how things work.

Trivially, to fix your program as it stands right now, you just use placement new:

void setValue(const std::string& data)
{
    new (&value.str) std::string(data);
}

But now, if you want to set a different member later, you will need to use placement delete on the string. To achieve that, you need to know that there was a string there to begin with. So you cannot easily separate the setValue from the setType.

One option (not particularly nice) is:

private:
  void setType( type ) {
      // Destruct existing type
      switch( this->type ) {
      case STR:
          value.str.~std::string();
          break;
      default:;
      }

      this->type = type;
  }

public:
  void setValue( const std::string& data )
  {
      setType( STR );
      new (&value.str) std::string(data);
  }

  void setValue( int data )
  {
      setType( INT );
      value.i = data;
  }

And don't forget to properly destruct the value in ~MyStruct.

Answer 2

Although C++11 does allow you to have classes inside a union, it is generally considered to be bad practice, because it's a minefield of undefined behavior.

If your union's constructor does not construct the union's member which is a class instance -- as is the case with your sample code -- by definition that class instance never gets constructed.

Your code then attempts to use the union's class member. Namely its operator=. Because this class instance has not been constructed, this becomes undefined behavior, which results in your crash.

Stick with PODs in your unions. Less grief.