28

C++11 allowed the use of standard layout types in a union: Member of Union has User-Defined Constructor

My question then is: Am I guaranteed the custom destructor will be called, when the union goes out of scope?

My understanding is that we must manually destroy and construct when switching: http://en.cppreference.com/w/cpp/language/union#Explanation

But what about an example like this:

{
    union S { string str;
              vector<int> vec;
              ~S() {} } s = { "Hello, world"s };
}

When s goes out of scope, have I leaked the memory the string allocated on the heap because I did not call string's destructor?

Community
  • 1
  • 1
Jonathan Mee
  • 35,107
  • 16
  • 95
  • 241
  • This does not even compile. AFAIK you need to use placement new for those things and call the constructor manually. So to be consistent the same goes for destructors. – Hayt Oct 18 '16 at 11:38
  • @Hayt So after my edit, this constructor will compile: http://ideone.com/Cf0OOQ but according to [Nathan Oliver's answer](http://stackoverflow.com/a/40107282/2642059) it *does* leak. – Jonathan Mee Oct 18 '16 at 11:58
  • 1
    I am not sure why the assignment works in the first place. It seems unintentional. But it still leaks. You can see here the `basic_string` constructor being called but not the destructor: https://godbolt.org/g/2uvrWn – Hayt Oct 18 '16 at 12:14
  • @Hyat I was asking Nathan Oliver the same thing: http://stackoverflow.com/questions/40106941/is-a-union-members-destructor-called#comment67486856_40107282 It does seem that the example given in http://en.cppreference.com/w/cpp/language/union#Explanation manually constructs and destroys. – Jonathan Mee Oct 18 '16 at 12:20
  • 1
    yes that what you should do and have to do. The case that it works with `{ }` assignment seems like a design flaw of unions because it implicitly allocates but not deallocates. – Hayt Oct 18 '16 at 12:26
  • @Hayt That's what I'm hearing from Nathan Oliver too. It seems like placement new is the only way to handle this :( – Jonathan Mee Oct 18 '16 at 12:29
  • They specifically state this on cppreference too. You get away easier when using variants instead of unions though – Hayt Oct 18 '16 at 12:30
  • @Hyat Incidentally, I think Nathan Oliver's answer is the correct one as it cites the standard to both answer my question and explain that the object will not be destroyed, so I'll be accepting it. That said, your assembly link was what really solidified it for me. If you post it as an answer you will get at least my upvote, and it may be helpful to more than me... – Jonathan Mee Oct 18 '16 at 12:34
  • 1
    I also agree with nathan being correct. But if it helps I can add an answer with some explanation. – Hayt Oct 18 '16 at 12:37

3 Answers3

22

In your example that you provided str will not be destructed. The standard states in [class.union]/2

A union can have member functions (including constructors and destructors), but not virtual (10.3) functions. A union shall not have base classes. A union shall not be used as a base class. If a union contains a non-static data member of reference type the program is ill-formed. At most one non-static data member of a union may have a brace-or-equal-initializer . [ Note: If any non-static data member of a union has a non-trivial default constructor (12.1), copy constructor (12.8), move constructor (12.8), copy assignment operator (12.8), move assignment operator (12.8), or destructor (12.4), the corresponding member function of the union must be user-provided or it will be implicitly deleted (8.4.3) for the union. — end note ]

emphasis mine

So since both str and vec have special member functions that are not trivial you will need to provide them for the union yourself.

Do note that as per bogdan's comments below the empty destructor is not enough. In [class.union]/8 we have

[...]If X is a union its variant members are the non-static data members;[...]

So all members of this union are variants. Then if we look at [class.dtor]/8 we have

After executing the body of the destructor and destroying any automatic objects allocated within the body, a destructor for class X calls the destructors for X’s direct non-variant non-static data members[...]

So the destructor will not automatically destroy the members of the union as they are variants.

You could make a tagged union like kennytm does here

struct TU {
   int type;
   union {
     int i;
     float f;
     std::string s;
   } u;

   TU(const TU& tu) : type(tu.type) {
     switch (tu.type) {
       case TU_STRING: new(&u.s)(tu.u.s); break;
       case TU_INT:    u.i = tu.u.i;      break;
       case TU_FLOAT:  u.f = tu.u.f;      break;
     }
   }
   ~TU() {
     if (tu.type == TU_STRING)
       u.s.~string();
   }
   ...
};

Which ensures the correct member is destroyed or just use a std::variant or boost::variant

Community
  • 1
  • 1
NathanOliver
  • 150,499
  • 26
  • 240
  • 331
  • So this says that the `union`'s copy constructor will be deleted if it has a member with a custom copy constructor right? But that's not what I'm seeing here: http://ideone.com/Cf0OOQ – Jonathan Mee Oct 18 '16 at 12:07
  • @JonathanMee You need to try and call it: http://coliru.stacked-crooked.com/a/656b9885a04be262 – NathanOliver Oct 18 '16 at 12:22
  • @JonathanMee The copy constructor exists, but it is deleted so the code will still compile until you try and make a copy. It fails to compile when the destructor is deleted because everything needs a destructor. – NathanOliver Oct 18 '16 at 12:24
  • Hmmm... your example link seems like a weird juxtaposition of my previous question and my current question. The `string` member does seem well formed to me: http://ideone.com/oacSyi Meaning that the copy constructor is *not* deleted. could there in fact be a caveat somewhere in the standard to allow this? – Jonathan Mee Oct 18 '16 at 12:27
  • 2
    @JonathanMee I can ensure you the copy constructor is deleted. The standard even has an example of a union with a `std::sting` in it in the next paragraph and it states *Since std::string (21.3) declares non-trivial versions of all of the special member functions, U will have an implicitly deleted default constructor, copy/move constructor, copy/move assignment operator, and destructor. To use U, some or all of these member functions must be user-provided* – NathanOliver Oct 18 '16 at 12:35
  • In the comments @Hyat has also suggested that the fact I can initialize without a placement new is a bug in gcc's implementation... Thanks for the answer I'll accept shortly... – Jonathan Mee Oct 18 '16 at 12:45
  • 1
    @JonathanMee It also happens in [clang](http://coliru.stacked-crooked.com/a/936b6fc205d39b3b). Looks like it does direct initialization instead of copy initialization or it is ignoring the deleted copy constructor and is eliding. Definitely not what you would expect. – NathanOliver Oct 18 '16 at 12:48
  • 1
    Looks like Visual Studio is also guilty: http://rextester.com/GUHD89332 A little ironic that the standard forbids this but the bigest compilers all support it :/ – Jonathan Mee Oct 18 '16 at 12:52
  • 2
    @JonathanMee No compiler is guilty of anything in those examples. That initialization doesn't involve the copy constructor of the union; that union is an aggregate, and that's aggregate initialization (it does involve copy initialization of the first union member, the `string`, which does call `string`'s copy constructor). – bogdan Oct 18 '16 at 17:58
  • @bogdan Perhaps you can enlighten me. My understanding from Nathan Oliver's answer was that the union should be explicitly disallowing any construction, because of the deleted copy, value, and default constructors required in his quote of the standard. – Jonathan Mee Oct 18 '16 at 18:11
  • 3
    @JonathanMee Those constructors are indeed deleted, but that doesn't necessarily mean that no construction is possible. An aggregate can still be initialized through aggregate initialization, even if all its constructors are deleted. That's not limited to unions. What *is* specific to unions is that union aggregate initialization is defined to initialize the first member. Try adding a user-provided constructor to that union; say, a default constructor like `S() { }`. The union will no longer be an aggregate and the initialization will have to use a constructor, which will fail. – bogdan Oct 18 '16 at 18:23
  • 1
    @bogdan Good call on that. I forgot unions are considered aggregates. I use them so little. – NathanOliver Oct 18 '16 at 18:27
  • 1
    @JonathanMee Even if the union weren't an aggregate, that initialization still wouldn't involve a copy of the whole object. It would directly call the matching constructor, considering the elements of the initializer list as arguments. That's how copy-list-initialization is defined; the difference from direct-list-initialization is just that the chosen constructor cannot be `explicit`. – bogdan Oct 18 '16 at 18:27
  • 2
    @NathanOliver And I think you're absolutely right to avoid them ;-). That being said, I don't think the quote you used explains why OP's code doesn't call `string`'s destructor; after all, yeah, the union's destructor would have been implicitly defined as deleted, but he provided one; why doesn't it work? I'd say a better quote is in [\[class.dtor\]/8](http://eel.is/c++draft/class.dtor#8); the key word is *non-variant*. The definition for variant members is in [\[class.union.anon\]/4](http://eel.is/c++draft/class.union#anon-4) (a union's members are variant members). – bogdan Oct 18 '16 at 18:38
  • @bogdan Orrginally the OP did not have a destructor originally so the first half of the answer was saying why it was not going to be destroyed since he lacked a destructor. – NathanOliver Oct 18 '16 at 18:41
  • @NathanOliver Ah, OK, then that's saying why that initial version wouldn't have compiled at all. – bogdan Oct 18 '16 at 18:43
  • @bogdan So... clarification here, if `S` had been a `struct` not a `union`, both `str` and `vec` would have gotten cleaned up, even though the `S`'s destructor did not explicitly do so. Why then is this not the case for a `union`? – Jonathan Mee Oct 18 '16 at 18:43
  • 3
    @JonathanMee Because of the quotes I gave above, as far as the standard is concerned. Practically, because the destructor wouldn't (always) know which subobject destructor to call. – bogdan Oct 18 '16 at 18:46
  • Nathan Oliver, I don't know if you'd consider expanding your answer with @bogdan's quote, but after reading through it, I do find that it concisely answers my question. – Jonathan Mee Oct 18 '16 at 18:58
  • 2
    @JonathanMee Updated. – NathanOliver Oct 18 '16 at 19:08
  • 1
    @bogdan Thanks for the quotes. Added to the answer. – NathanOliver Oct 18 '16 at 19:08
  • This implies that the string's constructor is not called, either. But why use placement-new - the union already allocated the space - why not explicitly call the constructor, as you do the destructor? – Richard Whitehead Jan 22 '20 at 09:28
  • @RichardWhitehead There is no way in C++ to explicitly call the constructor like you can the destructor. If you want to reconstruct an object, you need to use placement new and it will call the constructor for you. – NathanOliver Jan 22 '20 at 13:20
  • Hey everybody, a question if I may: when I tried implementing a tagged union, but instead of placement new, simply did `u.s = Thing()`, and also called the `Thing` destructor conditionally in the destructor (like shown here) - I saw the `Thing` destructor was being called twice. Changing `u.s = Thing()` to a placement new `new (&u.s) Thing()` fixed it. Why is that? I thought the destructor of a member of the union should never be called automatically - but seems like it is! Unless we use placement new of course. – Aviv Cohn Jun 24 '20 at 12:34
  • @AvivCohn You shold ask this as a question and include a [mre] – NathanOliver Jun 24 '20 at 12:35
  • @NathanOliver Done: https://stackoverflow.com/questions/62555368/destructor-of-union-member-seems-to-be-called-automatically – Aviv Cohn Jun 24 '20 at 12:46
4

Your example won't compile. Unions have, by default, a deleted destructor. Because of course, what destructor should be called? Surely you can't call both. And nowhere is any information stored about which member was actually constructed. It's up to you to provide a proper destructor.

Here's the output of GCC when trying to compile your code snippet:

In function ‘int main()’:
error: use of deleted function ‘main()::<anonymous union>::~<constructor>()’
       vector<int> vec; } s = { "Hello, world"s };
                                                ^

note: ‘main()::<anonymous union>::~<constructor>()’ is implicitly deleted because the default definition would be ill-formed:
      union { string str;
            ^
G. Sliepen
  • 5,327
  • 1
  • 11
  • 25
  • Thanks for the comment. I have cleaned up the code in question, to add a custom destructor to the `union` definition. [This builds](http://ideone.com/Cf0OOQ) so thanks for the info, but this answer doesn't attempt to address the question: What happens when the `union` goes out of scope? – Jonathan Mee Oct 18 '16 at 11:53
1

You always need to manually call the constructor of the objects in your struct with non trivial types.

Usually you always also need to construct them explicitly too. It seems weird that the assignment here works.

In case of doubt though you can always check the assembly if destructors get called.

The assembly of this code does call the basic_string constructor but not the destructor. So you will have leaks here.

using namespace std;
int main(int argc, char** argv){
    union S { string str;
              vector<int> vec;
              ~S() {} } s = { "Hello, world"s };
}

link to see the assembly: https://godbolt.org/g/wKu3vf

Hayt
  • 4,748
  • 24
  • 34
  • So it turns out there is a reason that the compilers are initializing the `union`: http://stackoverflow.com/questions/40106941/is-a-union-members-destructor-called/40107282?noredirect=1#comment67502209_40107282 – Jonathan Mee Oct 18 '16 at 18:39