806. unique_ptr::reset effects incorrect, too permissive

Section: 20.3.1.3.6 [unique.ptr.single.modifiers] Status: CD1 Submitter: Peter Dimov Opened: 2008-03-13 Last modified: 2016-01-28

Priority: Not Prioritized

View all other issues in [unique.ptr.single.modifiers].

View all issues with CD1 status.

Discussion:

void unique_ptr::reset(T* p = 0) is currently specified as:

Effects: If p == get() there are no effects. Otherwise get_deleter()(get()).

There are two problems with this. One, if get() == 0 and p != 0, the deleter is called with a NULL pointer, and this is probably not what's intended (the destructor avoids calling the deleter with 0.)

Two, the special check for get() == p is generally not needed and such a situation usually indicates an error in the client code, which is being masked. As a data point, boost::shared_ptr was changed to assert on such self-resets in 2001 and there were no complaints.

One might think that self-resets are necessary for operator= to work; it's specified to perform

reset( u.release() );

and the self-assignment

p = move(p);

might appear to result in a self-reset. But it doesn't; the release() is performed first, zeroing the stored pointer. In other words, p.reset( q.release() ) works even when p and q are the same unique_ptr, and there is no need to special-case p.reset( q.get() ) to work in a similar scenario, as it definitely doesn't when p and q are separate.

Proposed resolution:

Change 20.3.1.3.6 [unique.ptr.single.modifiers]:

void reset(T* p = 0);

-4- Effects: If p == get() == 0 there are no effects. Otherwise get_deleter()(get()).

Change 20.3.1.4.5 [unique.ptr.runtime.modifiers]:

void reset(T* p = 0);

...

-2- Effects: If p == get() == 0 there are no effects. Otherwise get_deleter()(get()).