Object ownership and memory management

[jakub.pieczonka]

I have three classes in my example. Foo, FooRepository and FooImpl.
Foo is a lightweight wrapper around FooImpl. The idea is that Foo
can be used to pass elements by value from FooRepository to the
client code.

One option would be to use shared_ptr, but this seems like overkill
for my particular situation. The client will not be storing or
copying the Foo objects, so I have chosen to implement Foo in the
following way:

class Foo {
public:
Foo(FooImpl* impl) : pimpl_(impl) {}
//A few functions follow that simply forward requests
//...
private:
auto_ptr<FooImpl> pimpl_;
};

FooRepository returns Foo objects by value. The signature is:

class FooRepository {
public:
Foo getFoo();
};

And the client code does the following:

Foo foo = aFooRepository.getFoo();

The idea is that the foo object created here has ownership of the
FooImpl and when it goes out of scope, the FooImpl is automatically
deleted. Resource leaks are also handled in case the return value
is ignored.

This works great on Forte compilers, but fails on GCC with the
error message:

Foo.cpp: In function `int main(int, char**)':
Foo.cpp:25: error: no matching function for call to `Foo::
Foo(Foo)'
Foo.h:16: error: candidates are:
Foo::Foo(Foo&)
Foo.h:18: error:
Foo::Foo(FooImpl*)

It appears that the presence of the auto_ptr in the Foo class
prevents the compiler from generating the copy constructor taking a
const-ref parameter, and the client code matching the function call
properly.

The error message makes no sense to me because the copy constructor
cannot be written to take a parameter by value.

What is the best way to handle this situation? Am I missing
something obvious? I realize that the tradeoff I made here was to
simplify memory management but gave up the possibility of storing
my Foo objects inside containers. Is there a standard pattern for
this type of object management?

 

[Howard Hinnant]

class Foo {
public:
Foo(FooImpl* impl) : pimpl_(impl) {}
//A few functions follow that simply forward requests
//...
private:
auto_ptr<FooImpl> pimpl_;
};

FooRepository returns Foo objects by value. The signature is:

class FooRepository {
public:
Foo getFoo();
};

And the client code does the following:

Foo foo = aFooRepository.getFoo();

The idea is that the foo object created here has ownership of the
FooImpl and when it goes out of scope, the FooImpl is automatically
deleted. Resource leaks are also handled in case the return value
is ignored.

This works great on Forte compilers, but fails on GCC with the
error message:

Foo.cpp: In function `int main(int, char**)':
Foo.cpp:25: error: no matching function for call to `Foo::
Foo(Foo)'
Foo.h:16: error: candidates are:
Foo::Foo(Foo&)
Foo.h:18: error:
Foo::Foo(FooImpl*)

It appears that the presence of the auto_ptr in the Foo class
prevents the compiler from generating the copy constructor taking a
const-ref parameter, and the client code matching the function call
properly.

The compiler is automatically generating this copy constructor:

Foo(Foo&);

The error message makes no sense to me because the copy constructor
cannot be written to take a parameter by value.

The compiler means that given the expression Foo(Foo(...)), there's no
matching constructor since it can't bind the rvalue Foo() to the
parameter in Foo(Foo&).

What is the best way to handle this situation? Am I missing
something obvious? I realize that the tradeoff I made here was to
simplify memory management but gave up the possibility of storing
my Foo objects inside containers. Is there a standard pattern for
this type of object management?

There will be a standard pattern in C++0X. ;-)

struct FooImpl {};

class FooRepository;

class Foo {
private:
friend class FooRepository;
explicit Foo(FooImpl* impl) : pimpl_(impl) {}
public:
//A few functions follow that simply forward requests
//...
private:
std::unique_ptr<FooImpl> pimpl_;
};

class FooRepository {
public:
static Foo getFoo() {return Foo(new FooImpl);}
};

int main()
{
Foo foo = FooRepository::getFoo();
}

But this won't work for you today. One thing you can try is this:

class Foo {
private:
friend class FooRepository;
explicit Foo(FooImpl* impl) : pimpl_(impl) {}
public:
Foo(const Foo& f) :
pimpl_(const_cast<std::auto_ptr<FooImpl>&>(f.pimpl_)) {}
//A few functions follow that simply forward requests
//...
private:
std::auto_ptr<FooImpl> pimpl_;
};

It is a little dangerous as you must always remember that every time you
copy from a Foo, the source is silently changed:

Foo x;
Foo y = x; // x is now non-owning

In your use case, if the client truly does not store copies of Foo
(other than from getFoo), then no harm is done since you only copy from
rvalues. However if your clients decide to start copying Foo's, things
can silently go bad. shared_ptr would be a safer but more expensive
alternative today. The unique_ptr example first shown will be cheap as
auto_ptr and safe tomorrow.

-Howard

 

[Gabriel]

I have three classes in my example. Foo, FooRepository and FooImpl.
Foo is a lightweight wrapper around FooImpl. The idea is that Foo
can be used to pass elements by value from FooRepository to the
client code.

One option would be to use shared_ptr, but this seems like overkill
for my particular situation. The client will not be storing or
copying the Foo objects, so I have chosen to implement Foo in the
following way:

[snip]

I don't think it's overkill. Use a good smart-pointer implementation
(e.g. boost). It saves you time and stress. Why reinvent the wheel? And
even if you can't save time by this (time you save by not writing code
vs. time you need to introduce a lib), you still get more familar with a
smart-pointer lib, which is very important and a good trade-off.

Gabriel

 

[jakub.pieczonka]

Thanks for the prompt responses. I believe that Gabriel is correct in
retrospect. It's better to go with shared_ptr than to risk the
possibility of things going silently bad with the const_cast solution.