views:

344

answers:

5

I had a question about C++ destructor behavior, more out of curiosity than anything else. I have the following classes:

Base.h

class BaseB;

class BaseA
{
    public:
        virtual int MethodA(BaseB *param1) = 0;
};

class BaseB
{
};

Imp.h

#include "Base.h"
#include <string>

class BImp;

class AImp : public BaseA
{
    public:
        AImp();
        virtual ~AImp();

    private:
        AImp(const AImp&);
        AImp& operator= (const AImp&);

    public:
        int MethodA(BaseB *param1) { return MethodA(reinterpret_cast<BImp *>(param1)); }

    private:
        int MethodA(BImp *param1);
};

class BImp : public BaseB
{
    public:
        BImp(std::string data1, std::string data2) : m_data1(data1), m_data2(data2) { }
        ~BImp();
        std::string m_data1;
        std::string m_data2;

    private:
        BImp();
        BImp(const BImp&);
        BImp& operator= (const BImp&);
};

Now, the issue is that with this code, everything works flawlessly. However, when I make the destructor for BImp virtual, on the call to AImp::MethodA, the class BImp seems to have its data (m_data1 and m_data2) uninitialized. I've checked and made sure the contained data is correct at construction time, so I was wondering what the reason behind this could be...

Cheers!

Edit: param1 was actually a reference to B in MethodA. Looks like I over-sanitized my real code a bit too much!

Edit2: Re-arranged the code a bit to show the two different files. Tested that this code compiles, a well. Sorry about that!

+2  A: 

MethodA takes its parameters by value. This means a copy is passed (and the copy has to be destroyed). That's my best guess for why you might have a BImpl being destroyed that you didn't expect to be, but I don't see what the virtual or non-virtual nature of A's destructor could possibly have to do with it.

But this code can't compile - you use class B in declaring the virtual function in A, but B isn't defined until later. And I don't know what's going on with that cast - you can't reinterpret_cast class types. Perhaps if you work up a test case which demonstrates your issue, and post that?

Steve Jessop
+9  A: 

If you are casting between related types as you do in this case, you should use static_cast or dynamic_cast, rather than reinterpret_cast, because the compiler may adjust the object pointer value while casting it to a more derived type. The result of reinterpret_cast is undefined in this case, because it just takes the pointer value and pretends it's another object without any regard for object layout.

Alex B
There was a typo in my code from over-sanitization. The actual MethodA accepts parameters by reference.
sohum
Edited. ...(at least 15 characters required)
Alex B
Hmm... let me try that out. So the virtual destructor changing behavior is purely coincidental?
sohum
Yes, it *may* be.
Alex B
Changed it to a static_cast since dynamic_cast is illegal (BaseB is not polymorphic). The behavior of the program is correct, now, although the debugger shows that the data members are bad. I'm doing a full rebuild to see if it's just cached debug data.
sohum
Yup, the full rebuild updated debug info! Seems like the reinterpret_cast was removing some information.
sohum
+1  A: 

There's a lot of iffy stuff in this code, so I'm amazed that it works or compiles in any case.

  • Passing parameters by value instead of reference to MethodA
  • Casting a B to a BImp via reinterpret_cast -- bad idea! If you're going to cast in that direction, dynamic_cast is the safest.
  • I fail to see how you're supposed to get a BImp out of a B. You are not invoking any constructors, and you have none that could be invoked that would accept a B. Your default constructor for BImp is private, and assigning a B that has no data, casted to a BImp that still has no data, to a BImp, still isn't going to give you any data!
Aaron Klotz
A: 

Your code is ill-formed. It is not valid C++. In C++ language reinterpret_cast can only be used to cast between pointer types, reference types, to perform pointer-to-integer conversions (in either direction).

In your code you are trying to use reinterpret_cast to convert from type B to type BImp. This is explicitly illegal in C++. If your compiler allows this code, you have to consult your compiler's documentation in order to determine what's going on.

Other replies already mentioned "slicing". Keep in mind that this is nothing more than just a guess about specific non-standard behavior of your specific compiler. It has nothing to do with C++ language.

AndreyT
+1  A: 

Several comments:

  • Your base classes should have virtual destructors so the derived class' dtor is called instead of the just the base class dtor when the object is deleted.

  • MethodA taking a BaseB pointer as a parameter only to have the pointer reinterpreted as a BImp (a derived class of BaseB) is dangerous. There is no guarantee something else other than BImp is passed to MethodA. What would happen if just a BaseB object was to MethodA? Potentially lots of bad things, I would suspect.

  • I'm guessing your code "works flawlessly" because you only pass BImp to MethodA. If you are only passing BImp to MethodA then make the signature match the intent (this has the added benefit of removing that awful reinterpret call).

TheGreatAvatar