Can a templated Pointer class have a virtual destructor?

Question

I ran into a surprising revelation when implementing the pimpl idiom with a home made pointer class (I know: why roll your own? But bear with me). The following three files contain a minimal example:

Pointer.h:

#pragma once 

template <typename T>
class Pointer
{
public:
    Pointer(T*p=0)
        : _p(p)
    {
    }
    virtual ~Pointer()
    {
        delete _p;
    }
private:
    void operator=(const Pointer&);
    Pointer(const Pointer&);

private:
    T*_p;
};

Foo.h:

#pragma once
#include "Pointer.h"

struct Foo
{
    Foo();
    ~Foo();

private:
    void operator=(const Foo&);
    Foo(const Foo&);

private:
    Pointer<struct FooPrivate> p;
};

main.cpp:

#include "Foo.h"

int main(int argc, char* argv[])
{
    Foo foo;
    return 0;
}

Never mind what the innards of Foo.cpp look like. When I compile main.cpp with MSVC 2008, I get the warning:

pointer.h(13) : warning C4150: deletion of pointer to incomplete type 'FooPrivate'; no destructor called

The warning can be avoided by removing the keyword virtual from Pointers destructor.

This makes no sense to me. Is this warning legit, or is it a bug in the MSVC compiler? If so, can I safely ignore the warning?

I know it makes no sense in this case to make the destructor virtual, but remember, this is just a minimal compilable example. My original code is a lot more complex.

Answer 1

Without virtual, there is only one place the destructor is going to be called; within ~Foo, at which point you have presumably fully defined FooPrivate. If another instance of Pointer<FooPrivate> is created elsewhere, you might get the warning back, but since you don't the compiler can tell you're behaving safely.

With virtual, you can theoretically derive from Pointer<FooPrivate>, and that new object could be destroyed from somewhere that FooPrivate isn't fully defined. The compiler isn't positive you don't do this, so it issues a warning. You can safely ignore it in this trivial case, but if you have an actual need for a virtual destructor it might be a good idea to take it to heart.

Answer 2

Since you are providing a destructor for class Foo, the warning appears to be completely incorrect & spurious.

Just to check that I added this code, in file [foo.cpp]:

#include "foo.h"
#include <iostream>
using namespace std;

struct FooPrivate
{
    FooPrivate() { cout << "FooPrivate::<init>" << endl; }
    ~FooPrivate() { cout << "FooPrivate::<destroy>" << endl; }
};

Foo::Foo()
    : p( new FooPrivate )
{
    cout << "Foo::<init>" << endl;
}

Foo::~Foo()
{
    cout << "Foo::<destroy>" << endl;
}

Which yielded the same warning (with Visual C++ 10.0) as you got, but output

FooPrivate::<init>
Foo::<init>
Foo::<destroy>
FooPrivate::<destroy>

Clearly, the executable is not doing what the sillywarning said it would…

Cheers & hth.,