Pimpl with smart pointers in a class with a template constructor: weird incomplete type issue

Question

When using smart pointers with the pImpl idiom, as in

struct Foo
{
private:
    struct Impl; 
    boost::scoped_ptr<Impl> pImpl;
};

the obvious problem is that Foo::Impl is incomplete at the point where the destructor of Foo is generated.

Compilers usually emit a warning there, and boost::checked_delete, which is used internally by Boost smart pointers, statically asserts that the class Foo::Impl is complete and triggers an error if it is not the case.

For the above example to compile, one therefore must write

struct Foo
{
    ~Foo();

private:
    struct Impl; 
    boost::scoped_ptr<Impl> pImpl;
};

and implement an empty Foo::~Foo in the implementation file, where Foo::Impl is complete. This is an advantage of smart pointers over bare pointers, because we can't fail to implement the destructor.

So far, so good. But I came across a weird behaviour when I try to introduce a template constructor in a similar Bar class (full code, please try it yourself):

// File Bar.h
#ifndef BAR_H
#define BAR_H 1

#include <vector>
#include <boost/scoped_ptr.hpp>

struct Bar
{
    template <typename I>
    Bar(I begin, I end);

    ~Bar();

private:
    struct Impl;
    boost::scoped_ptr<Impl> pImpl;

    void buildImpl(std::vector<double>&);
};


template <typename I>
Bar::Bar(I begin, I end)
{
    std::vector<double> tmp(begin, end);
    this->buildImpl(tmp);
}

#endif // BAR_H

// File Bar.cpp
#include "Bar.h"

struct Bar::Impl
{
    std::vector<double> v;
};

void Bar::buildImpl(std::vector<double>& v)
{
    pImpl.reset(new Impl);
    pImpl->v.swap(v);
}

Bar::~Bar() {}

// File Foo.h
#ifndef FOO_H
#define FOO_H 1

#include <boost/scoped_ptr.hpp>


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

private:
    struct Impl;
    boost::scoped_ptr<Impl> pImpl;
};

#endif // FOO_H

// File Foo.cpp
#include "Foo.h"

struct Foo::Impl
{};


Foo::Foo() : pImpl(new Impl)
{}


Foo::~Foo() {}


// File Main.cpp
#include "Foo.h"
#include "Bar.h"

int main()
{
    std::vector<double> v(42);
    Foo f;
    Bar b(v.begin(), v.end());
}

When compiling this example with Visual Studio 2005 SP1, I get an error with Bar but not with Foo:

1>Compiling...
1>main.cpp
1>c:\users\boost_1_45_0\boost\checked_delete.hpp(32) : error C2027: use of undefined type 'Bar::Impl'
1>        c:\users\visual studio 2005\projects\checkeddeletetest\checkeddeletetest\bar.h(15) : see declaration of 'Bar::Impl'
1>        c:\users\boost_1_45_0\boost\smart_ptr\scoped_ptr.hpp(80) : see reference to function template instantiation 'void boost::checked_delete<T>(T *)' being compiled
1>        with
1>        [
1>            T=Bar::Impl
1>        ]
1>        c:\users\boost_1_45_0\boost\smart_ptr\scoped_ptr.hpp(76) : while compiling class template member function 'boost::scoped_ptr<T>::~scoped_ptr(void)'
1>        with
1>        [
1>            T=Bar::Impl
1>        ]
1>        c:\users\visual studio 2005\projects\checkeddeletetest\checkeddeletetest\bar.h(16) : see reference to class template instantiation 'boost::scoped_ptr<T>' being compiled
1>        with
1>        [
1>            T=Bar::Impl
1>        ]
1>c:\users\boost_1_45_0\boost\checked_delete.hpp(32) : error C2118: negative subscript
1>c:\users\boost_1_45_0\boost\checked_delete.hpp(34) : warning C4150: deletion of pointer to incomplete type 'Bar::Impl'; no destructor called
1>        c:\users\visual studio 2005\projects\checkeddeletetest\checkeddeletetest\bar.h(15) : see declaration of 'Bar::Impl'

I will try this with a recent gcc as soon as I get home.

I don't understand what is going on: at the point where the destructor is defined (ie. in Bar.cpp), a definition of Bar::Impl is available, so there should not be any problem. Why does this work with Foo and not with Bar?

What am I missing here ?

score 5 · Accepted Answer · answered Feb 07 '11 at 13:19

5

This is the destructor of boost::shared_ptr<> that requires the object to be complete when using boost::checked_deleter<> deleter. Because you put the range constructor Bar::Bar(I begin, I end) in the header file the compiler must generate code that destroys already constructed members if your constructor throws, hence it is trying to instantiate boost::scoped_ptr<T>::~scoped_ptr(void) when instantiating this template constructor.

It is less than useful to use smart pointers with pimpl. Since you normally need to provide a destructor anyway, you could as well put delete pimpl in that destructor and be done with that.

answered Feb 07 '11 at 13:19

Maxim Egorushkin

119,842
14
147
239

Okay, I see what I was missing. The question is a toy simplification of a real world issue that happened to me today. The actual pointer class used is `boost::shared_ptr`, thus I can't have a bare pointer here. I will investigate using a custom deleter. Thanks. – Alexandre C. Feb 07 '11 at 13:28
3

I disagree with the less than useful. **Because** of the use of a `scoped_ptr` the issue is highlighted by the compiler if you forget to write the destructor, also the destructor body is extremely simple (empty...). On the other hand, using a raw pointer you would not be notified and would have to make the call. Also, using `shared_ptr`, thanks to the embedded deleter, actually relieves you from the burden of writing a destructor at all. – Matthieu M. Feb 07 '11 at 13:59
@Matthieu: It is a matter of taste, I prefer simple and powerful code, no unnecessary includes just for the comfort of it. – Maxim Egorushkin Feb 07 '11 at 14:13
I understand, I personally use a custom implementation of smart pointer `Pimpl` class, which wraps up a customer deleter (like Boost) to provide copying / destruction "for free". It's on the heavy side (though only takes a hundred lines of code or so), but not forcing the developer to write the Big Three while still cutting down on dependencies seems like a win-win. – Matthieu M. Feb 07 '11 at 14:23

score 1 · Answer 2 · answered Jun 11 '12 at 12:36

From the Boost Boost documentation:

Note that scoped_ptr requires that T be a complete type at destruction time, but shared_ptr does not.

Switch to shared_ptr and all should be well - no need to have a destructor either (empty or otherwise). If you want to make the class noncopyable so it has the semantics you'd have got from scoped_ptr, inherit (privately) from boost::noncopyable.

Pimpl with smart pointers in a class with a template constructor: weird incomplete type issue

2 Answers2