views:

445

answers:

5

Microsoft Visual Studio 2008 is giving me the following warning:

warning C4150: deletion of pointer to incomplete type 'GLCM::Component'; no destructor called

This is probably because I have defined Handles to forward declared types in several places, so now the Handle class is claiming it won't call the destructor on the given object.

I have VLD running and I'm not seeing any leaks. Is this literally not calling the destructor for this object or is this a "may not call destructor for object" warning?

Yet another memory leak question from me, haha.

A: 

It sounds like you're not following the p-impl idiom correctly.

Brief example:

// header
struct Other; // declare to use a pointer to it

struct Handle {
  Handle();
  ~Handle();

  void f();

private:
  Other* _obj;
};

// implementation (.cpp)
#include "header"

struct Other {
  void f();
};

Handle() : _obj(new Other()) {}
~Handle() { delete _obj; }

void Handle::f() { _obj->f(); }

Since using delete is now after the definition of class Other, the type will be complete. Without a complete type, the compiler can't know how to properly destroy it. (E.g. the dtor could be virtual or non-virtual, or even might be non-public.)

Roger Pate
I'm using a template Handle class, so does that mean I should never forward declare the type passed in for that handle?
Reggie
Ah, see, that's important info left out of the question. :) You will need the type to be complete at that point (and you won't be doing p-impl), unless you want to learn the difference between template instantiation and specialization, among other corners.
Roger Pate
Do you want p-impl? If so, use a non-template instead of a template. If not, and you cannot provide a complete type (the class definition of Other in my example), what role does Handle serve for you?
Roger Pate
If you use a template for the handle, then you just have to implement (potentially empty) constructors / destructors, *and put them into the .cpp file*, after the definition of the body class.
Johannes Schaub - litb
@Roger, not sure, but i think he's just using a `auto_ptr<Other>` or similar, and sticks it into `Handle`.
Johannes Schaub - litb
+3  A: 

Per the C++ Standard (ISO/IEC 14882:2003 5.3.5/5):

If the object being deleted has incomplete class type at the point of deletion and the complete class has a non-trivial destructor or a deallocation function, the behavior is undefined.

So, if your class has a non-trivial destructor, don't do this, regardless of how Visual C++ handles this situation.

James McNellis
+1  A: 

In g++, the warning could be reproduced too using following code:

class A;

void func(A *p)
{
        delete p;
}

int main(int argc, char **argv)
{
        return 0;
}

Warnings but no any errors:

test.cpp: In function void func(A*):
test.cpp:6: warning: possible problem detected in invocation 
  of delete operator:
test.cpp:4: warning: A has incomplete type
test.cpp:2: warning: forward declaration of struct A
test.cpp:6: note: neither the destructor nor the class-specific
  operator delete will be called, even if they are declared
  when the class is defined.

While g++ states very clear here that destructor will not be called because it doesn't know whether needs a destructor or not, or where is the destructor.

In this case delete() downgrates to the C call free(), that is, it just free the object memory itself, but any heap data allocated by the object itself internally (e.g., in constructor) will be keeped, the same behavior as free(p).

I think it would be a similar way if in MS VC/++; and a correct way is you to include the interface headers of A.

EffoStaff Effo
+1  A: 

You may have no leaks even with the destructor not called if the object being deleted doesn't contain pointers to heap-allocated objects. However don't try this - as James McNellis mentions in his answer it's undefined behaviour according to the C++ standard.

When delete is called (explicitly or from a smart pointer) first the object destructor is run and then memory is deallocated. VC++ basically tells you that it has no idea of what destructor to run and so will not run any but instead will just deallocate memory. If the object is a POD type or it has no pointers set to heap-allocated objects (either contains no pointers or contains pointers set to null or to objects owned by someone else and therefore not needed to deallocate) there's no reason for a leak.

So perhaps VC++ will produce the same behaviour as it would with first casting the object to void*. But again don't rely on this - at best you will have unportable code, at worst your code will break apart as soon as you change version of or patch your VC++ compiler.

sharptooth
+1  A: 

It often happen when using Pimpl, so I'll focus on the solution there:

class FooImpl;

class Foo
{
public:
  // stuff
private:
  Pimpl<FooImpl> m_impl;
};

The problem here is that unless you declare a destructor, it will be automatically generated, inline, by the compiler. But of course, the compiler will have no idea of the complete type of FooImpl there.

You thus have to explicitly define the destructor, even if empty, and put the definition somewhere where the complete type of FooImpl is visible.

// cpp file
class FooImpl
{
};

Foo::~Foo() {} // Empty, but now correctly generated
               // because FooImpl complete at this point.

Also, if like me you defined your Pimpl class to be pretty smart (regarding construction, copy and assignment), then those will also need to be defined in the .cpp file.

It's really a hassle, but then you have nicely encapsulated your implementation details, so I suppose it's worth it.

Matthieu M.