16

When using the pImpl idiom is it preferable to use a boost:shared_ptr instead of a std::auto_ptr? I'm sure I once read that the boost version is more exception friendly?

class Foo
{
public:
    Foo();
private:
    struct impl;
    std::auto_ptr<impl> impl_;
};

class Foo
{
public:
    Foo();
private:
    struct impl;
    boost::shared_ptr<impl> impl_;
};

[EDIT] Is it always safe to use std::auto_ptr<> or are there situations when an alternative boost smart pointer is required?

Oleg Svechkarenko
  • 2,340
  • 22
  • 28
Rob
  • 70,036
  • 53
  • 151
  • 189
  • 2
    Something really bugs me: you're willing to use the pimpl idiom, a compiler firewall. So you're trying to limit compile time dependencies. However, you're happily injecting a smart pointer implementation (`scoped_ptr`, `shared_ptr`, `auto_ptr` or whatever) while your "shell" class is ONLY forwarding calls to the implementation class and deleting the pointer in its destructor. Somehow, I feel like if the shell class does something more, then design might be broken. – Gregory Pakosz Dec 07 '10 at 16:07
  • I believe this deserves an update for the new C++. Herb Sutter recently [blogged](http://herbsutter.com/gotw/_100/) about this. – Björn Pollex Nov 18 '11 at 10:04
  • Similar question: http://stackoverflow.com/questions/5576922/pimpl-shared-ptr-or-unique-ptr – Rolf Kristensen Nov 21 '11 at 22:32

9 Answers9

38

You shouldn't really use std::auto_ptr for this. The destructor won't be visible at the point you declare the std::auto_ptr, so it might not be called properly. This is assuming that you are forward declaring your pImpl class, and creating the instance inside the constructor in another file.

If you use boost::scoped_ptr (no need for shared_ptr here, you won't be sharing the pimpl with any other objects, and this is enforced by scoped_ptr being noncopyable), you only need the pimpl destructor visible at the point you call the scoped_ptr constructor.

E.g.

// in MyClass.h

class Pimpl;

class MyClass 
{ 
private:
    std::auto_ptr<Pimpl> pimpl;

public: 
    MyClass();
};

// Body of these functions in MyClass.cpp

Here, the compiler will generate the destructor of MyClass. Which must call auto_ptr's destructor. At the point where the auto_ptr destructor is instantiated, Pimpl is an incomplete type. So in to the auto_ptr destructor when it deletes the the Pimpl object, it won't know how to call the Pimpl destructor.

boost::scoped_ptr (and shared_ptr) don't have this problem, because when you call the constructor of a scoped_ptr (or the reset method) it also makes a function-pointer-equivalent that it will use instead of calling delete. The key point here is that it instantiates the deallocation function when Pimpl is not an incomplete type. As a side note, shared_ptr allows you to specify a custom deallocation function, so you can use for it for things like GDI handles or whatever else you may want - but that's overkill for your needs here.

If you really want to use std::auto_ptr, then you need to take extra care by making sure you define your MyClass destructor in MyClass.cpp when Pimpl is fully defined.

// in MyClass.h

class Pimpl;

class MyClass 
{ 
private:
    std::auto_ptr<Pimpl> pimpl;

public: 
    MyClass();
    ~MyClass();
};

and

// in MyClass.cpp

#include "Pimpl.h"

MyClass::MyClass() : pimpl(new Pimpl(blah))
{
}

MyClass::~MyClass() 
{
    // this needs to be here, even when empty
}

The compiler will generate the code destruct all of the MyClass members effectively 'in' the empty destructor. So at the point the auto_ptr destructor is instantiated, Pimpl is no longer incomplete and the compiler now knows how to call the destructor.

Personally, I don't think it's worth the hassle of making sure everything is correct. There's also the risk that somebody will come along later and tidy up the code by removing the seemingly redundant destructor. So it's just safer all round to go with boost::scoped_ptr for this kind of thing.

FactualHarmony
  • 532
  • 1
  • 5
  • 14
Wilka
  • 26,494
  • 13
  • 71
  • 94
  • 1
    But you can't call pimpl(new Pimpl(blah)) when you use scoped_ptr since it's not copyable. So isn't shared_ptr better? – Frank Mar 21 '09 at 23:55
  • 5
    Is there a reason that you declare the Pimpl class outside MyClass? I ask because I've gotten into the habit of using a private struct for the pimpl object. – jamuraa Dec 02 '09 at 16:51
  • 3
    is this answer still correct? In Boost 1.41 at least, it looks like scoped_ptr requires defining the destructor and constructor where the class is complete, and it calls delete directly. – Amnon Dec 22 '10 at 10:06
  • GCC gives off an error about absent destructor on this icorrect usage of auto_ptr for pimpl, so destructor implementation won't be forgotten anyway. IMHO, that reduces problem to nothig and makes auto_ptr preferable over boost solutions. – Basilevs Mar 10 '11 at 07:55
  • 4
    This answer contains misinformation. `boost::scoped_ptr` does *not* use type-erasure to capture a deleter at the time of construction like `shared_ptr` does. Nor does `std::unique_ptr` for that matter. Both of them suffer from the same problems as `std::auto_ptr` with respect to incomplete types. See [scoped_ptr](http://www.boost.org/libs/smart_ptr/scoped_ptr.htm) and [GOTW 100](http://herbsutter.com/gotw/_100/). – Andrew Durward Oct 03 '12 at 12:55
12

I tend to use auto_ptr. Be sure to make your class noncopyable (declare private copy ctor & operator=, or else inherit boost::noncopyable). If you use auto_ptr, one wrinkle is that you need to define a non-inline destructor, even if the body is empty. (This is because if you let the compiler generate the default destructor, impl will be an incomplete type when the call to delete impl_ is generated, invoking undefined behaviour).

There's little to choose between auto_ptr & the boost pointers. I tend not to use boost on stylistic grounds if a standard library alternative will do.

fizzer
  • 12,963
  • 8
  • 37
  • 60
  • That reminds me, if your class does intend to be copyable (e.g., for use in STL containers), then either shared_ptr is required (if you want to share the impl), or else a deep copy of the impl. – Chris Jester-Young Nov 22 '08 at 10:30
  • 3
    Can you please provide more details on why it is undefined behavior if you not have a user-defined destructor (maybe standard section?) – Johannes Schaub - litb Nov 22 '08 at 11:12
  • It's not explicitly stated in the Standard, but is a consequence of the compiler having to instantiate an inline default dtor of the outer class when it sees that no dtor has been declared. As there is an auto_ptr member variable, the default dtor must call auto_ptr dtor. – fizzer Nov 22 '08 at 11:34
  • So ~auto_ptr must get instantiated here and will include a call to `delete impl_;` At this point in the translation unit, `impl_` is incomplete, so the delete is undefined by 5.3.5 (3). Real compilers (e.g MSVC++) will warn about deleting an incomplete type here. – fizzer Nov 22 '08 at 11:37
  • In contrast, if you declare a dtor, the compiler will not generate a default one. Later, in your cpp file you define struct impl at the top, and later on the (possibly empty) body of your dtor. Again, ~auto_ptr gets instantiated, but this time impl is complete and the delete defined. – fizzer Nov 22 '08 at 11:41
  • I think this is an unintended consequence of the Standard. If you search comp.lang.c++.moderated for 'auto_ptr incomplete type', you will find better analysis by people smarter than me. See also the boost smart_ptr pages which IIRC discuss the issue and why some of their pointers don't suffer it. – fizzer Nov 22 '08 at 11:47
  • I just remembered, in this case you're not choosing between standard library versus Boost, you're choosing between pre-TR1 and TR1 standard libraries. :-) – Chris Jester-Young Nov 22 '08 at 20:23
  • Compatibility issues make this choice equally hard. – Basilevs Mar 10 '11 at 07:58
4

The boost alternative to std::auto_ptr is boost::scoped_ptr. The main difference from auto_ptr is that boost::scoped_ptr is noncopyable.

See this page for more details.

kshahar
  • 9,617
  • 9
  • 45
  • 66
  • that's not right. `auto_ptr` is ought to be movable, `scoped_ptr` is not. The doc you linked does also mention this: "The primary reason to use scoped_ptr rather than auto_ptr is to let readers of your code know that you intend "resource acquisition is initialization" to be applied only for the current scope, and have no intent to transfer ownership." – Sebastian Mach Aug 10 '11 at 14:57
4

boost::shared_ptr is specially tailored to work for pimpl idiom. One of the main advantages is that it allows not to define the destructor for the class holding pimpl. Shared ownership policy maybe both advantage and disadvantage. But in later case you can define copy constructor properly.

user49248
  • 291
  • 2
  • 2
  • Even if it is tailored for the pimpl idiom it brings its nature with itself; you share the impl, which means that by default two copies of the object will in fact be the same object. That behavior is surprising, and the last think you want to do is to surprise the users of your class. – daramarak May 05 '10 at 15:37
  • You might want two copies to have the same object. – CashCow Mar 09 '11 at 17:08
1

If you are being really pedantic there is no absolute guarantee that using an auto_ptr member does not require a full definition of the auto_ptr's template parameter at the point at which it is used. Having said that, I've never seen this not work.

One variation is to use a const auto_ptr. This works so long as you can construct your 'pimpl' with a new expression inside the initialiser list and guarantees that the compiler cannot generate default copy constructor and assignment methods. A non-inline destructor for the enclosing class still needs to be provided.

Other things being equal, I would favour an implementation that uses just the standard libraries as it keeps things more portable.

CB Bailey
  • 648,528
  • 94
  • 608
  • 638
  • shared_ptr and scoped_ptr _are_ in the standard libraries...of TR1. :-) – Chris Jester-Young Nov 22 '08 at 20:22
  • True, but TR1 is not part of either of the 1998 or 2003 standards and there are many C++ environments that implement these standards reasonably completely but don't provide TR1 libraries. The fewer dependencies you have; the more portable your code. – CB Bailey Nov 23 '08 at 00:11
  • shared_ptr is in TR1. scoped_ptr is not. – Rimo Jun 04 '10 at 07:31
1

If you want a copyable class, use scoped_ptr, which forbids copying, thus making your class hard to use wrong by default (compared to using shared_ptr, the compiler won't emit copy facilities on its own; and in case of shared_ptr, if you don't know what you do [which is often enough the case even for wizards], there would be strange behaviour when suddenly a copy of something also modifies that something), and then out-define a copy-constructor and copy-assignment:

class CopyableFoo {
public:
    ...
    CopyableFoo (const CopyableFoo&);
    CopyableFoo& operator= (const CopyableFoo&);
private:
    scoped_ptr<Impl> impl_;
};

...
CopyableFoo (const CopyableFoo& rhs)
    : impl_(new Impl (*rhs.impl_))
{}
Sebastian Mach
  • 36,158
  • 4
  • 87
  • 126
  • you should derive from boost::noncopyable to make your class non-copyable – fmuecke Oct 11 '11 at 14:36
  • @fmuecke: But my `CopyableFoo` is intended to be copyable. Using `scoped_ptr<>` ensures that you don't forget to write copy-assignment and -constructor. Because `scoped_ptr<>` in itself is not copyable, the compiler won't try to emit them automatically. If you use `shared_ptr<>`, the compiler will emit copy-operations that potentially don't do what you intend. Remember, my class is intended to be copyable, not non-copyable. Of course you are right that `boost::noncopyable` should be used to make so. – Sebastian Mach Oct 11 '11 at 15:29
0

shared_ptr is much preferable to auto_ptr for pImpl because your outer class could suddenly end up losing its pointer when you copy it.

With shared_ptr you can use a forwardly-declared type so that works. auto_ptr does not allow a forwardly-declared type. Nor does scoped_ptr and if your outer class is going to be non-copyable anyway and only has one pointer, it may as well be a regular one.

There is a lot to be said for using an intrusive reference count in the pImpl and get the outer class to call its copy and assign semantics in its implementation. Assuming this is a true vendor (supplies the class) model it is better the vendor does not force the user to be using shared_ptr, or to be using the same version of shared_ptr (boost or std).

CashCow
  • 29,087
  • 4
  • 53
  • 86
0

I have been really happy about impl_ptr by Vladimir Batov [modified]. It makes it really easy to create a pImpl without needing to make explicit copy-constructor and assignment operator.

I have modified the original code, so it now resembles a shared_ptr, so it can be used in epilog code, and remains speedy.

Rolf Kristensen
  • 12,232
  • 1
  • 41
  • 52
-8

Don't try so hard to shoot yourself in the foot, in C++ you have plenty of opportunities :) There is no real need to use either auto pointers, since you perfectly know when your object should go in and out of life (in your constructor(s) and destructor).

Keep it simple.

Marco M.
  • 252
  • 1
  • 4
  • 7
    _Strongly_ disagree. This approach does not defend against exceptions caused in the ctor itself, that may happen after your impl object is created. When this happens, the dtor is not called, and your impl object is leaked. – Chris Jester-Young Nov 22 '08 at 10:24
  • If you use the pimpl idiom it isn't unlikly that the impl pointer is the only pointer in the class. Using unmanaged pointers in that case is problem free if the destructor deletes the pointer. Resource leaks in the constructor is not possible because with only one member, only new can throw and will not allocate memory. – daramarak May 05 '10 at 15:52
  • The issue though is that you now have a resource that you have to delete in your destructor so you have to also implement copy and assign semantics, and probably you will want to end up doing reference counting anyway, possibly intrusively. – CashCow Mar 09 '11 at 16:44