Destructor of union member seems to be called automatically

Question

I'm attempting to implement a tagged union.

My understanding was that in a C++ union, the non-trivial (i.e. not empty) destructors of non-static members are never called, thus we have to call them ourselves. That's what I did:

#include <iostream>

class C {
public:
  C() {
    std::cout << "C Ctor" << std::endl;
  }
  ~C() {
    std::cout << "C Dtor" << std::endl;
  }
};

class B {
public:
  B() {
    std::cout << "B Ctor" << std::endl;
  }
  ~B() {
    std::cout << "B Dtor" << std::endl;
  }
};

struct S {
  int type;

  union U {
    C c;
    B b;

    U() {

    }

    ~U() {}
  } u;

  S(int type) : type(type) {
    if (type == 0) {
      u.c = C();
    } else {
      u.b = B();
    }
  }

  ~S() {
    if (type == 0) {
      u.c.~C();
    } else {
      u.b.~B();
    }
  }
};

int main() {
  S s(0);
  return 0;
}

However, the output is:

C Ctor
C Dtor
C Dtor

Meaning, the C destructor is being called twice, instead of just once.

What is going on? And if you notice additional issues with my tagged union implementation, please point them out.

Wheels

FYI: std::endl is usually unnecessary. "\n" is just as portable and doesn't try to flush the output at every line. – Maziar

Typically you would use an anonymous union to implement a tagged union, you can read about these in the standard and/or check out the implementation of std::variant. – Dabney

Answer 1

In

S(int type) : type(type) {
    if (type == 0) {
      u.c = C();
    } else {
      u.b = B();
    }
  }
  

Since you are in the body of the constrcutor, u.c = C(); is not initialization but is instead assigment. That means you see the constructor called for C(), and then at the end of the expression the first destructor call is called to destroy that temporary. We can see this by adding

C& operator=(const C&) { std::cout << "operator=(const C&)\n"; return *this; }

to C which changes the output to

C Ctor
operator=(const C&)
C Dtor
C Dtor

Then the second destructor call is when s goes out of scope in main and its destructor is ran.

---

Do note that as is, the code has undefined behavior. Unions do not activate a member in the constructor user provided constructor you wrote so when you do

u.c = C();

you are assigning to an object that is not yet alive. You can't modify an object that isn't alive.

Inconsiderable

I see. So the destructor is called once for the temporary and only once for the member. Correct? If so - how would you suggest changing this? The reason I instantiate C in the body and not the initializer list is becasue I need to branch on the type... I guess I can do trinary operator in the initializer list, but is this common? – Wheels

@AvivCohn Use placement new like S(int type) : type(type) { if (type == 0) { new (&u.c) C(); } else { new (&u.b) B(); } }. That gives you C Ctor C Dtor as the output. – Inconsiderable

@NathanOliver: You could also use a tool like Compiler Explorer to view the code's disassembly and see where the deconstructor is called; works well for small examples like this. – Feoff

@Inconsiderable I see regarding placement new. Is it correct that the reason why this works is because it avoids creating an additional temporary on the stack? – Wheels

@AvivCohn Correct. It directly constructs the object the the memory the union occupies. – Inconsiderable

@AvivCohn If you want to see how to do this in production level code, take a look at one of the implementations of std::variant. That is the C++ standard tagged union. – Inconsiderable

@AvivCohn The reason why your example doesn't work is because it has undefined behaviour, because it calls assignment operator of inactive union member that is non-trivially constructible. Nathan's suggestion (in comments) doesn't do that, and the behaviour is well defined. – Continuant

@Continuant I see - I was about to make my peace with just creating a temporary on the stack like shown in the question. But, are you saying that it's actually illegal because assigning to a union which has not been constructed yet is UB? If I get that right, than the only way on C++ to assign a value to a union member which is not trivially-constructible and hasn't been constructed yet, is using placement new or the constructor initializer list. Correct? – Wheels

@AvivCohn As far as I know yes, the only way to activate an inactive non-trivial union member is placement new. – Continuant

@Continuant I learned something. Thanks! Coming from C, some stuff in C++ is surprisingly involved... in C this stuff takes you 3 minutes. I guess I have a lot more to learn. – Wheels

Also, u.c = C(); causes undefined behaviour since u.c is not the active member and has a user-provided constructor. (There is no active member since the union constructor was user-provided and did not set an active member). You can see in the output that operator= is called on an object that was never constructed. (as you point out, the previous constructor call is the temporary, not u.c) – Dabney

Answer 2

In your constructor you create the temporary instance of C:

u.c = C();

which is copied and then destructed. So, first 2 lines of output belong to this instance. And the last output line is result of your ~S() call.

Besides this, beginning from C++17 you have standard powerful implementation of tagged union: https://en.cppreference.com/w/cpp/utility/variant

Rouvin