Commit a9527ce3 authored by danakj's avatar danakj Committed by Commit bot

Add nullptr support to scoped_ptr.

This adds support to use nullptr to construct, assign, or return a
scoped_ptr<T> and scoped_ptr<T[]>. Support for this requires the use
of a move-only constructor.

The changes are:

- Add a constructor that takes decltype(nullptr) as a parameter. This
allows behaviour such as scoped_ptr<T>(nullptr), but also allows a
function with return type scoped_ptr<T> to "return nullptr;" instead
of "return scoped_ptr<T>();".

- Add an operator=(decltype(nullptr)) that resets the scoped_ptr to
empty and deletes anything it held.

- Add/Modify a constructor to take a scoped_ptr<U,E>&& parameter for
constructing a scoped_ptr from another using move-only semantics. This
piece is critical for allowing the function returning nullptr to be
assigned to some other scoped_ptr at the callsite. In particular, take
the following code:
  scoped_ptr<T> Function() { return nullptr; }
  scoped_ptr<T> var = Function();
In this case the constructor which takes a nullptr allows Function() to
be written, but not to be used. The move-only constructor allows the
assignment from Function() to var. See "C++11 feature proposal:
Move-only constructors" on chromium-dev for more explanation why.

The scoped_ptr<T> class already had a constructor which took
scoped_ptr<U,E> as an argument, so this was changed to be
scoped_ptr<U,E>&& instead. The scoped_ptr<T[]> class had no such
constructor, so a scoped_ptr&& constructor was added. These match
the constructors found on the unique_ptr class.

- Remove the RValue type and the contructor that constructs a
scoped_ptr from an RValue. Change Pass() to return a scoped_ptr&&
instead of a scoped_ptr::RValue, to avoid the type conversion and
remove some complexity. This is done with a new emulation macro that
still provides Pass() and makes the type go down the MoveOnlyType
path in base::Callback code.

This adds base_unittests to demonstrate and use these changes.

The use of Pass() remains unchanged until std::move() is written
or allowed. At that time std::move() could be used instead of Pass.

R=brettw@chromium.org, jamesr@chromium.org

Committed: https://crrev.com/2299e91d3508f8d5d18ef990cf6024ea4371250a
Cr-Commit-Position: refs/heads/master@{#297072}

Review URL: https://codereview.chromium.org/599313003

Cr-Commit-Position: refs/heads/master@{#297116}
parent d4c67a01
...@@ -58,7 +58,7 @@ ...@@ -58,7 +58,7 @@
// TakesOwnership(ptr.Pass()); // ptr no longer owns Foo("yay"). // TakesOwnership(ptr.Pass()); // ptr no longer owns Foo("yay").
// scoped_ptr<Foo> ptr2 = CreateFoo(); // ptr2 owns the return Foo. // scoped_ptr<Foo> ptr2 = CreateFoo(); // ptr2 owns the return Foo.
// scoped_ptr<Foo> ptr3 = // ptr3 now owns what was in ptr2. // scoped_ptr<Foo> ptr3 = // ptr3 now owns what was in ptr2.
// PassThru(ptr2.Pass()); // ptr2 is correspondingly NULL. // PassThru(ptr2.Pass()); // ptr2 is correspondingly nullptr.
// } // }
// //
// Notice that if you do not call Pass() when returning from PassThru(), or // Notice that if you do not call Pass() when returning from PassThru(), or
...@@ -189,7 +189,7 @@ template <typename T> struct IsNotRefCounted { ...@@ -189,7 +189,7 @@ template <typename T> struct IsNotRefCounted {
template <class T, class D> template <class T, class D>
class scoped_ptr_impl { class scoped_ptr_impl {
public: public:
explicit scoped_ptr_impl(T* p) : data_(p) { } explicit scoped_ptr_impl(T* p) : data_(p) {}
// Initializer for deleters that have data parameters. // Initializer for deleters that have data parameters.
scoped_ptr_impl(T* p, const D& d) : data_(p, d) {} scoped_ptr_impl(T* p, const D& d) : data_(p, d) {}
...@@ -214,7 +214,7 @@ class scoped_ptr_impl { ...@@ -214,7 +214,7 @@ class scoped_ptr_impl {
} }
~scoped_ptr_impl() { ~scoped_ptr_impl() {
if (data_.ptr != NULL) { if (data_.ptr != nullptr) {
// Not using get_deleter() saves one function call in non-optimized // Not using get_deleter() saves one function call in non-optimized
// builds. // builds.
static_cast<D&>(data_)(data_.ptr); static_cast<D&>(data_)(data_.ptr);
...@@ -223,7 +223,7 @@ class scoped_ptr_impl { ...@@ -223,7 +223,7 @@ class scoped_ptr_impl {
void reset(T* p) { void reset(T* p) {
// This is a self-reset, which is no longer allowed: http://crbug.com/162971 // This is a self-reset, which is no longer allowed: http://crbug.com/162971
if (p != NULL && p == data_.ptr) if (p != nullptr && p == data_.ptr)
abort(); abort();
// Note that running data_.ptr = p can lead to undefined behavior if // Note that running data_.ptr = p can lead to undefined behavior if
...@@ -236,13 +236,13 @@ class scoped_ptr_impl { ...@@ -236,13 +236,13 @@ class scoped_ptr_impl {
// then it will incorrectly dispatch calls to |p| rather than the original // then it will incorrectly dispatch calls to |p| rather than the original
// value of |data_.ptr|. // value of |data_.ptr|.
// //
// During the transition period, set the stored pointer to NULL while // During the transition period, set the stored pointer to nullptr while
// deleting the object. Eventually, this safety check will be removed to // deleting the object. Eventually, this safety check will be removed to
// prevent the scenario initially described from occuring and // prevent the scenario initially described from occuring and
// http://crbug.com/176091 can be closed. // http://crbug.com/176091 can be closed.
T* old = data_.ptr; T* old = data_.ptr;
data_.ptr = NULL; data_.ptr = nullptr;
if (old != NULL) if (old != nullptr)
static_cast<D&>(data_)(old); static_cast<D&>(data_)(old);
data_.ptr = p; data_.ptr = p;
} }
...@@ -263,7 +263,7 @@ class scoped_ptr_impl { ...@@ -263,7 +263,7 @@ class scoped_ptr_impl {
T* release() { T* release() {
T* old_ptr = data_.ptr; T* old_ptr = data_.ptr;
data_.ptr = NULL; data_.ptr = nullptr;
return old_ptr; return old_ptr;
} }
...@@ -293,8 +293,8 @@ class scoped_ptr_impl { ...@@ -293,8 +293,8 @@ class scoped_ptr_impl {
// A scoped_ptr<T> is like a T*, except that the destructor of scoped_ptr<T> // A scoped_ptr<T> is like a T*, except that the destructor of scoped_ptr<T>
// automatically deletes the pointer it holds (if any). // automatically deletes the pointer it holds (if any).
// That is, scoped_ptr<T> owns the T object that it points to. // That is, scoped_ptr<T> owns the T object that it points to.
// Like a T*, a scoped_ptr<T> may hold either NULL or a pointer to a T object. // Like a T*, a scoped_ptr<T> may hold either nullptr or a pointer to a T
// Also like T*, scoped_ptr<T> is thread-compatible, and once you // object. Also like T*, scoped_ptr<T> is thread-compatible, and once you
// dereference it, you get the thread safety guarantees of T. // dereference it, you get the thread safety guarantees of T.
// //
// The size of scoped_ptr is small. On most compilers, when using the // The size of scoped_ptr is small. On most compilers, when using the
...@@ -318,14 +318,17 @@ class scoped_ptr { ...@@ -318,14 +318,17 @@ class scoped_ptr {
typedef T element_type; typedef T element_type;
typedef D deleter_type; typedef D deleter_type;
// Constructor. Defaults to initializing with NULL. // Constructor. Defaults to initializing with nullptr.
scoped_ptr() : impl_(NULL) { } scoped_ptr() : impl_(nullptr) {}
// Constructor. Takes ownership of p. // Constructor. Takes ownership of p.
explicit scoped_ptr(element_type* p) : impl_(p) { } explicit scoped_ptr(element_type* p) : impl_(p) {}
// Constructor. Allows initialization of a stateful deleter. // Constructor. Allows initialization of a stateful deleter.
scoped_ptr(element_type* p, const D& d) : impl_(p, d) { } scoped_ptr(element_type* p, const D& d) : impl_(p, d) {}
// Constructor. Allows construction from a nullptr.
scoped_ptr(decltype(nullptr)) : impl_(nullptr) {}
// Constructor. Allows construction from a scoped_ptr rvalue for a // Constructor. Allows construction from a scoped_ptr rvalue for a
// convertible type and deleter. // convertible type and deleter.
...@@ -338,12 +341,13 @@ class scoped_ptr { ...@@ -338,12 +341,13 @@ class scoped_ptr {
// use of SFINAE. You only need to care about this if you modify the // use of SFINAE. You only need to care about this if you modify the
// implementation of scoped_ptr. // implementation of scoped_ptr.
template <typename U, typename V> template <typename U, typename V>
scoped_ptr(scoped_ptr<U, V> other) : impl_(&other.impl_) { scoped_ptr(scoped_ptr<U, V>&& other)
: impl_(&other.impl_) {
COMPILE_ASSERT(!base::is_array<U>::value, U_cannot_be_an_array); COMPILE_ASSERT(!base::is_array<U>::value, U_cannot_be_an_array);
} }
// Constructor. Move constructor for C++03 move emulation of this type. // Constructor. Move constructor for C++03 move emulation of this type.
scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { } scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) {}
// operator=. Allows assignment from a scoped_ptr rvalue for a convertible // operator=. Allows assignment from a scoped_ptr rvalue for a convertible
// type and deleter. // type and deleter.
...@@ -356,24 +360,31 @@ class scoped_ptr { ...@@ -356,24 +360,31 @@ class scoped_ptr {
// You only need to care about this if you modify the implementation of // You only need to care about this if you modify the implementation of
// scoped_ptr. // scoped_ptr.
template <typename U, typename V> template <typename U, typename V>
scoped_ptr& operator=(scoped_ptr<U, V> rhs) { scoped_ptr& operator=(scoped_ptr<U, V>&& rhs) {
COMPILE_ASSERT(!base::is_array<U>::value, U_cannot_be_an_array); COMPILE_ASSERT(!base::is_array<U>::value, U_cannot_be_an_array);
impl_.TakeState(&rhs.impl_); impl_.TakeState(&rhs.impl_);
return *this; return *this;
} }
// operator=. Allows assignment from a nullptr. Deletes the currently owned
// object, if any.
scoped_ptr& operator=(decltype(nullptr)) {
reset();
return *this;
}
// Reset. Deletes the currently owned object, if any. // Reset. Deletes the currently owned object, if any.
// Then takes ownership of a new object, if given. // Then takes ownership of a new object, if given.
void reset(element_type* p = NULL) { impl_.reset(p); } void reset(element_type* p = nullptr) { impl_.reset(p); }
// Accessors to get the owned object. // Accessors to get the owned object.
// operator* and operator-> will assert() if there is no current object. // operator* and operator-> will assert() if there is no current object.
element_type& operator*() const { element_type& operator*() const {
assert(impl_.get() != NULL); assert(impl_.get() != nullptr);
return *impl_.get(); return *impl_.get();
} }
element_type* operator->() const { element_type* operator->() const {
assert(impl_.get() != NULL); assert(impl_.get() != nullptr);
return impl_.get(); return impl_.get();
} }
element_type* get() const { return impl_.get(); } element_type* get() const { return impl_.get(); }
...@@ -394,7 +405,9 @@ class scoped_ptr { ...@@ -394,7 +405,9 @@ class scoped_ptr {
scoped_ptr::*Testable; scoped_ptr::*Testable;
public: public:
operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; } operator Testable() const {
return impl_.get() ? &scoped_ptr::impl_ : nullptr;
}
// Comparison operators. // Comparison operators.
// These return whether two scoped_ptr refer to the same object, not just to // These return whether two scoped_ptr refer to the same object, not just to
...@@ -408,10 +421,9 @@ class scoped_ptr { ...@@ -408,10 +421,9 @@ class scoped_ptr {
} }
// Release a pointer. // Release a pointer.
// The return value is the current pointer held by this object. // The return value is the current pointer held by this object. If this object
// If this object holds a NULL pointer, the return value is NULL. // holds a nullptr, the return value is nullptr. After this operation, this
// After this operation, this object will hold a NULL pointer, // object will hold a nullptr, and will not own the object any more.
// and will not own the object any more.
element_type* release() WARN_UNUSED_RESULT { element_type* release() WARN_UNUSED_RESULT {
return impl_.release(); return impl_.release();
} }
...@@ -452,8 +464,8 @@ class scoped_ptr<T[], D> { ...@@ -452,8 +464,8 @@ class scoped_ptr<T[], D> {
typedef T element_type; typedef T element_type;
typedef D deleter_type; typedef D deleter_type;
// Constructor. Defaults to initializing with NULL. // Constructor. Defaults to initializing with nullptr.
scoped_ptr() : impl_(NULL) { } scoped_ptr() : impl_(nullptr) {}
// Constructor. Stores the given array. Note that the argument's type // Constructor. Stores the given array. Note that the argument's type
// must exactly match T*. In particular: // must exactly match T*. In particular:
...@@ -463,18 +475,27 @@ class scoped_ptr<T[], D> { ...@@ -463,18 +475,27 @@ class scoped_ptr<T[], D> {
// T and the derived types had different sizes access would be // T and the derived types had different sizes access would be
// incorrectly calculated). Deletion is also always undefined // incorrectly calculated). Deletion is also always undefined
// (C++98 [expr.delete]p3). If you're doing this, fix your code. // (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 // - it cannot be const-qualified differently from T per unique_ptr spec
// (http://cplusplus.github.com/LWG/lwg-active.html#2118). Users wanting // (http://cplusplus.github.com/LWG/lwg-active.html#2118). Users wanting
// to work around this may use implicit_cast<const T*>(). // to work around this may use implicit_cast<const T*>().
// However, because of the first bullet in this comment, users MUST // However, because of the first bullet in this comment, users MUST
// NOT use implicit_cast<Base*>() to upcast the static type of the array. // NOT use implicit_cast<Base*>() to upcast the static type of the array.
explicit scoped_ptr(element_type* array) : impl_(array) { } explicit scoped_ptr(element_type* array) : impl_(array) {}
// Constructor. Allows construction from a nullptr.
scoped_ptr(decltype(nullptr)) : impl_(nullptr) {}
// Constructor. Allows construction from a scoped_ptr rvalue.
scoped_ptr(scoped_ptr&& other) : impl_(&other.impl_) {}
// Constructor. Move constructor for C++03 move emulation of this type. // Constructor. Move constructor for C++03 move emulation of this type.
scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { } scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) {}
// operator=. Allows assignment from a scoped_ptr rvalue.
scoped_ptr& operator=(scoped_ptr&& rhs) {
impl_.TakeState(&rhs.impl_);
return *this;
}
// operator=. Move operator= for C++03 move emulation of this type. // operator=. Move operator= for C++03 move emulation of this type.
scoped_ptr& operator=(RValue rhs) { scoped_ptr& operator=(RValue rhs) {
...@@ -482,13 +503,20 @@ class scoped_ptr<T[], D> { ...@@ -482,13 +503,20 @@ class scoped_ptr<T[], D> {
return *this; return *this;
} }
// operator=. Allows assignment from a nullptr. Deletes the currently owned
// array, if any.
scoped_ptr& operator=(decltype(nullptr)) {
reset();
return *this;
}
// Reset. Deletes the currently owned array, if any. // Reset. Deletes the currently owned array, if any.
// Then takes ownership of a new object, if given. // Then takes ownership of a new object, if given.
void reset(element_type* array = NULL) { impl_.reset(array); } void reset(element_type* array = nullptr) { impl_.reset(array); }
// Accessors to get the owned array. // Accessors to get the owned array.
element_type& operator[](size_t i) const { element_type& operator[](size_t i) const {
assert(impl_.get() != NULL); assert(impl_.get() != nullptr);
return impl_.get()[i]; return impl_.get()[i];
} }
element_type* get() const { return impl_.get(); } element_type* get() const { return impl_.get(); }
...@@ -504,7 +532,9 @@ class scoped_ptr<T[], D> { ...@@ -504,7 +532,9 @@ class scoped_ptr<T[], D> {
scoped_ptr::*Testable; scoped_ptr::*Testable;
public: public:
operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; } operator Testable() const {
return impl_.get() ? &scoped_ptr::impl_ : nullptr;
}
// Comparison operators. // Comparison operators.
// These return whether two scoped_ptr refer to the same object, not just to // These return whether two scoped_ptr refer to the same object, not just to
...@@ -518,10 +548,9 @@ class scoped_ptr<T[], D> { ...@@ -518,10 +548,9 @@ class scoped_ptr<T[], D> {
} }
// Release a pointer. // Release a pointer.
// The return value is the current pointer held by this object. // The return value is the current pointer held by this object. If this object
// If this object holds a NULL pointer, the return value is NULL. // holds a nullptr, the return value is nullptr. After this operation, this
// After this operation, this object will hold a NULL pointer, // object will hold a nullptr, and will not own the object any more.
// and will not own the object any more.
element_type* release() WARN_UNUSED_RESULT { element_type* release() WARN_UNUSED_RESULT {
return impl_.release(); return impl_.release();
} }
......
...@@ -406,6 +406,8 @@ TEST(ScopedPtrTest, PassBehavior) { ...@@ -406,6 +406,8 @@ TEST(ScopedPtrTest, PassBehavior) {
// Should auto-destruct logger by end of scope. // Should auto-destruct logger by end of scope.
scoper.Pass(); scoper.Pass();
// This differs from unique_ptr, as Pass() has side effects but std::move()
// does not.
EXPECT_FALSE(scoper.get()); EXPECT_FALSE(scoper.get());
} }
EXPECT_EQ(0, constructed); EXPECT_EQ(0, constructed);
...@@ -602,3 +604,55 @@ TEST(ScopedPtrTest, OverloadedNewAndDelete) { ...@@ -602,3 +604,55 @@ TEST(ScopedPtrTest, OverloadedNewAndDelete) {
EXPECT_EQ(1, OverloadedNewAndDelete::delete_count()); EXPECT_EQ(1, OverloadedNewAndDelete::delete_count());
EXPECT_EQ(1, OverloadedNewAndDelete::new_count()); EXPECT_EQ(1, OverloadedNewAndDelete::new_count());
} }
scoped_ptr<int> NullIntReturn() {
return nullptr;
}
TEST(ScopedPtrTest, Nullptr) {
scoped_ptr<int> scoper1(nullptr);
scoped_ptr<int> scoper2(new int);
scoper2 = nullptr;
scoped_ptr<int> scoper3(NullIntReturn());
scoped_ptr<int> scoper4 = NullIntReturn();
EXPECT_EQ(nullptr, scoper1.get());
EXPECT_EQ(nullptr, scoper2.get());
EXPECT_EQ(nullptr, scoper3.get());
EXPECT_EQ(nullptr, scoper4.get());
}
scoped_ptr<int[]> NullIntArrayReturn() {
return nullptr;
}
TEST(ScopedPtrTest, NullptrArray) {
scoped_ptr<int[]> scoper1(nullptr);
scoped_ptr<int[]> scoper2(new int[3]);
scoper2 = nullptr;
scoped_ptr<int[]> scoper3(NullIntArrayReturn());
scoped_ptr<int[]> scoper4 = NullIntArrayReturn();
EXPECT_EQ(nullptr, scoper1.get());
EXPECT_EQ(nullptr, scoper2.get());
EXPECT_EQ(nullptr, scoper3.get());
EXPECT_EQ(nullptr, scoper4.get());
}
class Super {};
class Sub : public Super {};
scoped_ptr<Sub> SubClassReturn() {
return make_scoped_ptr(new Sub);
}
TEST(ScopedPtrTest, Conversion) {
scoped_ptr<Sub> sub1(new Sub);
scoped_ptr<Sub> sub2(new Sub);
// Upcast with Pass() works.
scoped_ptr<Super> super1 = sub1.Pass();
super1 = sub2.Pass();
// Upcast with an rvalue works.
scoped_ptr<Super> super2 = SubClassReturn();
super2 = SubClassReturn();
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment