auto_ptr and PIMPL

[red floyd]

It seems that the use of auto_ptr<> is discouraged in many places in
favor of boost::shared_p tr<> (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::pImpl an
auto_ptr<Widget Impl>? It seems to me that this would be a bit safer as
well.

[Victor Bazarov]

Re: auto_ptr and PIMPL

red floyd wrote:[color=blue]
> It seems that the use of auto_ptr<> is discouraged in many places in
> favor of boost::shared_p tr<> (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::pImpl an
> auto_ptr<Widget Impl>? It seems to me that this would be a bit safer as
> well.
>[/color]

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]

Re: auto_ptr and PIMPL

red floyd wrote:[color=blue]
> It seems that the use of auto_ptr<> is discouraged in many places in
> favor of boost::shared_p tr<> (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.[/color]

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.
[color=blue]
>
> 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::pImpl an
> auto_ptr<Widget Impl>? It seems to me that this would be a bit safer as
> well.[/color]

[Earl Purple]

Re: auto_ptr and PIMPL


red floyd wrote:[color=blue]
> It seems that the use of auto_ptr<> is discouraged in many places in
> favor of boost::shared_p tr<> (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::pImpl an
> auto_ptr<Widget Impl>? It seems to me that this would be a bit safer as
> well.[/color]

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_p tr 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]

Re: auto_ptr and PIMPL


red floyd wrote:

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

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]

Re: auto_ptr and PIMPL


Maxim Yegorushkin wrote:[color=blue]
> red floyd wrote:
>
> []
>[color=green]
> > In this case, is there any thing wrong with making Widget::pImpl an
> > auto_ptr<Widget Impl>? It seems to me that this would be a bit safer as
> > well.[/color]
>
> 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.[/color]

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.
[color=blue]
> Also, as others have pointed out, you have to implement the copy ctor
> and the assignment or disable them.[/color]

Assignment would already be disabled.

[Maxim Yegorushkin]

Re: auto_ptr and PIMPL


Earl Purple wrote:

[]
[color=blue][color=green]
> > Also, as others have pointed out, you have to implement the copy ctor
> > and the assignment or disable them.[/color]
>
> Assignment would already be disabled.[/color]

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

[Alf P. Steinbach]

Re: auto_ptr and PIMPL

* red floyd:[color=blue]
> It seems that the use of auto_ptr<> is discouraged in many places in
> favor of boost::shared_p tr<> (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 {[/color]

Of course you meant 'WidgetImpl' here.

[color=blue]
> private:
> friend class WidgetInterface ;[/color]

Of course you meant 'Widget' here.
[color=blue]
> WidgetImpl() { }
> ~WidgetImpl() { }
> // yada yada yada
> };[/color]
[color=blue]
>
> Widget::Widget( ) : pImpl(new WidgetImpl)
> {}
>
> Widget::~Widget ()
> {
> delete pImpl;
> }
>
> In this case, is there any thing wrong with making Widget::pImpl an
> auto_ptr<Widget Impl>?[/color]

Formally, yes there's something wrong, because formally even the
declaration of the std::auto_ptr<W idgetImpl> 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<W idgetImpl>
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.

[color=blue]
> It seems to me that this would be a bit safer as well.[/color]

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

--
A: Because it messes up the order in which people normally read text.
Q: Why is it such a bad thing?
A: Top-posting.
Q: What is the most annoying thing on usenet and in e-mail?

[red floyd]

Re: auto_ptr and PIMPL

Alf P. Steinbach wrote:[color=blue]
> * red floyd:[color=green]
>> [sample code with typos redacted]
>>
>> In this case, is there any thing wrong with making Widget::pImpl an
>> auto_ptr<Widget Impl>?[/color]
>
> Formally, yes there's something wrong, because formally even the
> declaration of the std::auto_ptr<W idgetImpl> 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.[/color]

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 :)
[color=blue]
> 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<W idgetImpl>
> has access to the complete WidgetImpl class definition.[/color]

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.[color=blue]
>
> 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.
>
>[color=green]
>> It seems to me that this would be a bit safer as well.[/color]
>
> From a practical point of view, yes -- but not formally.
>[/color]

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]

Re: auto_ptr and PIMPL

On Tue, 17 Jan 2006 18:35:46 GMT, red floyd <no.spam@here.d ude> wrote:
[color=blue]
>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 :)[/color]

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<typena me 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=(GrinP tr const &);
public:
GrinPtr(T* plain)
: m_DeleteFunc(&D eleteFunc)
, 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
NoSpamPlease@Ho me.com

[Bob Hairgrove]

Re: auto_ptr and PIMPL

>Here's a very stripped-down version, typed from memory, so caveats are[color=blue]
>in place:[/color]

Oops ... got the typedef wrong: it should take T* as argument and be
within the class declaration, of course:
[color=blue]
>template<typen ame 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=(GrinP tr const &);
>public:
> GrinPtr(T* plain)
> : m_DeleteFunc(&D eleteFunc)
> , 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; }
>};[/color]

Here's a link to Alan's code:

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

--
Bob Hairgrove
NoSpamPlease@Ho me.com