[libc++] ~unique_ptr() should not set stored pointer to null

[dcheng]

Summary

This better matches the behavior of MSVC and libstdc++: neither
standard library sets the stored pointer to null when destroying the
unique_ptr.

[dblaikie]

I'd still be curious to see a stronger justification from the standard
about this.

I know you mentioned/quoted the definition of ~unique_ptr in the standard -
but I'm not sure that is meant to imply observable behavior during
destruction (though I could be wrong). Is there anything in the standard
about accessing objects while their dtors are executing?

[dcheng]

In D13080#251324, @dblaikie wrote:

I'd still be curious to see a stronger justification from the standard
about this.

I know you mentioned/quoted the definition of ~unique_ptr in the standard -
but I'm not sure that is meant to imply observable behavior during
destruction (though I could be wrong). Is there anything in the standard
about accessing objects while their dtors are executing?

From 3.8.5 [basic.life]:
Before the lifetime of an object has started but after the storage which the object will occupy has been
allocated or, after the lifetime of an object has ended and before the storage which the object occupied is
reused or released, any pointer that refers to the storage location where the object will be or was located
may be used but only in limited ways. For an object under construction or destruction, see 12.7.

From 12.7.1 [class.dtor]:
For an object with a non-trivial destructor, referring to any non-static member or base class of the object after the destructor finishes execution results in undefined behavior.

So I believe the standard permits object access while the dtor is still executing.

[thakis]

mclow should review this, not me.

[mclow.lists]

Please DO NOT commit this patch.

See http://cplusplus.github.io/LWG/lwg-active.html#2224 for a discussion of this code, and the (proposed) resolution is that this is undefined behavior.

[dcheng]

OK, thanks for highlighting the proposed resolution.

include/memory

template <class _Up>
                                       is_convertible<_Up*, _Tp*>::value &&
                                       is_same<_Dp, default_delete<_Tp> >::value,
                                       unique_ptr&
                                   >::type
         operator=(auto_ptr<_Up> __p)
             {reset(__p.release()); return *this;}
 
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
-    _LIBCPP_INLINE_VISIBILITY ~unique_ptr() {reset();}
+    _LIBCPP_INLINE_VISIBILITY ~unique_ptr()
+    {
+        if (__ptr_.first() != pointer())
+            __ptr_.second()(__ptr_.first());
+    }
 
     _LIBCPP_INLINE_VISIBILITY unique_ptr& operator=(nullptr_t) _NOEXCEPT
     {
         reset();
         return *this;
     }
 
     _LIBCPP_INLINE_VISIBILITY typename add_lvalue_reference<_Tp>::type operator*() const
#else  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
     _LIBCPP_INLINE_VISIBILITY unique_ptr& operator=(__rv<unique_ptr> __u)
     {
         reset(__u->release());
         __ptr_.second() = _VSTD::forward<deleter_type>(__u->get_deleter());
         return *this;
     }
 
 #endif  // _LIBCPP_HAS_NO_RVALUE_REFERENCES
-    _LIBCPP_INLINE_VISIBILITY ~unique_ptr() {reset();}
+    _LIBCPP_INLINE_VISIBILITY ~unique_ptr()
+    {
+        if (__ptr_.first() != pointer())
+            __ptr_.second()(__ptr_.first());
+    }
 
     _LIBCPP_INLINE_VISIBILITY unique_ptr& operator=(nullptr_t) _NOEXCEPT
     {
         reset();
         return *this;
     }
 
     _LIBCPP_INLINE_VISIBILITY typename add_lvalue_reference<_Tp>::type operator[](size_t __i) const

test/std/utilities/memory/unique.ptr/unique.ptr.single/unique.ptr.single.dtor/stored_pointer.pass.cpp

+//===----------------------------------------------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+
+// <memory>
+
+// unique_ptr
+
+// ~unique_ptr() should not set the stored pointer to nullptr.
+
+#include <memory>
+#include <cassert>
+
+struct A;
+
+struct B
+{
+    std::unique_ptr<A> a;
+};
+
+struct A
+{
+    B* b;
+   ~A() {assert(b->a);}
+};
+
+int main()
+{
+    B b;
+    b.a.reset(new A);
+    b.a->b = &b;
+}