Use std::default_delete as the default deleter for scoped_ptr.

base/memory/scoped_ptr.h

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // Scopers help you manage ownership of a pointer, helping you easily manage a
 // pointer within a scope, and automatically destroying the pointer at the end
 // of a scope.  There are two main classes you will use, which correspond to the
 // operators new/delete and new[]/delete[].
 //
 // Example usage (scoped_ptr<T>):
@@ skipping 66 matching lines @@
 #ifndef BASE_MEMORY_SCOPED_PTR_H_
 #define BASE_MEMORY_SCOPED_PTR_H_
 
 // This is an implementation designed to match the anticipated future TR2
 // implementation of the scoped_ptr class.
 
 #include <assert.h>
 #include <stddef.h>
 #include <stdlib.h>
 
-#include <algorithm>  // For std::swap().
 #include <iosfwd>
+#include <memory>
+#include <utility>

[dcheng, 2015/11/16 18:22:59]

In line with bratell@'s proposal to #include <utility> instead of <algorithm>, I
went ahead and just changed it here (since it's included so widely).

 
 #include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/move.h"
 #include "base/template_util.h"
 
 namespace base {
 
 namespace subtle {
 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) {
-    // IMPLEMENTATION NOTE: C++11 20.7.1.1.2p2 only provides this constructor
-    // if U* is implicitly convertible to T* and U is not an array type.
-    //
-    // Correct implementation should use SFINAE to disable this
-    // constructor. However, since there are no other 1-argument constructors,
-    // 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
-    // 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 declare something like scoped_ptr<int[10]>.

[danakj, 2015/11/16 22:19:02]

Is this check provided by libc++ somewhere? Should we implement something to
watch for this in clang/plugin or presubmit?

[dcheng, 2015/11/17 17:08:42]

It's not explicitly provided by libc++. For example, this compiles:
  std::unique_ptr<int[5]> p;

But this does not:
  std::unique_ptr<int[5]> a(new int[5]);

(Like many C++ warnings, the compiler error can be cast around, but hopefully if
someone's doing that, they're going to have second thoughts after writing the
cast...)

So I think we're OK in that department.

-  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 {
   enum {
     value = !base::is_convertible<T*, base::subtle::RefCountedBase*>::value &&
         !base::is_convertible<T*, base::subtle::RefCountedThreadSafeBase*>::
             value
   };
 };
 
 template <typename T>
 struct ShouldAbortOnSelfReset {

[danakj, 2015/11/16 22:19:02]

Does this go away then?

[dcheng, 2015/11/17 17:08:42]

Yes, it should.

   template <typename U>
   static NoType Test(const typename U::AllowSelfReset*);

[danakj, 2015/11/16 22:19:02]

And any instances of AllowSelfReset in the codebase?

[dcheng, 2015/11/17 17:08:42]

Good catch, I forgot that there's some code in //net using it (kind of
embarrassing since I think I'm the one that was involved in writing this).

 
   template <typename U>
   static YesType Test(...);
 
   static const bool value = sizeof(Test<T>(0)) == sizeof(YesType);
 };
 
 // Minimal implementation of the core logic of scoped_ptr, suitable for
 // reuse in both scoped_ptr and its specializations.
 template <class T, class D>
@@ skipping 31 matching lines @@
     //    in a reference cycle (see ScopedPtrTest.ReferenceCycle).
     // 3. If |this| is accessed in the future, in a use-after-free bug, attempts
     //    to dereference |this|'s pointer should cause either a failure or a
     //    segfault closer to the problem. If |this| wasn't reset to nullptr,
     //    the access would cause the deleted memory to be read or written
     //    leading to other more subtle issues.
     reset(nullptr);
   }
 
   void reset(T* p) {
-    // This is a self-reset, which is no longer allowed for default deleters:
-    // https://crbug.com/162971
-    assert(!ShouldAbortOnSelfReset<D>::value || p == nullptr || p != data_.ptr);

[danakj, 2015/11/16 22:19:02]

I'm not sure why this went away in this CL TBH. I agree about making things
match, but using std::default_delete doesn't change our expectations here at
all, does it?

[dcheng, 2015/11/17 17:08:42]

This is just making the interface match std::unique_ptr more closely. The type
trait we use to detect this currently isn't implemented at all by libc++, nor is
the check.

If you feel strongly about this, I could make this select on whether or not the
default deleter is being used.

[danakj, 2015/11/17 19:09:27]

Yah, I kinda figured this would make a reasonable separate CL. Cuz
ShouldAbortOnSelfReset would work with default_delete too, right? Anyways, it
needs to go at some point so this is fine.

-
     // Match C++11's definition of unique_ptr::reset(), which requires changing
     // the pointer before invoking the deleter on the old pointer. This prevents
     // |this| from being accessed after the deleter is run, which may destroy
     // |this|.
     T* old = data_.ptr;
     data_.ptr = p;
     if (old != nullptr)
       static_cast<D&>(data_)(old);
   }
 
@@ skipping 41 matching lines @@
 }  // namespace base
 
 // A scoped_ptr<T> is like a T*, except that the destructor of scoped_ptr<T>
 // automatically deletes the pointer it holds (if any).
 // That is, scoped_ptr<T> owns the T object that it points to.
 // Like a T*, a scoped_ptr<T> may hold either nullptr or a pointer to a T
 // object. 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 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
+// std::default_delete, 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> >
+template <class T, class D = std::default_delete<T>>
 class scoped_ptr {
   MOVE_ONLY_TYPE_WITH_MOVE_CONSTRUCTOR_FOR_CPP_03(scoped_ptr)
 
   COMPILE_ASSERT(base::internal::IsNotRefCounted<T>::value,
                  T_is_refcounted_type_and_needs_scoped_refptr);
 
  public:
   // The element and deleter types.
   typedef T element_type;
   typedef D deleter_type;
@@ skipping 261 matching lines @@
 scoped_ptr<T> make_scoped_ptr(T* ptr) {
   return scoped_ptr<T>(ptr);
 }
 
 template <typename T>
 std::ostream& operator<<(std::ostream& out, const scoped_ptr<T>& p) {
   return out << p.get();
 }
 
 #endif  // BASE_MEMORY_SCOPED_PTR_H_