1

I mean the following. I have a few classes which inherit the same base class. Union consists of pointers of these classes:

#include "stdio.h"

class A {
public:
    A() { printf("A\n"); }
    virtual ~A() { printf("~A\n"); }
};

class B  : public A {
public:
    B() { printf("B\n"); }
    virtual ~B() { printf("~B\n"); }
};

class C : public A {
public:
    C() { printf("C\n"); }
    virtual ~C() { printf("~C\n"); }
};

int main() {
    union {
        B* b;
        C* c;
    } choice;
    choice.b = new B();
    delete choice.c;    //We have B object, but deleting C
    return 0;
}

It seems to work, but I'm not sure if it isn't implementation-specific behaviour. Can I use such weird deleting method or should I remember a type of stored object and delete it respectively?

P.S. I use C++11 and want it works on both GCC and Visual C++ (2012 and higher). In a real project I have more complex class hierarchy but all of them are successors (directly or indirectly) of the same abstract base class

user2717575
  • 369
  • 7
  • 16
  • 6
    Obvious undefined behavior is obvious. – T.C. Jul 20 '14 at 19:45
  • The members of a union occupy the same bytes. You are assigning member `b` which overwrites member `c`, then calling `C::~C` with that pointer to `B`. – Khouri Giordano Jul 20 '14 at 19:48
  • What you want to do is reasonable, but you have to have some way of knowing what type of object you have there. Also, is it so important to save the size of that extra pointer? Why can't you just have two pointers? – Khouri Giordano Jul 20 '14 at 19:50
  • You may be getting lucky in that the vtables for B and C align. You cannot count on this for any non-trivial (or even trivial) case. Why not just do A *object = new B() or new C(), as dictated by your needs? – Steger Jul 20 '14 at 19:50
  • Visual C++ 2012 prints the following: `A` `B` `~B` `~A` There is no ~C calling – user2717575 Jul 20 '14 at 19:51
  • 1
    @Steger, I have to use union with pointers of different classes. I just wanted to get rid of huge switch when deleting the data. – user2717575 Jul 20 '14 at 19:54
  • My point (and that of answers below) is that you only need to store a pointer to the base class, so... no need for a union. Once you get into the whole mode of oo thinking, programming gets a lot more elegant! You simply call delete on the base class and the correct derived class deleter gets called for you. – Steger Jul 20 '14 at 19:56
  • @Steger, Yes, you are right. It would be easier. But it seems that my particular task requires using union to avoid huge amount of static_cast's (I have to have getters, setters, operators == and copy/move operations. All these things would require static_cast). But I see your point – user2717575 Jul 20 '14 at 20:24
  • @T.C. Was that a Cinema Sins reference? :D – David G Jul 20 '14 at 20:42

4 Answers4

5

This is a double dose of undefined behavior. First, you can't delete a B through a pointer to C. §5.3.5 [expr.delete]/p3:

In the first alternative (delete object), if the static type of the object to be deleted is different from its dynamic type, the static type shall be a base class of the dynamic type of the object to be deleted and the static type shall have a virtual destructor or the behavior is undefined. In the second alternative (delete array) if the dynamic type of the object to be deleted differs from its static type, the behavior is undefined.

Second, accessing the inactive member of a union is also undefined behavior in C++.

There's no need to use an union here anyway. B and C share the same base class, so you can just store the pointer in an A *.

Community
  • 1
  • 1
T.C.
  • 133,968
  • 17
  • 288
  • 421
2

You shouldn't. You are only allowed to read from the union member you last wrote into and you're only allowed to delete an object through a pointer to a base class (if it has a virtual destructor). It may seem to work now, but you may find it to break randomly in the future, usually due to an aggressive optimizer.

Why don't you store a pointer to A instead of the union?

avakar
  • 32,009
  • 9
  • 68
  • 103
  • B and C contain additional methods and fields. Therefore I should use union with different pointers because of higher performance. I access to this union through special methods which check presence and returns nullptr or valid pointer of need type. I think that using dynamic_cast will decrease performance – user2717575 Jul 20 '14 at 19:57
  • @user2717575, what do you mean `dynamic_cast`? In your current solution, you must already have some mechanism to determine the dynamic type of the object in order to access the correct member of the union. With `A*`, you use the same mechanism and then you simply `static_cast`. – avakar Jul 20 '14 at 20:04
  • Yes, you are absolutely right, static_cast will work but as far as I know it's not safe to static_cast from A* to B*. That's why I can't dare to use static_cast from A* to B* even if I know it will be correct. – user2717575 Jul 20 '14 at 20:09
  • 1
    @user2717575, then you may now rest easy: you can safely `static_cast` from `A*` to `B*` as long as the dynamic type of the object is derived from `B`. – avakar Jul 20 '14 at 20:12
1

As it has been said in other answer, this is not proper C++.

My impression is that you want to keep an union of pointers because in certain circumstances you need an instance of a (sub)class of B, and in another an instance of C, with the issue of B and C having not quite the same interface. Perhaps you store several of these in a container, or simply you don't know until runtime which instance will be used.

So you may keep your code as it was, with perhaps a type tag somewhere indicating which instance has been created, and then use a switch each time you need to determine the correct code to run, or you could leverage your classes to actually invoke the proper function at run time, by including in the common base class of B and C(1) a virtual method, and overload this method in B and C with the proper branch of the switch, then replace the union with a simple pointer to the base class.


(1) that base class doesn't have to be A: if you don't want to clutter your class tree, just make a different class having the minimal interface needed there, and thanks to C++ multiple inheritance, have B and C inherit from it as well. Don't forget the virtual destructor!

didierc
  • 14,572
  • 3
  • 32
  • 52
-1

For me this case looks legit and there is no undefined behaviour.

  1. He is using type-punning and it is legit because it B* b and C* c in union members is a pointers and could be converted to char array.
  2. Both B and C have virtual destructor because of base class (not because base is a same! but because base have virtual destructor).

    12.4.9 If a class has a base class with a virtual destructor, its destructor (whether user- or implicitly-declared) is virtual.

  3. While destructor call, (because it is virtual) exact function address will be picked up from choice variable and proper destructor sequence will be called. So there is no ANY undefined behaviour at all.
Kirilodius
  • 136
  • 1
  • 5
  • _"He is using type-punning"_ - which is not allowed in C++. – Captain Obvlious Jul 20 '14 at 22:53
  • 9.5 Note: One special guarantee is made in order to simplify the use of unions: If a standard-layout union contains several standard-layout structs that share a common initial sequence (9.2), and if an object of this standard-layout union type contains one of the standard-layout structs, it is permitted to inspect the common initial sequence of any of standard-layout struct members; see 9.2. —end note – Kirilodius Jul 20 '14 at 23:12
  • @Kirilodius: Does that mean I don't have to make `struct box { enum contenttype, union content { struct type1 {...}, struct type2 {...} } }` but I can go with `union box { struct type1 { enum contenttype, ... }, struct type2 { enum contenttype, ... } }` ? – SF. Jul 25 '14 at 11:20
  • If your union and struct definition are fits into standard-layout definition - yes. – Kirilodius Jul 25 '14 at 12:34