Extend scoped_ptr to be closer to unique_ptr. Support custom deleters, and deleting arrays.

base/memory/scoped_ptr.h

Index: base/memory/scoped_ptr.h
diff --git a/base/memory/scoped_ptr.h b/base/memory/scoped_ptr.h
index 3547b7a15397f9c58f3c5193401d6d9defae333d..2d5d026caf99b8c4021be6c18bd804844006cbbf 100644
--- a/base/memory/scoped_ptr.h
+++ b/base/memory/scoped_ptr.h
@@ -95,6 +95,8 @@
 #include <stddef.h>
 #include <stdlib.h>
 
+#include <algorithm>  // For std::swap().
+
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/move.h"
@@ -107,6 +109,72 @@ class RefCountedBase;
 class RefCountedThreadSafeBase;
 }  // namespace subtle
 
+// Function object which deletes its parameter, which must be a pointer.
+// If C is an array type, invokes 'delete[]' on the parameter; otherwise,
+// invokes 'delete'. The default deleter for scoped_ptr<T>.
+template <class T>
+struct DefaultDeleter {
+  DefaultDeleter() {}
+  template <typename U> DefaultDeleter(const DefaultDeleter<U>& other) {
+    // Spec for std::default_deleter only provies this constructor of
+    // U* is implicitly convertible to T* and U is not an array type.

[Ryan Sleevi, 2012/12/15 02:24:12]

nit: typo (provies -> provides)
nit: typo (of -> if)
nit: default_deleter -> default_delete
nit: instead of saying "spec for std::default_deleter"

"IMPLEMENTATION NOTE: C++11 20.7.1.1.2p2 only provides this constructor if ..."

[awong, 2012/12/15 02:32:27]

Done.

+    //
+    // Correct implementation should use SFINAE to disable this
+    // constructor. However, since there are no other 1-argument constructors

[Ryan Sleevi, 2012/12/15 02:24:12]

grammar nit: "constructors" -> "constructors,"

[awong, 2012/12/15 02:32:27]

Done.

+    // using a COMPILE_ASSERT() based on is_convertible<> and requiring
+    // complete types is simpler and will cause compile failures for equivalent
+    // misuses.
+    //
+    // Note, the is_convertible<U*, T*> check also ensures that U is not an
+    // array. T is guaranteed to be a non-array so any U* where U is an array

[Ryan Sleevi, 2012/12/15 02:24:12]

grammar nit: "non-array so" -> "non-array, so"

[awong, 2012/12/15 02:32:27]

Done.

+    // cannot convert to T*.
+    enum { T_must_be_complete = sizeof(T) };
+    enum { U_must_be_complete = sizeof(U) };
+    COMPILE_ASSERT((base::is_convertible<U*, T*>::value),
+                   U_ptr_must_implicitly_convert_to_T_ptr);
+  }
+  inline void operator()(T* ptr) const {
+    enum { type_must_be_complete = sizeof(T) };
+    delete ptr;
+  }
+};
+
+// Specialization of DefaultDeleter for array types.
+template <class T>
+struct DefaultDeleter<T[]> {
+  inline void operator()(T* ptr) const {
+    enum { type_must_be_complete = sizeof(T) };
+    delete[] ptr;
+  }
+
+ private:
+  // Disable this operator for any U != T because it is undefined to execute
+  // an array delete when the static type of the array mismatches the dynamic
+  // type.
+  //
+  // References:
+  //   C++98 [expr.delete]p3
+  //   http://cplusplus.github.com/LWG/lwg-defects.html#938
+  template <typename U> void operator()(U* array) const;
+};
+
+template <class T, int n>
+struct DefaultDeleter<T[n]> {
+  // Never allow someone to delcare something like scoped_ptr<int[10]>.

[Ryan Sleevi, 2012/12/15 02:24:12]

typo: delcare -> declare

[awong, 2012/12/15 02:32:27]

Done.

+  COMPILE_ASSERT(sizeof(T) == -1, do_not_use_array_with_size_as_type);
+};
+
+// Function object which invokes 'free' on its parameter, which must be
+// a pointer. Can be used to store malloc-allocated pointers in scoped_ptr:
+//
+// scoped_ptr<int, base::FreeDeleter> foo_ptr(
+//     static_cast<int*>(malloc(sizeof(int))));
+struct FreeDeleter {
+  inline void operator()(void* ptr) const {
+    free(ptr);
+  }
+};
+
 namespace internal {
 
 template <typename T> struct IsNotRefCounted {
@@ -117,7 +185,104 @@ template <typename T> struct IsNotRefCounted {
   };
 };
 
+// Minimal implementation of the core logic of scoped_ptr, suitable for
+// reuse in both scoped_ptr and its specializations.
+template <class T, class D>
+class scoped_ptr_impl {
+ public:
+  explicit scoped_ptr_impl(T* p) : data_(p) { }
+
+  // Initializer for deleters that have data parameters.
+  scoped_ptr_impl(T* p, const D& d) : data_(p, d) {}
+
+  // Templated constructor that destructively takes the value from another
+  // scoped_ptr_impl.
+  template <typename U, typename V>
+  scoped_ptr_impl(scoped_ptr_impl<U, V>* other)
+      : data_(other->release(), other->get_deleter()) {
+    // We do not support move-only deleters.  We could modify our move
+    // emulation to have base::subtle::move() and base::subtle::forward()
+    // functions that are imperfect emulations of their C++11 equivalents,
+    // but until there's a requirement, just assume deleters are copyable.
+  }
+
+  template <typename U, typename V>
+  void TakeState(scoped_ptr_impl<U, V>* other) {
+    // See comment in templated constructor above regarding lack of support
+    // for move-only deleters.
+    reset(other->release());
+    get_deleter() = other->get_deleter();
+  }
+
+  ~scoped_ptr_impl() {
+    if (data_.ptr != NULL) {
+      // Not using get_deleter() saves one function call in non-optimized
+      // builds.
+      static_cast<D&>(data_)(data_.ptr);
+    }
+  }
+
+  void reset(T* p) {
+    // This self-reset check is deprecated.
+    // this->reset(this->get()) currently works, but it is DEPRECATED, and
+    // will be removed once we verify that no one depends on it.
+    //
+    // TODO(ajwong): Change this behavior to match unique_ptr<>.
+    // http://crbug.com/162971
+    if (p != data_.ptr) {
+      if (data_.ptr != NULL) {
+        // Note that this can lead to undefined behavior and memory leaks
+        // in the unlikely but possible case that get_deleter()(get())
+        // indirectly deletes this. The fix is to reset ptr_ before deleting
+        // its old value, but first we need to clean up the code that relies
+        // on the current sequencing.
+        static_cast<D&>(data_)(data_.ptr);
+      }
+      data_.ptr = p;
+    }
+  }
+
+  T* get() const { return data_.ptr; }
+
+  D& get_deleter() { return data_; }
+  const D& get_deleter() const { return data_; }
+
+  void swap(scoped_ptr_impl& p2) {
+    // Standard swap idiom: 'using std::swap' ensures that std::swap is
+    // present in the overload set, but we call swap unqualified so that
+    // any more-specific overloads can be used, if available.
+    using std::swap;
+    swap(static_cast<D&>(data_), static_cast<D&>(p2.data_));
+    swap(data_.ptr, p2.data_.ptr);
+  }
+
+  T* release() {
+    T* old_ptr = data_.ptr;
+    data_.ptr = NULL;
+    return old_ptr;
+  }
+
+ private:
+  // Needed to allow type-converting constructor.
+  template <typename U, typename V> friend class scoped_ptr_impl;
+
+  // Use the empty base class optimization to allow us to have a D
+  // member, while avoiding any space overhead for it when D is an
+  // empty class.  See e.g. http://www.cantrip.org/emptyopt.html for a good
+  // discussion of this technique.
+  struct Data : public D {
+    explicit Data(T* ptr_in) : ptr(ptr_in) {}
+    Data(T* ptr_in, const D& other) : D(other), ptr(ptr_in) {}
+    T* ptr;
+  };
+
+  Data data_;
+
+  DISALLOW_COPY_AND_ASSIGN(scoped_ptr_impl);
+};
+
 }  // namespace internal
+
 }  // namespace base
 
 // A scoped_ptr<T> is like a T*, except that the destructor of scoped_ptr<T>
@@ -127,95 +292,109 @@ template <typename T> struct IsNotRefCounted {
 // Also like T*, scoped_ptr<T> is thread-compatible, and once you
 // dereference it, you get the thread safety guarantees of T.
 //
-// The size of a scoped_ptr is small:
-// sizeof(scoped_ptr<C>) == sizeof(C*)
-template <class C>
+// The size of scoped_ptr is small. On most compilers, when using the
+// DefaultDeleter, sizeof(scoped_ptr<T>) == sizeof(T*). Custom deleters will
+// increase the size proportional to whatever state they need to have. See
+// comments inside scoped_ptr_impl<> for details.
+//
+// Current implementation targets having a strict subset of  C++11's
+// unique_ptr<> features. Known deficiencies include not supporting move-only
+// deleteres, function pointers as deleters, and deleters with reference
+// types.
+template <class T, class D = base::DefaultDeleter<T> >
 class scoped_ptr {
   MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr, RValue)
 
-  COMPILE_ASSERT(base::internal::IsNotRefCounted<C>::value,
-                 C_is_refcounted_type_and_needs_scoped_refptr);
+  COMPILE_ASSERT(base::internal::IsNotRefCounted<T>::value,
+                 T_is_refcounted_type_and_needs_scoped_refptr);
 
  public:
-
-  // The element type
-  typedef C element_type;
+  // The element and deleter types.
+  typedef T element_type;
+  typedef D deleter_type;
 
   // Constructor.  Defaults to initializing with NULL.
-  // There is no way to create an uninitialized scoped_ptr.
-  // The input parameter must be allocated with new.
-  explicit scoped_ptr(C* p = NULL) : ptr_(p) { }
+  scoped_ptr() : impl_(NULL) { }
 
-  // Constructor.  Allows construction from a scoped_ptr rvalue for a
-  // convertible type.
-  template <typename U>
-  scoped_ptr(scoped_ptr<U> other) : ptr_(other.release()) { }
+  // Constructor.  Takes ownership of p.
+  explicit scoped_ptr(element_type* p) : impl_(p) { }
 
-  // Constructor.  Move constructor for C++03 move emulation of this type.
-  scoped_ptr(RValue rvalue)
-      : ptr_(rvalue.object->release()) {
-  }
+  // Constructor.  Allows initialization of a stateful deleter.
+  scoped_ptr(element_type* p, const D& d) : impl_(p, d) { }
 
-  // Destructor.  If there is a C object, delete it.
-  // We don't need to test ptr_ == NULL because C++ does that for us.
-  ~scoped_ptr() {
-    enum { type_must_be_complete = sizeof(C) };
-    delete ptr_;
+  // Constructor.  Allows construction from a scoped_ptr rvalue for a
+  // convertible type and deleter.
+  //
+  // IMPLEMENTATION NOTE: C++11 unique_ptr<> keeps this constructor distinct
+  // from the normal move constructor. By C++11 20.7.1.2.1.21, this constructor
+  // has different post-conditions if D is a reference type. Since this
+  // implementation does not support deleters with reference type,
+  // we do not need a separate move constructor allowing us to avoid one
+  // use of SFINAE. You only need to care about this if you modify the
+  // implementation of scoped_ptr.
+  template <typename U, typename V>
+  scoped_ptr(scoped_ptr<U, V> other) : impl_(&other.impl_) {
+    COMPILE_ASSERT(!base::is_array<U>::value, U_cannot_be_an_array);
   }
 
-  // operator=.  Allows assignment from a scoped_ptr rvalue for a convertible
-  // type.
-  template <typename U>
-  scoped_ptr& operator=(scoped_ptr<U> rhs) {
-    reset(rhs.release());
-    return *this;
-  }
+  // Constructor.  Move constructor for C++03 move emulation of this type.
+  scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { }
 
-  // operator=.  Move operator= for C++03 move emulation of this type.
-  scoped_ptr& operator=(RValue rhs) {
-    swap(*rhs->object);
+  // operator=.  Allows assignment from a scoped_ptr rvalue for a convertible
+  // type and deleter.
+  //
+  // IMPLEMENTATION NOTE: C++11 unique_ptr<> keeps this operator= distinct from
+  // the normal move assignment operator. By C++11 20.7.1.2.3.4, this templated
+  // form has different requirements on for move-only Deleters. Since this
+  // implementation does not support move-only Deleters, we do not need a
+  // separate move assignment operator allowing us to avoid one use of SFINAE.
+  // You only need to care about this if you modify the implementation of
+  // scoped_ptr.
+  template <typename U, typename V>
+  scoped_ptr& operator=(scoped_ptr<U, V> rhs) {
+    COMPILE_ASSERT(!base::is_array<U>::value, U_cannot_be_an_array);
+    impl_.TakeState(&rhs.impl_);
     return *this;
   }
 
-  // Reset.  Deletes the current owned object, if any.
+  // Reset.  Deletes the currently owned object, if any.
   // Then takes ownership of a new object, if given.
-  // this->reset(this->get()) works.
-  void reset(C* p = NULL) {
-    if (p != ptr_) {
-      enum { type_must_be_complete = sizeof(C) };
-      delete ptr_;
-      ptr_ = p;
-    }
-  }
+  void reset(element_type* p = NULL) { impl_.reset(p); }
 
   // Accessors to get the owned object.
   // operator* and operator-> will assert() if there is no current object.
-  C& operator*() const {
-    assert(ptr_ != NULL);
-    return *ptr_;
+  element_type& operator*() const {
+    assert(impl_.get() != NULL);
+    return *impl_.get();
   }
-  C* operator->() const  {
-    assert(ptr_ != NULL);
-    return ptr_;
+  element_type* operator->() const  {
+    assert(impl_.get() != NULL);
+    return impl_.get();
   }
-  C* get() const { return ptr_; }
+  element_type* get() const { return impl_.get(); }
 
-  // Allow scoped_ptr<C> to be used in boolean expressions, but not
+  // Access to the deleter.
+  deleter_type& get_deleter() { return impl_.get_deleter(); }
+  const deleter_type& get_deleter() const { return impl_.get_deleter(); }
+
+  // Allow scoped_ptr<element_type> to be used in boolean expressions, but not
   // implicitly convertible to a real bool (which is dangerous).
-  typedef C* scoped_ptr::*Testable;
-  operator Testable() const { return ptr_ ? &scoped_ptr::ptr_ : NULL; }
+ private:
+  typedef base::internal::scoped_ptr_impl<element_type, deleter_type>
+      scoped_ptr::*Testable;
+
+ public:
+  operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; }
 
   // Comparison operators.
   // These return whether two scoped_ptr refer to the same object, not just to
   // two different but equal objects.
-  bool operator==(C* p) const { return ptr_ == p; }
-  bool operator!=(C* p) const { return ptr_ != p; }
+  bool operator==(element_type* p) const { return impl_.get() == p; }
+  bool operator!=(element_type* p) const { return impl_.get() != p; }
 
   // Swap two scoped pointers.
   void swap(scoped_ptr& p2) {
-    C* tmp = ptr_;
-    ptr_ = p2.ptr_;
-    p2.ptr_ = tmp;
+    impl_.swap(p2.impl_);
   }
 
   // Release a pointer.
@@ -223,44 +402,163 @@ class scoped_ptr {
   // If this object holds a NULL pointer, the return value is NULL.
   // After this operation, this object will hold a NULL pointer,
   // and will not own the object any more.
-  C* release() WARN_UNUSED_RESULT {
-    C* retVal = ptr_;
-    ptr_ = NULL;
-    return retVal;
+  element_type* release() WARN_UNUSED_RESULT {
+    return impl_.release();
   }
 
+  // C++98 doesn't support functions templates with default parameters which
+  // makes it hard to write a PassAs() that understands converting the deleter
+  // while preserving simple calling semantics.
+  //
+  // Until there is a use case for PassAs() with custom deleters, just ignore
+  // the custom deleter.
   template <typename PassAsType>
   scoped_ptr<PassAsType> PassAs() {
-    return scoped_ptr<PassAsType>(release());
+    return scoped_ptr<PassAsType>(Pass());
   }
 
  private:
-  C* ptr_;
+  // Needed to reach into |impl_| in the constructor.
+  template <typename U, typename V> friend class scoped_ptr;
+  base::internal::scoped_ptr_impl<element_type, deleter_type> impl_;
+
+  // Forbid comparison of scoped_ptr types.  If U != T, it totally
+  // doesn't make sense, and if U == T, it still doesn't make sense
+  // because you should never have the same object owned by two different
+  // scoped_ptrs.
+  template <class U> bool operator==(scoped_ptr<U> const& p2) const;
+  template <class U> bool operator!=(scoped_ptr<U> const& p2) const;
+};
+
+template <class T, class D>
+class scoped_ptr<T[], D> {
+  MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr, RValue)
+
+ public:
+  // The element and deleter types.
+  typedef T element_type;
+  typedef D deleter_type;
+
+  // Constructor.  Defaults to initializing with NULL.
+  scoped_ptr() : impl_(NULL) { }
+
+  // Constructor. Stores the given array. Note that the argument's type
+  // must exactly match T*. In particular:
+  // - it cannot be a pointer to a type derived from T, because it is
+  //   inherently unsafe in the general case to access an array through a
+  //   pointer whose dynamic type does not match its static type (eg., if
+  //   T and the derived types had different sizes access would be
+  //   incorrectly calculated). Deletion is also always undefined
+  //   (C++98 [expr.delete]p3). If you're doing this, fix your code.
+  // - it cannot be NULL, because NULL is an integral expression, not a
+  //   pointer to T. Use the no-argument version instead of explicitly
+  //   passing NULL.
+  // - it cannot be const-qualified differently from T per unique_ptr spec
+  //   (http://cplusplus.github.com/LWG/lwg-active.html#2118). Users wanting
+  //   to work around this may use implicit_cast<const T*>().
+  //   However, because of the first bullet in this comment, users MUST
+  //   NOT use implicit_cast<Base*>() to upcast the static type of the array.
+  explicit scoped_ptr(element_type* array) : impl_(array) { }
+
+  // Constructor.  Move constructor for C++03 move emulation of this type.
+  scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { }
+
+  // operator=.  Move operator= for C++03 move emulation of this type.
+  scoped_ptr& operator=(RValue rhs) {
+    impl_.TakeState(&rhs.object->impl_);
+    return *this;
+  }
+
+  // Reset.  Deletes the currently owned array, if any.
+  // Then takes ownership of a new object, if given.
+  void reset(element_type* array = NULL) { impl_.reset(array); }
+
+  // Accessors to get the owned array.
+  element_type& operator[](size_t i) const {
+    assert(impl_.get() != NULL);
+    return impl_.get()[i];
+  }
+  element_type* get() const { return impl_.get(); }
+
+  // Access to the deleter.
+  deleter_type& get_deleter() { return impl_.get_deleter(); }
+  const deleter_type& get_deleter() const { return impl_.get_deleter(); }
 
-  // Forbid comparison of scoped_ptr types.  If C2 != C, it totally doesn't
-  // make sense, and if C2 == C, it still doesn't make sense because you should
-  // never have the same object owned by two different scoped_ptrs.
-  template <class C2> bool operator==(scoped_ptr<C2> const& p2) const;
-  template <class C2> bool operator!=(scoped_ptr<C2> const& p2) const;
+  // Allow scoped_ptr<element_type> to be used in boolean expressions, but not
+  // implicitly convertible to a real bool (which is dangerous).
+ private:
+  typedef base::internal::scoped_ptr_impl<element_type, deleter_type>
+      scoped_ptr::*Testable;
+
+ public:
+  operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; }
+
+  // Comparison operators.
+  // These return whether two scoped_ptr refer to the same object, not just to
+  // two different but equal objects.
+  bool operator==(element_type* array) const { return impl_.get() == array; }
+  bool operator!=(element_type* array) const { return impl_.get() != array; }
+
+  // Swap two scoped pointers.
+  void swap(scoped_ptr& p2) {
+    impl_.swap(p2.impl_);
+  }
 
+  // Release a pointer.
+  // The return value is the current pointer held by this object.
+  // If this object holds a NULL pointer, the return value is NULL.
+  // After this operation, this object will hold a NULL pointer,
+  // and will not own the object any more.
+  element_type* release() WARN_UNUSED_RESULT {
+    return impl_.release();
+  }
+
+ private:
+  // Force element_type to be a complete type.
+  enum { type_must_be_complete = sizeof(element_type) };
+
+  // Actually hold the data.
+  base::internal::scoped_ptr_impl<element_type, deleter_type> impl_;
+
+  // Disable initialization from any type other than element_type*, by
+  // providing a constructor that matches such an initialization, but is
+  // private and has no definition. This is disabled because it is not safe to
+  // call delete[] on an array whose static type does not match its dynamic
+  // type.
+  template <typename U>
+  explicit scoped_ptr(U* array);
+
+  // Disable reset() from any type other than element_type*, for the same
+  // reasons as the constructor above.
+  template <typename U>
+  void reset(U* array);

[Ryan Sleevi, 2012/12/15 02:24:12]

nit: unnecessary line wrapping (and on 529), particularly when compared to
540/541 and 429/430.

[awong, 2012/12/15 02:32:27]

Done.

+
+  // Forbid comparison of scoped_ptr types.  If U != T, it totally
+  // doesn't make sense, and if U == T, it still doesn't make sense
+  // because you should never have the same object owned by two different
+  // scoped_ptrs.
+  template <class U> bool operator==(scoped_ptr<U> const& p2) const;
+  template <class U> bool operator!=(scoped_ptr<U> const& p2) const;
 };
 
 // Free functions
-template <class C>
-void swap(scoped_ptr<C>& p1, scoped_ptr<C>& p2) {
+template <class T, class D>
+void swap(scoped_ptr<T, D>& p1, scoped_ptr<T, D>& p2) {
   p1.swap(p2);
 }
 
-template <class C>
-bool operator==(C* p1, const scoped_ptr<C>& p2) {
+template <class T, class D>
+bool operator==(T* p1, const scoped_ptr<T, D>& p2) {
   return p1 == p2.get();
 }
 
-template <class C>
-bool operator!=(C* p1, const scoped_ptr<C>& p2) {
+template <class T, class D>
+bool operator!=(T* p1, const scoped_ptr<T, D>& p2) {
   return p1 != p2.get();
 }
 
+// DEPRECATED: Use scoped_ptr<C[]> instead.
+//
 // scoped_array<C> is like scoped_ptr<C>, except that the caller must allocate
 // with new [] and the destructor deletes objects with delete [].
 //
@@ -298,7 +596,7 @@ class scoped_array {
 
   // operator=.  Move operator= for C++03 move emulation of this type.
   scoped_array& operator=(RValue rhs) {
-    swap(*rhs.object);
+    reset(rhs.object->release());
     return *this;
   }
 
@@ -380,19 +678,12 @@ bool operator!=(C* p1, const scoped_array<C>& p2) {
   return p1 != p2.get();
 }
 
-// This class wraps the c library function free() in a class that can be
-// passed as a template argument to scoped_ptr_malloc below.
-class ScopedPtrMallocFree {
- public:
-  inline void operator()(void* x) const {
-    free(x);
-  }
-};
-
+// DEPRECATED: Use scoped_ptr<C, base::FreeDeleter> instead.
+//
 // scoped_ptr_malloc<> is similar to scoped_ptr<>, but it accepts a
 // second template argument, the functor used to free the object.
 
-template<class C, class FreeProc = ScopedPtrMallocFree>
+template<class C, class FreeProc = base::FreeDeleter>
 class scoped_ptr_malloc {
   MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr_malloc, RValue)
 
@@ -420,7 +711,7 @@ class scoped_ptr_malloc {
 
   // operator=.  Move operator= for C++03 move emulation of this type.
   scoped_ptr_malloc& operator=(RValue rhs) {
-    swap(*rhs.object);
+    reset(rhs.object->release());
     return *this;
   }
 
@@ -429,8 +720,10 @@ class scoped_ptr_malloc {
   // this->reset(this->get()) works.
   void reset(C* p = NULL) {
     if (ptr_ != p) {
-      FreeProc free_proc;
-      free_proc(ptr_);
+      if (ptr_ != NULL) {
+        FreeProc free_proc;
+        free_proc(ptr_);
+      }
       ptr_ = p;
     }
   }