Howard Hinnant wrote:
On Apr 14, 2007, at 1:28 PM, Peter Dimov wrote:
Joe Gottman wrote:
I see that rvalue reference support has just been added to the shared_ptr template, and I think that there's a subtle bug in the implementation. It is implemented as follows:
shared_ptr( shared_ptr && r ): px( r.px ), pn() // never throws { pn.swap( r.pn ); }
The problem is that after the move constructor has been called the moved -from shared_ptr, r, is in a dangerously inconsistent state. Its has an empty shared count, but it contains a non-null pointer.
[...]
In this particular case, I think it might be worth the extra assignment to null r.px just to make the class invariant simpler.
I don't mind clearing r.px in the Boost implementation of the move constructors, but I'm not sure whether the specification ought to mandate it. The current implementation is a reformulation of shared_ptr( shared_ptr && r ): px( move(r.px) ), pn( move(r.pn) ) { } I'm not sure that we want to prohibit it. If we decide to clear r.px, there's also the matter of the move assignments. One obvious implementation of shared_ptr& operator=( shared_ptr && r ); is to just call swap( r ). I'm not positive that we want to prohibit that, either. We can enforce a postcondition of "r is empty" for the assignments too by using shared_ptr( r ).swap( *this ) as usual, but this strikes me as overspecification. FWIW, shared_ptr by itself does not have an invariant that ties the pointer (px) and the ownership group (pn) together, although it's pretty common for the surrounding code to enforce or assume such an invariant. So I could go either way on that one.