Pimpl using auto_ptr

[Keith Willis]

I've been beating my head against this for a while, so I'm now looking
for some hints and tips from the cognocenti...

I've been playing around with auto_ptr and as an exercise I've tried
to change a class of my own which uses the Pimpl idiom.

The original version is something like this:

//x.h
class MyClass
{
<snip stuff>
private:
class MyPrivate; // forward declaration
MyPrivate* p_;
};

//x.cc
// Constructor
MyClass::MyClass : p_(new MyPrivate()) {}

// Copy Constructor
MyClass::MyClass(const MyClass& that) : p_(new MyClass(*that.p_)) {}

// Assignment operator
MyClass&
MyClass:perator=(const MyClass& rhs)
{
MyPrivate* t_ = new MyPrivate(*rhs.p_);
delete p_;
p_ = t_;
return *this;
}

The problem is I can't see for the life of me how to write the copy
constructor and assignment operator if I change the declaration of p_
to this:

//x.h
class MyClass
{
<snip>
private:
class MyPrivate; // forward declaration
std::auto_ptr<MyPrivate> p_;
};

Is there a 'canonical form' for expressing these?
TIA!

 

[AnonMail2005]

I've been beating my head against this for a while, so I'm now looking
for some hints and tips from the cognocenti...

I've been playing around with auto_ptr and as an exercise I've tried
to change a class of my own which uses the Pimpl idiom.

The original version is something like this:

//x.h
class MyClass
{
<snip stuff>
private:
class MyPrivate; // forward declaration
MyPrivate* p_;

};

//x.cc
// Constructor
MyClass::MyClass : p_(new MyPrivate()) {}

// Copy Constructor
MyClass::MyClass(const MyClass& that) : p_(new MyClass(*that.p_)) {}

// Assignment operator
MyClass&
MyClass:perator=(const MyClass& rhs)
{
MyPrivate* t_ = new MyPrivate(*rhs.p_);
delete p_;
p_ = t_;
return *this;

}

The problem is I can't see for the life of me how to write the copy
constructor and assignment operator if I change the declaration of p_
to this:

//x.h
class MyClass
{
<snip>
private:
class MyPrivate; // forward declaration
std::auto_ptr<MyPrivate> p_;

};

Is there a 'canonical form' for expressing these?
TIA!

The existing code should almost work (although I haven't typed it in
and checked) except
you don't need to explicitly delete _p in your copy construtor. If
I'm not mistaken,
assigning an auto_ptr should delete what it previously points to.
Because of this
you should make an explicit check for self assignment and do nothing
if, in fact,
it is self assignment.

HTH

 

[anon]

I've been beating my head against this for a while, so I'm now looking
for some hints and tips from the cognocenti...

You haven't bang your head enough, cause your example suck. Anyway, take
a look at this:



#include <memory>
#include <iostream>

class MyClass
{
public:
MyClass(int a);
~MyClass();
MyClass(const MyClass& that);
MyClass& operator=(const MyClass& rhs);

void a() const;

private:
class MyPrivate; // forward declaration
std::auto_ptr< MyPrivate > p_;
};

class MyClass::MyPrivate
{
public:
MyPrivate(){}
~MyPrivate(){}

int a;
};

//x.cc
// Constructor
MyClass::MyClass(int a) : p_(new MyPrivate)
{
p_->a=a;
}

MyClass::~MyClass()
{
}

// Copy Constructor
MyClass::MyClass(const MyClass& that)
{
this->p_.reset( new MyPrivate );
this->p_->a = that.p_->a;
}

// Assignment operator
MyClass& MyClass:perator=(const MyClass& rhs)
{
this->p_.reset( new MyPrivate );
this->p_->a = rhs.p_->a;
return *this;
}

void MyClass::a() const
{
std::cout<<p_->a<<std::endl;
}

int main()
{
MyClass a(2);
MyClass b(3);
a.a();
b.a();
a = b;
a.a();
b.a();
}

 

[Alf P. Steinbach]

* Keith Willis:

I've been beating my head against this for a while, so I'm now looking
for some hints and tips from the cognocenti...

I've been playing around with auto_ptr and as an exercise I've tried
to change a class of my own which uses the Pimpl idiom.

std::auto_ptr requires a complete class. For the code to be valid you
can only use std::auto_ptr in PIMPL if all the relevant compilers
provide additional guarantees for std::auto_ptr. However, in practice
it might work.

Using std::auto_ptr for PIMPL is the most infamous error in the
(original) GOTW articles.

Look it up.


Cheers, & hth.,

- Alf

 

[red floyd]

* Keith Willis:

std::auto_ptr requires a complete class. For the code to be valid you
can only use std::auto_ptr in PIMPL if all the relevant compilers
provide additional guarantees for std::auto_ptr. However, in practice
it might work.

Using std::auto_ptr for PIMPL is the most infamous error in the
(original) GOTW articles.

Look it up.

Yep. Sutter uses it in GOTW 59, 61, and 62.

I never realized that you couldn't use an auto_ptr on an incomplete
class. It doesn't really seem to need knowledge of the internals.

 

[Alf P. Steinbach]

* red floyd:

Yep. Sutter uses it in GOTW 59, 61, and 62.

I never realized that you couldn't use an auto_ptr on an incomplete
class. It doesn't really seem to need knowledge of the internals.

Mentioned in e.g. §17.4.3.6/2 last hyphen point, library wide requirements.

The main practical constraint is however §5.3.5/5, that at the /point of
destruction/ you need a complete class or a trivial destructor and
deallocation function.

With that in mind someone once posted a workaround where the public
class has a declared destructor, implemented in the implementation file
where the full private implementation class is known, and that's the
in-practice technique that seems to work with most compilers (if one
absolutely must use std::auto_ptr instead of e.g. boost::shared_ptr) in
spite of the standard's formal Undefined Behavior.


Cheers,

- Alf

 

[Alf P. Steinbach]

* Alf P. Steinbach:

* red floyd:

Mentioned in e.g. §17.4.3.6/2 last hyphen point, library wide requirements.

The main practical constraint is however §5.3.5/5, that at the /point of
destruction/ you need a complete class or a trivial destructor and
deallocation function.

With that in mind someone once posted a workaround where the public
class has a declared destructor, implemented in the implementation file
where the full private implementation class is known, and that's the
in-practice technique that seems to work with most compilers (if one
absolutely must use std::auto_ptr instead of e.g. boost::shared_ptr) in
spite of the standard's formal Undefined Behavior.

Forgot to mention: for that workaround the public class also needs
non-inline constructor, so that the std::auto_ptr is constructed by the
code in the implementation file.

 

[Barry]

Forgot to mention: for that workaround the public class also needs
non-inline constructor, so that the std::auto_ptr is constructed by the
code in the implementation file.

This is not necessary

struct X;
X* px = 0; // if this is not illformed -- condition1

<code>
#include <memory>

struct Wrapper {
Wrapper() : sp_(/*0*/) {}
~Wrapper();
private:
struct Impl;
std::auto_ptr<Impl> sp_;
};

struct Wrapper::Impl {};

Wrapper::~Wrapper() {}

int main()
{
Wrapper w;
}
</code>

if condition1 is true, then the code above is well-formed
And I think condition1 is true.

 

[Joe Greer]

This is not necessary

struct X;
X* px = 0; // if this is not illformed -- condition1

<code>
#include <memory>

struct Wrapper {
Wrapper() : sp_(/*0*/) {}
~Wrapper();
private:
struct Impl;
std::auto_ptr<Impl> sp_;
};

struct Wrapper::Impl {};

Wrapper::~Wrapper() {}

int main()
{
Wrapper w;
}
</code>

if condition1 is true, then the code above is well-formed
And I think condition1 is true.

It still doesn't necessarily know how to destroy an Impl, which I
believe is the problem. That is, if Impl has any sort of inheritance
heirarchy, it is likely that the inherited destructors won't be invoked
because the compiler doesn't know about them. There are also problems
if Impl is an abstract interface. At least, that is my experience.

joe

 

[Alf P. Steinbach]

* Barry:

This is not necessary

It is.

struct X;
X* px = 0; // if this is not illformed -- condition1

This is OK.

<code>
#include <memory>

struct Wrapper {
Wrapper() : sp_(/*0*/) {}
~Wrapper();
private:
struct Impl;
std::auto_ptr<Impl> sp_;
};

struct Wrapper::Impl {};

Wrapper::~Wrapper() {}

int main()
{
Wrapper w;
}
</code>

This is not PIMPL. The point of PIMPL is to have the definition of
Wrapper::Impl in a separately compiled file.

if condition1 is true, then the code above is well-formed

See earlier postings for the relevant paragraphs of the standard. The
code above seems to be OK (although it's tricky, and I might be wrong
about that OK). But it's not PIMPL.

Cheers,

- Alf