auto_ptr and PIMPL

[Victor Bazarov]

It seems that the use of auto_ptr<> is discouraged in many places in
favor of boost::shared_ptr<> (or tr1::shared_ptr<>). But consider a
PIMPL idiom, where there is a strict 1-1 relationship between interface
objects and implementation object, and the implementation object lasts
for the lifetime of the interface object.

i.e.
// header file
class WidgetImpl;
class Widget {
private:
WidgetImpl* const pImpl;
public:
Widget();
~Widget();
// yada yada yada
};

// source file

class WidgetInterface {
private:
friend class WidgetInterface;
WidgetImpl() { }
~WidgetImpl() { }
// yada yada yada
};

Widget::Widget() : pImpl(new WidgetImpl)
{
}

Widget::~Widget()
{
delete pImpl;
}


In this case, is there any thing wrong with making Widget:Impl an
auto_ptr<WidgetImpl>? It seems to me that this would be a bit safer as
well.

What's happened to the Rule of Three here? Can a 'Widget' be
copy-constructed? What about copy-assignment? If it can be copied,
do you know what auto_ptr has rather peculiar "copy" semantics? The
ownership of the implementation is expressed incompletely here. Please
correct and re-post.

V

 

[Shark]

It seems that the use of auto_ptr<> is discouraged in many places in
favor of boost::shared_ptr<> (or tr1::shared_ptr<>). But consider a
PIMPL idiom, where there is a strict 1-1 relationship between interface
objects and implementation object, and the implementation object lasts
for the lifetime of the interface object.

I have another argument in favor of auto_ptr. With auto_ptr<> you are
guaranteed that a regular hosted implementation has the necessary
libraries to compile your project. Boost and TR1 need to be
additionally obtained from other sources. For example on my FreeBSD 6.0
standard installation i only have STL and C++ standard library. I can
install a Boost package, but tr1 doesn't seem to exist as a package.

 

[red floyd]

It seems that the use of auto_ptr<> is discouraged in many places in
favor of boost::shared_ptr<> (or tr1::shared_ptr<>). But consider a
PIMPL idiom, where there is a strict 1-1 relationship between interface
objects and implementation object, and the implementation object lasts
for the lifetime of the interface object.

i.e.
// header file
class WidgetImpl;
class Widget {
private:
WidgetImpl* const pImpl;
public:
Widget();
~Widget();
// yada yada yada
};

// source file

class WidgetInterface {
private:
friend class WidgetInterface;
WidgetImpl() { }
~WidgetImpl() { }
// yada yada yada
};

Widget::Widget() : pImpl(new WidgetImpl)
{
}

Widget::~Widget()
{
delete pImpl;
}


In this case, is there any thing wrong with making Widget:Impl an
auto_ptr<WidgetImpl>? It seems to me that this would be a bit safer as
well.

 

[Earl Purple]

It seems that the use of auto_ptr<> is discouraged in many places in
favor of boost::shared_ptr<> (or tr1::shared_ptr<>). But consider a
PIMPL idiom, where there is a strict 1-1 relationship between interface
objects and implementation object, and the implementation object lasts
for the lifetime of the interface object.

i.e.
// header file
class WidgetImpl;
class Widget {
private:
WidgetImpl* const pImpl;
public:
Widget();
~Widget();
// yada yada yada
};

// source file

class WidgetInterface {
private:
friend class WidgetInterface;
WidgetImpl() { }
~WidgetImpl() { }
// yada yada yada
};

Widget::Widget() : pImpl(new WidgetImpl)
{
}

Widget::~Widget()
{
delete pImpl;
}


In this case, is there any thing wrong with making Widget:Impl an
auto_ptr<WidgetImpl>? It seems to me that this would be a bit safer as
well.

auto_ptr would be fine as long as you disable or overload copying, but
you'd have to do that anyway. Assignment will automatically be disabled
as you cannot assign auto_ptr.

Beware of using boost::shared_ptr here. It might not behave the way you
want it to - because effectively it would make your widget a smart
(shared) pointer to its implementation. If you copied your widget, both
widgets would have a pointer to the SAME implementation. Maybe that's
what you want - may be you want to be able to return your widget from
functions or have a collection of them. Effectively though your widget
would be a wrapper for shared_ptr< Impl > and no more.

(It's very rare you'd want to clone widgets so my guess is that would
be the desired behaviour).

If, for some reason, you did want to clone the widget then you could
either implement that in your copy-constructor (and in operator= using
swap() ) or you could use a smart-pointer that clones (either
automatically or with copy-on-write).

If you just want your widget to have move semantics and no more, you
can create a mover. (In future versions of C++ they will be part of the
language). A simple mover would have a pointer to the implementation
and would snatch it from the widget (either snatch the pointer itself
or snatch the ownership - in this case you also have an ownership
flag). When you create a widget from a mover it receives the ownership
(if the mover has it). I think that boost have created a unique_ptr
that can implement that as a template.

 

[Maxim Yegorushkin]

red floyd wrote:

[]

In this case, is there any thing wrong with making Widget:Impl an
auto_ptr<WidgetImpl>? It seems to me that this would be a bit safer as
well.

You'll have to implement the destructor anyway, otherwise the
compiler-generated possibly inline destructor would end up calling
delete p where p is a pointer to an incomplete type. You have to make
sure that the definition of your pimpl class is accessible from the
destructor.

Also, as others have pointed out, you have to implement the copy ctor
and the assignment or disable them.

 

[Earl Purple]

red floyd wrote:

[]

In this case, is there any thing wrong with making Widget:Impl an
auto_ptr<WidgetImpl>? It seems to me that this would be a bit safer as
well.

You'll have to implement the destructor anyway, otherwise the
compiler-generated possibly inline destructor would end up calling
delete p where p is a pointer to an incomplete type. You have to make
sure that the definition of your pimpl class is accessible from the
destructor.

You wouldn't actually have to implement the destructor, but you should
declare it in your header then implement it empty in your compilation
unit where your WidgetImpl is complete. Easy to forget though.

Also, as others have pointed out, you have to implement the copy ctor
and the assignment or disable them.

Assignment would already be disabled.

 

[Maxim Yegorushkin]

Earl Purple wrote:

[]

Assignment would already be disabled.

Compiler generates the assignment with X& operator=(X&) signature,
unless auto_ptr<pimpl> is declared const.

 

[Alf P. Steinbach]

* red floyd:

It seems that the use of auto_ptr<> is discouraged in many places in
favor of boost::shared_ptr<> (or tr1::shared_ptr<>). But consider a
PIMPL idiom, where there is a strict 1-1 relationship between interface
objects and implementation object, and the implementation object lasts
for the lifetime of the interface object.

i.e.
// header file
class WidgetImpl;
class Widget {
private:
WidgetImpl* const pImpl;
public:
Widget();
~Widget();
// yada yada yada
};

// source file

class WidgetInterface {

Of course you meant 'WidgetImpl' here.

private:
friend class WidgetInterface;

Of course you meant 'Widget' here.

WidgetImpl() { }
~WidgetImpl() { }
// yada yada yada
};

Widget::Widget() : pImpl(new WidgetImpl)
{}

Widget::~Widget()
{
delete pImpl;
}

In this case, is there any thing wrong with making Widget:Impl an
auto_ptr<WidgetImpl>?

Formally, yes there's something wrong, because formally even the
declaration of the std::auto_ptr<WidgetImpl> member requires a complete
WidgetImpl class. Herb Sutter once used std::auto_ptr incorrectly in
this way, in a GOTW article, which was the now infamous case that made
us, or at least me, aware that std::auto_ptr isn't PIMPL-compatible.
Last time I checked that GOTW was still there without comment.

But in practice, although I'd hesitate to use this (guideline: don't use
techniques that rely on obscure language features, or in short, don't
use too "clever" code) the only remaining obstacle is to declare also
the copy constructor Widget(Widget&) -- no const here -- and copy
assignment operator, and define them or not, in the implementation file,
so that all code that actually instantiates std::auto_ptr<WidgetImpl>
has access to the complete WidgetImpl class definition.

Someone wrote an article (I think that was in CUJ) about the in-practice
technique, and brought that article up for discussion in clc++m, which
is where I know it from, although in that article mistakenly thinking it
solved the formal problem.

It seems to me that this would be a bit safer as well.

From a practical point of view, yes -- but not formally.

 

[red floyd]

* red floyd:

[sample code with typos redacted]

In this case, is there any thing wrong with making Widget:Impl an
auto_ptr<WidgetImpl>?

Formally, yes there's something wrong, because formally even the
declaration of the std::auto_ptr<WidgetImpl> member requires a complete
WidgetImpl class. Herb Sutter once used std::auto_ptr incorrectly in
this way, in a GOTW article, which was the now infamous case that made
us, or at least me, aware that std::auto_ptr isn't PIMPL-compatible.
Last time I checked that GOTW was still there without comment.

Thanks. I wasn't aware that auto_ptr<> required a complete definition.
I thought it was usable with incomplete types. If Herb Sutter used it
the wrong way, then I guess I'm in good company

But in practice, although I'd hesitate to use this (guideline: don't use
techniques that rely on obscure language features, or in short, don't
use too "clever" code) the only remaining obstacle is to declare also
the copy constructor Widget(Widget&) -- no const here -- and copy
assignment operator, and define them or not, in the implementation file,
so that all code that actually instantiates std::auto_ptr<WidgetImpl>
has access to the complete WidgetImpl class definition.

Yeah, those were elided as part of the "yada yada yada" section.
Good points though, I need to ensure that the copy constructor and the
assignment operator are private and unimplemented. I know that Victor
complained about the rule of 3. I should have made the elision clear.

Someone wrote an article (I think that was in CUJ) about the in-practice
technique, and brought that article up for discussion in clc++m, which
is where I know it from, although in that article mistakenly thinking it
solved the formal problem.



From a practical point of view, yes -- but not formally.

If it's not formally correct, then I'd probably be better off going with
the initial sample version (WidgetImpl *const pImpl) and manually
deleting it in the destructor.

 

[Bob Hairgrove]

Thanks. I wasn't aware that auto_ptr<> required a complete definition.
I thought it was usable with incomplete types. If Herb Sutter used it
the wrong way, then I guess I'm in good company

There was something called the "grin pointer" which I found on the
internet about three years ago. I believe that the author of this code
(Alan Griffiths, IIRC) has since deprecated it in favor of the Boost
family of smart pointer types (Alan, please correct me if you are
reading this, or provide an update on the URL ... unfortunately, I've
forgotten where it was located). But it still comes in handy when you
don't want/need the Boost things. We used it pretty much exclusively
for auto-deletion with pImpl pointers, and I believe that was exactly
what it was meant to do. <g>

Essentially, you delegate destruction (e.g. call of delete on the
plain pointer) to a static member function whose address is stored in
a member function pointer. As long as the complete type is known by
the time the smart pointer is constructed, it works beautifully. That
means that you can still put just a forward-declaration of T in the
header file, but you must include T's header in the .cpp file which
includes the definition of the class where is defined which has a
GrinPtr<T> as a member.

Here's a very stripped-down version, typed from memory, so caveats are
in place:

typedef void (*DeleteType)();
template<typename T>
class GrinPtr {
private:
static void DeleteFunc(T* ptr) { delete ptr; }
T* m_ptr;
DeleteType m_DeleteFunc;
// disallow default ctor, copy ctor
// and op= by not implementing the following:
GrinPtr();
GrinPtr(GrinPtr const &);
GrinPtr& operator=(GrinPtr const &);
public:
GrinPtr(T* plain)
: m_DeleteFunc(&DeleteFunc)
, m_ptr(plain) { assert(sizeof(T)); }
~GrinPtr() { m_DeleteFunc(m_ptr); }
T* operator->() { return m_ptr; }
T const * operator->() const { return m_ptr; }
T& operator*() { return *m_ptr; }
T const & operator*() const { return *m_ptr; }
};

The assert(sizeof(T)) statement in the body of the constructor will
generate an error if T is still an incomplete type when the GrinPtr<T>
is constructed.

Of course, you can also extend it to accept a constructor argument of
type bool indicating whether or not the pointer is an array pointer
(default value of false comes in handy), or to disallow null pointers.
We even implemented a shared smart pointer, using this as our starting
point, for use in STL containers.

But you really should look at the Boost smart pointers and only resort
to rolling your own as a last resort!

 

[Bob Hairgrove]

Here's a very stripped-down version, typed from memory, so caveats are

in place:

Oops ... got the typedef wrong: it should take T* as argument and be
within the class declaration, of course:

template<typename T>
class GrinPtr {
private:
typedef void (*DeleteType)(T*);
static void DeleteFunc(T* ptr) { delete ptr; }
T* m_ptr;
DeleteType m_DeleteFunc;
// disallow default ctor, copy ctor
// and op= by not implementing the following:
GrinPtr();
GrinPtr(GrinPtr const &);
GrinPtr& operator=(GrinPtr const &);
public:
GrinPtr(T* plain)
: m_DeleteFunc(&DeleteFunc)
, m_ptr(plain) { assert(sizeof(T)); }
~GrinPtr() { m_DeleteFunc(m_ptr); }
T* operator->() { return m_ptr; }
T const * operator->() const { return m_ptr; }
T& operator*() { return *m_ptr; }
T const & operator*() const { return *m_ptr; }
};

Here's a link to Alan's code:

http://www.octopull.demon.co.uk/arglib/TheGrin.html