The compiler shoots itself in the foot when trying to optimise/inline my trivially looking but non-trivial dtor, what am I doing wrong?

Question

I have this shared pimpl*. It forward declares the implementation object and has a custom-implemented shared pointer object to implement the pimpl idiom (again, with sharing semantics). Condensed, it looks like this:

Foo.h

#include "SharedPtr.h"

class Foo_impl;
class FooFactory;
class Foo {
  friend class FooFactory;
  private:
    SharedPtr<Foo_impl> pimpl;
    Foo(SharedPtr<Foo_impl>);
  public:
    ~Foo();
};
struct FooFactory {
  Foo build() const;
};

Foo.cpp

#include "Foo.h"

Foo FooFactory::build() const {
  return Foo(SharedPtr<Foo_impl>(new Foo_impl(/*...*/)));
}
Foo::Foo(SharedPtr<Foo_impl> pimpl)
  : pimpl(pimpl) {
}
Foo::~Foo() {
}

Now, (I think) the compiler gets really smart when compiling Bar.cpp (which uses Foo objects and other SharedPtr objects) and complains:

SharedPtr.h: In member function ‘void Deallocator<T>::operator()(T*) const [with T = Foo_impl]’:
SharedPtr.h:54:   instantiated from ‘void SharedPtr<T, Delete>::drop() [with T = Foo_impl, Delete = Deallocator<Foo_impl>]’
SharedPtr.h:68:   instantiated from ‘SharedPtr<T, Delete>::~SharedPtr() [with T = Foo_impl, Delete = Deallocator<Foo_impl>]’
SharedPtr.h:44: warning: possible problem detected in invocation of delete operator:
SharedPtr.h:42: warning: ‘t’ has incomplete type
Foo.h:29: warning: forward declaration of ‘struct Foo_impl’
SharedPtr.h:44: note: neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined.

Who could possibly be calling ~SharedPtr<Foo_impl> other than Foo and FooFactory? Where does that come from and how can I fix it?

Note: Making ~Foo virtual doesn't help, which is even more puzzling to me.

_{* The fact that the impl is shared is irrelevant here, I just want to prevent the typical "please define a copy-ctor/assignment method" comments. It is fully intentional that the pimpl pointer is shared.}

Edit: The SharedPtr interface:

 33     template <typename T> struct Deallocator {
 34       private:
 35         bool doDelete; // not const to be def. assignable
 36       public:
 38         Deallocator(bool doDelete = true) : doDelete(doDelete) {}
 39         bool willDelete() const {
 40           return doDelete;
 41         }
 42         void operator()(T* t) const {
 43           if (doDelete)
 44             delete t;
 45         }
 46     };
 47
 48     template <typename T, typename Delete = Deallocator<T> > class SharedPtr : private SharedPtrBase {
 49       private:
 50         Delete del;
 51         T* ptr;
 52         void drop() {
 53           if (ptr && shouldDelete()) {
 54             del(ptr);
 55           }
 56           ptr = NULL;
 57           leave();
 58         }
 59       public:
 60         // SharedPtr(p,false) will not delete the pointer! Useful for Stackobjects!
 61         explicit SharedPtr(T* ptr = NULL, Delete del = Delete())
 62           : SharedPtrBase(), del(del), ptr(ptr) {
 63         }
 64         SharedPtr(SharedPtr const& from)
 65           : SharedPtrBase(from), del(from.del), ptr(from.ptr) {
 66         }
 67         ~SharedPtr() {
 68           drop();
 69         }
 70         SharedPtr& operator=(SharedPtr const& from) {
 71           if (&from != this) {
 72             drop();
 73             del = from.del;
 74             ptr = from.ptr;
 75             join(&from);
 76           }
 77           return *this;
 78         }
 79         SharedPtr& operator=(T* from) {
 80           return *this = SharedPtr(from,del);
 81         }
 ...

@ildjarn: It's a project-specific custom shared pointer type. `Deallocator` is also a custom `delete` wrapper. — bitmask, Jun 29 '12 at 20:01
And you don't think its implementation might be pertinent to your question..? — ildjarn, Jun 29 '12 at 20:02
Please post the definition of `SharedPtr`. It is likely that it requires the template parameter to be a complete type in its assignment operator. — CB Bailey, Jun 29 '12 at 20:03
@ildjarn: I don't see why, it works essentially like `std::shared_ptr` other than having an `explicit` ctor. But I'll add the implementation of `Deallocator` -- I guess that could be useful. — bitmask, Jun 29 '12 at 20:05
`std::shared_ptr` can work with incomplete types, your `SharedPtr` clearly does not. — ildjarn, Jun 29 '12 at 20:06
@CharlesBailey, ildjarn: I added the relevant parts of the implementation. — bitmask, Jun 29 '12 at 20:10

score 2 · Accepted Answer · answered Jun 29 '12 at 20:18

2

You don't declare an assignment operator for Foo so if you use it the compiler will define one for you. The compiler generated one will use then copy assignment operator of SharedPtr which, through a number of intermediate functions, calls delete on a Foo_impl.

I can't see your Bar.cpp so I can't say where you might be copying a Foo.

answered Jun 29 '12 at 20:18

CB Bailey

755,051
104
632
656

Ah! Thanks. I now defined a custom destruction object (so it wont use the default `Deallocator`), but your explanation helps me understand where the original problem came from. Didn't see it. – bitmask Jun 29 '12 at 20:26

The compiler shoots itself in the foot when trying to optimise/inline my trivially looking but non-trivial dtor, what am I doing wrong?

Foo.h

Foo.cpp

1 Answers1