Boost logo

Boost :

From: Peter Dimov (pdimov_at_[hidden])
Date: 2008-03-18 15:13:53

Frank Mori Hess:

> Hash: SHA1
> Here's a proof-of-concept patch to svn trunk for
> enable_shared_from_this.hpp
> and shared_ptr.hpp. After applying the patch, you should be able to use
> shared_from_this() successfully from a constructor. It's only a
> proof-of-concept because it includes some quick-and-dirty nastiness (it
> makes
> the shared_count in shared_ptr public!).

A few comments.

I liked your original formulation with two shared_ptr instances more. It's
better to avoid direct use of shared_count. The aliasing constructor may
prove helpful.

I don't see why pn needs to be made public.

The sp_enable_shared_from_this logic needs to go into a member function
(_init? _adopt?) of enable_shared_from_this.

I'm unclear on the purpose of pn.swap(...) in sp_enable_shared_from_this. It
seems to me that this would overwrite the user's deleter, breaking
get_deleter. I don't see why it's needed.

sp_enable_shared_from_this may need to be changed to take a shared_ptr
instead of a shared_count.

I'm disturbed by the fact that we now provide an easy (beginner-accessible)
way to create dangling pointers. Consistent shared_ptr use is supposed to
make this hard.

I'd be interested to hear your ideas about the new precondition of
shared_from_this and its new thread safety.

Overall, the direction seems promising, and I thank you for the work so far.

A final note: changes to enable_shared_from_this unfortunately expose the
fact that the shared_ptr/weak_ptr test suite is incomplete; it doesn't
perform the full array of tests with types that derive from
enable_shared_from_this. I suspect that we need at least partial coverage
here to catch regressions.

Boost list run by bdawes at, gregod at, cpdaniel at, john at