auto_ptr and PIMPL

Discussion in 'C++' started by Victor Bazarov, Jan 16, 2006.

  1. red floyd wrote:
    > 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::pImpl 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
     
    Victor Bazarov, Jan 16, 2006
    #1
    1. Advertising

  2. Victor Bazarov

    Shark Guest

    red floyd wrote:
    > 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.

    >
    > 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<WidgetImpl>? It seems to me that this would be a bit safer as
    > well.
     
    Shark, Jan 16, 2006
    #2
    1. Advertising

  3. Victor Bazarov

    red floyd Guest

    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::pImpl an
    auto_ptr<WidgetImpl>? It seems to me that this would be a bit safer as
    well.
     
    red floyd, Jan 16, 2006
    #3
  4. Victor Bazarov

    Earl Purple Guest

    red floyd wrote:
    > 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::pImpl 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.
     
    Earl Purple, Jan 17, 2006
    #4
  5. red floyd wrote:

    []

    > In this case, is there any thing wrong with making Widget::pImpl 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.
     
    Maxim Yegorushkin, Jan 17, 2006
    #5
  6. Victor Bazarov

    Earl Purple Guest

    Maxim Yegorushkin wrote:
    > red floyd wrote:
    >
    > []
    >
    > > In this case, is there any thing wrong with making Widget::pImpl 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.
     
    Earl Purple, Jan 17, 2006
    #6
  7. Earl Purple wrote:

    []

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

    >
    > Assignment would already be disabled.


    Compiler generates the assignment with X& operator=(X&) signature,
    unless auto_ptr<pimpl> is declared const.
     
    Maxim Yegorushkin, Jan 17, 2006
    #7
  8. * 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::pImpl 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.

    --
    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?
     
    Alf P. Steinbach, Jan 17, 2006
    #8
  9. Victor Bazarov

    red floyd Guest

    Alf P. Steinbach wrote:
    > * red floyd:
    >> [sample code with typos redacted]
    >>
    >> In this case, is there any thing wrong with making Widget::pImpl 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.
    >
    >
    >> It seems to me that this would be a bit safer as well.

    >
    > 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.
     
    red floyd, Jan 17, 2006
    #9
  10. On Tue, 17 Jan 2006 18:35:46 GMT, red floyd <> wrote:

    >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
     
    Bob Hairgrove, Jan 17, 2006
    #10
  11. >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

    --
    Bob Hairgrove
     
    Bob Hairgrove, Jan 21, 2006
    #11
    1. Advertising

Want to reply to this thread or ask your own question?

It takes just 2 minutes to sign up (and it's free!). Just click the sign up button to choose a username and then you can ask your own questions on the forum.
Similar Threads
  1. JackC
    Replies:
    3
    Views:
    609
    Alan Griffiths
    Aug 13, 2004
  2. Siemel Naran

    auto_ptr<Derived> to auto_ptr<Base>

    Siemel Naran, Jan 10, 2005, in forum: C++
    Replies:
    2
    Views:
    1,586
    Dave Rahardja
    Jan 11, 2005
  3. Peteris Krumins
    Replies:
    2
    Views:
    530
    Peteris Krumins
    Aug 31, 2005
  4. Keith Willis

    Pimpl using auto_ptr

    Keith Willis, Nov 7, 2007, in forum: C++
    Replies:
    9
    Views:
    597
    Alf P. Steinbach
    Nov 8, 2007
  5. Sousuke
    Replies:
    9
    Views:
    1,189
    Bart van Ingen Schenau
    Mar 16, 2010
Loading...

Share This Page