auto_ptr within Pimpl Class
Author
I read in another forum the other day that you should not use the std::auto_ptr class to hold the member data pointer within a class that follows the Pimpl Idiom class. The person said that auto_ptr required a complete class. He said that Herb Sutter's implementation was wrong. My compiler (gcc under cygwin) allows it as long as I put my class constructor within the source file.
Can you use the auto_ptr class if you overload the copy constructor and assignment operator? Was this guy wrong?
Thanks,
Scott
Quote:My compiler (gcc under cygwin) allows it as long as I put my class constructor within the source file.Which makes sense, since the class is completely declared there.
Apart from the question if it is accepted or not, I wonder why you would want to use an auto_ptr to implement the Pimpl-idiom. Your interface class will and should be the only one referencing this pointer, and would always keep explicit ownership.
Author
Quote:Original post by DaBonoExactly. However, using the auto_ptr class as Herb Sutter suggested removes any possibility of forgetting to free the memory in the destructor.
... Your interface class will and should be the only one referencing this pointer, and would always keep explicit ownership.
Quote:Original post by DaBono
Apart from the question if it is accepted or not, I wonder why you would want to use an auto_ptr to implement the Pimpl-idiom. Your interface class will and should be the only one referencing this pointer, and would always keep explicit ownership.
This saves the pain of writing a destructor (since the default destructor would destruct the auto_ptr, and thus the implementation pointer.
Author
Quote:Original post by ToohrVyk
This saves the pain of writing a destructor (since the default destructor would destruct the auto_ptr, and thus the implementation pointer.
So, I assume you're confirming that it is OK by the standard to implement the Pimpl idiom using an auto_ptr?
Quote:Original post by scottdewald
Was this guy wrong?
Yes and no. std::auto_ptr requires that the destructor of the class be available. That should the case in the imeplentation file for the class your are wrapping using the pimpl idiom.
I would recommend using std::auto_ptr in your pimpl class. It does the right thing at the right time and documents pointer ownership. It forces you to write appropriate copy constructors and assignment operators. It helps you avoid common errors.
--smw
I think the issue with the pimpl idiom and auto_ptr is about the destructor of the pimpl class.
Eg if the pimpl class was declared with a virtual destructor / custom operator delete etc.
For example, the following code gives this error under gcc:
[edit:] Yeah, what Bregma said. Curse leaving the computer and finishing posts wihtout checking for an update [smile]
Eg if the pimpl class was declared with a virtual destructor / custom operator delete etc.
For example, the following code gives this error under gcc:
class PimplUser{public: PimplUser();private: class Pimpl; std::auto_ptr<Pimpl> pimpl;};void foo(){ PimplUser pimplUser;}// C:\Dev-Cpp\include\c++\3.4.2\memory In destructor `std::auto_ptr<_Tp>::~auto_ptr() [with _Tp = PimplUser::Pimpl]': // 473 D:\Programming\c++\rip_asteroids\src\common\main.cpp instantiated from here // 258 C:\Dev-Cpp\include\c++\3.4.2\memory [Warning] possible problem detected in invocation of delete operator: // 258 C:\Dev-Cpp\include\c++\3.4.2\memory [Warning] invalid use of undefined type `struct PimplUser::Pimpl' // 467 D:\Programming\c++\rip_asteroids\src\common\main.cpp [Warning] forward declaration of `struct PimplUser::Pimpl' // 467 D:\Programming\c++\rip_asteroids\src\common\main.cpp neither the destructor nor the class-specific operator delete will be called, even if they are declared when the class is defined. [edit:] Yeah, what Bregma said. Curse leaving the computer and finishing posts wihtout checking for an update [smile]
Quote:Original post by Bregma
I would recommend using std::auto_ptr in your pimpl class. It does the right thing at the right time and documents pointer ownership. It forces you to write appropriate copy constructors and assignment operators.
?!
std::auto_ptr provides a copy constructor and assignment operator, so it doesn't force you to write any (the pimpl's auto-generated cctor and assignop would just delegate to the auto_ptr, in effect). Meanwhile, the auto_ptr's semantics for those arguments are, generally speaking, WRONG for a pimpl; the original object will get nuked.
... Oh, wait. It *will* force writing those operations, won't it? Because of const-correctness issues (auto_ptr's assignop and cctor accept non-const references, so they won't be liked by the default-generator)?
But then, that just means more work - you could instead implement a auto-ptr-alike that implements the proper semantics (i.e., invoking the pointed-at-thing's cctor and assignop), and never have to put that code into any pimpl you create. Not to mention, *nothing* you put into the pimpl can force you to write a proper cctor or assignop for *the worker class*. :s Unless I'm just spectacularly unimaginative?
Author
I ran a few tests with a class that had an auto_ptr as a member variable, and my compiler (gcc under cygwin) required me to overload operator= to deal with the const correctness issue, but it did *not* require me to overload the copy constructor. This surprised me since the auto_ptr class doesn't have the appropriate const copy constructor for this. Am I missing something or is my compiler wrong?
Quote:Original post by ToohrVykQuote:Original post by DaBono
Apart from the question if it is accepted or not, I wonder why you would want to use an auto_ptr to implement the Pimpl-idiom. Your interface class will and should be the only one referencing this pointer, and would always keep explicit ownership.
This saves the pain of writing a destructor (since the default destructor would destruct the auto_ptr, and thus the implementation pointer.
Yes and no. If you let the compiler write the destructor for you, you have to make sure that the encapsulated type is complete - otherwise your encapsulated object will not be destroyed (in most implementation).
The following code leaks:
// A.hclass A{ std::vector<int> v;public: A(std::size_t s) : v(s) { }};// B.hclass A; // forward declarationclass B{ std::auto_ptr<A> a;public: B();};// B.cpp#include "B.h"#include "A.h"B::B() : a(new A(100)) { }The reason is that the implicit destructor of B will call the destructor of auto_ptr<A>; since A is not a complete type, the compiler will either issue a warning or will silently give A an implicit trivial destructor (and as a consequence, this destructor will not release the allocated vector).
To correct the problem, simply add a destructor to class B, and put the destructor code in B.cpp (even if the destructor does nothing).
Regards,
This topic is closed to new replies.
Advertisement
Popular Topics
Advertisement
