Re: [boost] [smart_ptr] enable_shared_from_this enhancement (patch)

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

Frank Mori Hess:

> -----BEGIN PGP SIGNED MESSAGE-----
> 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.