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..0c5a6a3d12e4ec2a9dbdea940c2a57ecb73b5431 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,47 @@ 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) {
+    // All default single-object deleters can trivially convert to one another.
+  }
+  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 unsafe to execute
+  // an array delete when the static type of the array mismatches the dynamic
+  // type.
+  template <typename U> void operator()(U* array) const;
+};
+
+// 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))));

[Jeffrey Yasskin, 2012/11/28 06:07:18]

static_cast<int*>, right?

[awong, 2012/11/28 10:20:35]

Right. :)

+struct FreeDeleter {
+  inline void operator()(void* ptr) const {

[Jeffrey Yasskin, 2012/11/28 06:07:18]

This isn't going to work on const or volatile pointers, but maybe we don't care
much, especially about the volatile ones.

[awong, 2012/11/28 10:20:35]

I don't think we care. This is basically staging a replacement of
scoped_ptr_malloc which already has this problem.

+    free(ptr);
+  }
+};
+
 namespace internal {
 
 template <typename T> struct IsNotRefCounted {
@@ -117,7 +160,106 @@ 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 {
+  MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr_impl, RValue)
+
+ 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) {}
+
+  template <typename U, typename V>
+  scoped_ptr_impl(scoped_ptr_impl<U, V> other)
+    : data_(other.data_.ptr, other.data_) {
+    // 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.
+    other.data_.ptr = NULL;
+  }
+
+  template <typename U, typename V>
+  const scoped_ptr_impl& operator=(scoped_ptr_impl<U, V> rhs) {
+    // See comment in move type-coverting constructor above regarding lack of
+    // support for move-only deleters.
+    reset(rhs.release());
+    get_deleter() = rhs.get_deleter();
+    return *this;
+  }
+
+  scoped_ptr_impl(RValue rvalue) : data_(rvalue.object->data_) {
+    rvalue.object->data_.ptr = NULL;
+  }
+
+  ~scoped_ptr_impl() {
+    if (data_.ptr != NULL) {
+      get_deleter()(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.
+        get_deleter()(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_;
+};
+
 }  // namespace internal
+
 }  // namespace base
 
 // A scoped_ptr<T> is like a T*, except that the destructor of scoped_ptr<T>
@@ -129,93 +271,81 @@ template <typename T> struct IsNotRefCounted {
 //
 // The size of a scoped_ptr is small:
 // sizeof(scoped_ptr<C>) == sizeof(C*)
-template <class C>
+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.  Takes ownership of p.
+  explicit scoped_ptr(element_type* p) : impl_(p) { }
+
+  // Constructor.  Allows initialization of a stateful deleter.
+  scoped_ptr(element_type* p, const D& d) : impl_(p, d) { }
 
   // Constructor.  Allows construction from a scoped_ptr rvalue for a
-  // convertible type.
-  template <typename U>
-  scoped_ptr(scoped_ptr<U> other) : ptr_(other.release()) { }
+  // convertible type and deleter.
+  template <typename U, typename V>
+  scoped_ptr(scoped_ptr<U, V> other) : impl_(other.impl_.Pass()) { }
 
   // Constructor.  Move constructor for C++03 move emulation of this type.
-  scoped_ptr(RValue rvalue)
-      : ptr_(rvalue.object->release()) {
-  }
-
-  // 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_;
-  }
+  scoped_ptr(RValue rvalue) : impl_(rvalue.object->impl_.Pass()) { }
 
   // 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());
+  // type and deleter.
+  template <typename U, typename V>
+  scoped_ptr& operator=(scoped_ptr<U, V> rhs) {
+    impl_ = rhs.impl_.Pass();
     return *this;
   }
 
-  // operator=.  Move operator= for C++03 move emulation of this type.
-  scoped_ptr& operator=(RValue rhs) {
-    swap(*rhs->object);
-    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(); }
+
+  // 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<C> to be used in boolean expressions, but not
+  // 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,41 +353,154 @@ 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 to access an array through a pointer whose
+  //   dynamic type does not match its static type. If you're doing this,
+  //   fix your code.
+  //   http://cplusplus.github.com/LWG/lwg-defects.html#938
+  // - 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
+  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_.Pass()) { }
+
+  // operator=.  Move operator= for C++03 move emulation of this type.
+  scoped_ptr& operator=(RValue rhs) {
+    impl_ = rhs.object->impl_.Pass();
+    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(); }
 
-  // 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;
+  // 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).
+ 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);
+
+  // 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();
 }
 
@@ -298,7 +541,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;
   }
 
@@ -307,8 +550,10 @@ class scoped_array {
   // this->reset(this->get()) works.
   void reset(C* p = NULL) {
     if (p != array_) {
-      enum { type_must_be_complete = sizeof(C) };
-      delete[] array_;
+      if (array_ != NULL) {

[Jeffrey Yasskin, 2012/11/28 06:07:18]

Note that delete[] does nothing if array_ is NULL, so you don't need to add this
line. (There's a corner case when the delete[] is provided by a class, but I
don't think we need to worry about that.)

[awong, 2012/11/28 10:20:35]

yeah. Good point. I spent about an hour of headache with an tagentially related
unittest  because scoped_ptr_malloc,>::reset() was calling the deleter even when
it was NULL (this is disallowed by unique_ptr's spec) but you're right that this
case is always benign.

+        enum { type_must_be_complete = sizeof(C) };
+        delete[] array_;
+      }
       array_ = p;
     }
   }
@@ -380,19 +625,10 @@ 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);
-  }
-};
-
 // scoped_ptr_malloc<> is similar to scoped_ptr<>, but it accepts a

[Jeffrey Yasskin, 2012/11/28 06:07:18]

Should this be deprecated in favor of scoped_ptr<T, base::FreeDeleter>?

[awong, 2012/11/28 10:20:35]

Yes.

 // 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 +656,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 +665,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;
     }
   }