From df429882af5dc876432b4a3d4b67353be0492745 Mon Sep 17 00:00:00 2001 From: "andrew@webrtc.org" Date: Mon, 27 Oct 2014 16:12:38 +0000 Subject: [PATCH] Upgrade our scoped_ptr copy to match Chromium's latest. In particular add the move constructor and assignment operator. Diff between our version and Chromium's: https://paste.googleplex.com/4887047529562112 R=henrike@webrtc.org, kjellander@webrtc.org Review URL: https://webrtc-codereview.appspot.com/31789004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7531 4adac7df-926f-26a2-2b94-8c16560cd09d --- webrtc/system_wrappers/interface/scoped_ptr.h | 164 +++++++++++------- .../system_wrappers/interface/scoped_vector.h | 7 +- webrtc/system_wrappers/source/move.h | 21 ++- 3 files changed, 116 insertions(+), 76 deletions(-) diff --git a/webrtc/system_wrappers/interface/scoped_ptr.h b/webrtc/system_wrappers/interface/scoped_ptr.h index 42bb8a6dd..a9ddc5c11 100644 --- a/webrtc/system_wrappers/interface/scoped_ptr.h +++ b/webrtc/system_wrappers/interface/scoped_ptr.h @@ -10,10 +10,10 @@ // Borrowed from Chromium's src/base/memory/scoped_ptr.h. -// Scopers help you manage ownership of a pointer, helping you easily manage the -// 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[]. +// 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): // { @@ -66,7 +66,7 @@ // TakesOwnership(ptr.Pass()); // ptr no longer owns Foo("yay"). // scoped_ptr ptr2 = CreateFoo(); // ptr2 owns the return Foo. // scoped_ptr 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 @@ -180,12 +180,23 @@ struct FreeDeleter { namespace internal { +template +struct ShouldAbortOnSelfReset { + template + static NoType Test(const typename U::AllowSelfReset*); + + template + static YesType Test(...); + + static const bool value = sizeof(Test(0)) == sizeof(YesType); +}; + // Minimal implementation of the core logic of scoped_ptr, suitable for // reuse in both scoped_ptr and its specializations. template class scoped_ptr_impl { public: - explicit scoped_ptr_impl(T* p) : data_(p) { } + 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) {} @@ -210,7 +221,7 @@ class 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 // builds. static_cast(data_)(data_.ptr); @@ -218,12 +229,12 @@ class scoped_ptr_impl { } void reset(T* p) { - // This is a self-reset, which is no longer allowed: http://crbug.com/162971 - if (p != NULL && p == data_.ptr) - abort(); + // This is a self-reset, which is no longer allowed for default deleters: + // https://crbug.com/162971 + assert(!ShouldAbortOnSelfReset::value || p == nullptr || p != data_.ptr); // Note that running data_.ptr = p can lead to undefined behavior if - // get_deleter()(get()) deletes this. In order to pevent this, reset() + // get_deleter()(get()) deletes this. In order to prevent this, reset() // should update the stored pointer before deleting its old value. // // However, changing reset() to use that behavior may cause current code to @@ -232,13 +243,13 @@ class scoped_ptr_impl { // then it will incorrectly dispatch calls to |p| rather than the original // 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 - // prevent the scenario initially described from occuring and + // prevent the scenario initially described from occurring and // http://crbug.com/176091 can be closed. T* old = data_.ptr; - data_.ptr = NULL; - if (old != NULL) + data_.ptr = nullptr; + if (old != nullptr) static_cast(data_)(old); data_.ptr = p; } @@ -259,7 +270,7 @@ class scoped_ptr_impl { T* release() { T* old_ptr = data_.ptr; - data_.ptr = NULL; + data_.ptr = nullptr; return old_ptr; } @@ -287,8 +298,8 @@ class scoped_ptr_impl { // A scoped_ptr is like a T*, except that the destructor of scoped_ptr // automatically deletes the pointer it holds (if any). // That is, scoped_ptr owns the T object that it points to. -// Like a T*, a scoped_ptr may hold either NULL or a pointer to a T object. -// Also like T*, scoped_ptr is thread-compatible, and once you +// Like a T*, a scoped_ptr may hold either nullptr or a pointer to a T +// object. Also like T*, scoped_ptr 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 @@ -298,25 +309,33 @@ class scoped_ptr_impl { // // 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 +// deleters, function pointers as deleters, and deleters with reference // types. template > class scoped_ptr { - WEBRTC_MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr, RValue) + RTC_MOVE_ONLY_TYPE_WITH_MOVE_CONSTRUCTOR_FOR_CPP_03(scoped_ptr) + + // TODO(ajm): If we ever import RefCountedBase, this check needs to be + // enabled. + //COMPILE_ASSERT(webrtc::internal::IsNotRefCounted::value, + // T_is_refcounted_type_and_needs_scoped_refptr); 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. Defaults to initializing with nullptr. + scoped_ptr() : impl_(nullptr) {} // 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. - 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 // convertible type and deleter. @@ -329,13 +348,11 @@ class scoped_ptr { // use of SFINAE. You only need to care about this if you modify the // implementation of scoped_ptr. template - scoped_ptr(scoped_ptr other) : impl_(&other.impl_) { + scoped_ptr(scoped_ptr&& other) + : impl_(&other.impl_) { COMPILE_ASSERT(!webrtc::is_array::value, U_cannot_be_an_array); } - // Constructor. Move constructor for C++03 move emulation of this type. - scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { } - // operator=. Allows assignment from a scoped_ptr rvalue for a convertible // type and deleter. // @@ -347,24 +364,31 @@ class scoped_ptr { // You only need to care about this if you modify the implementation of // scoped_ptr. template - scoped_ptr& operator=(scoped_ptr rhs) { + scoped_ptr& operator=(scoped_ptr&& rhs) { COMPILE_ASSERT(!webrtc::is_array::value, U_cannot_be_an_array); impl_.TakeState(&rhs.impl_); 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. // 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. // operator* and operator-> will assert() if there is no current object. element_type& operator*() const { - assert(impl_.get() != NULL); + assert(impl_.get() != nullptr); return *impl_.get(); } element_type* operator->() const { - assert(impl_.get() != NULL); + assert(impl_.get() != nullptr); return impl_.get(); } element_type* get() const { return impl_.get(); } @@ -385,7 +409,9 @@ class scoped_ptr { scoped_ptr::*Testable; public: - operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; } + operator Testable() const { + return impl_.get() ? &scoped_ptr::impl_ : nullptr; + } // Comparison operators. // These return whether two scoped_ptr refer to the same object, not just to @@ -399,25 +425,13 @@ class scoped_ptr { } // 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. + // The return value is the current pointer held by this object. If this object + // holds a nullptr, the return value is nullptr. After this operation, this + // object will hold a nullptr, and will not own the object any more. 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 - scoped_ptr PassAs() { - return scoped_ptr(Pass()); - } - private: // Needed to reach into |impl_| in the constructor. template friend class scoped_ptr; @@ -436,15 +450,15 @@ class scoped_ptr { template class scoped_ptr { - WEBRTC_MOVE_ONLY_TYPE_FOR_CPP_03(scoped_ptr, RValue) + RTC_MOVE_ONLY_TYPE_WITH_MOVE_CONSTRUCTOR_FOR_CPP_03(scoped_ptr) 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. Defaults to initializing with nullptr. + scoped_ptr() : impl_(nullptr) {} // Constructor. Stores the given array. Note that the argument's type // must exactly match T*. In particular: @@ -454,32 +468,39 @@ class scoped_ptr { // 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(). // However, because of the first bullet in this comment, users MUST // NOT use implicit_cast() 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. Move constructor for C++03 move emulation of this type. - scoped_ptr(RValue rvalue) : impl_(&rvalue.object->impl_) { } + // Constructor. Allows construction from a nullptr. + scoped_ptr(decltype(nullptr)) : impl_(nullptr) {} - // operator=. Move operator= for C++03 move emulation of this type. - scoped_ptr& operator=(RValue rhs) { - impl_.TakeState(&rhs.object->impl_); + // Constructor. Allows construction from a scoped_ptr rvalue. + scoped_ptr(scoped_ptr&& other) : impl_(&other.impl_) {} + + // operator=. Allows assignment from a scoped_ptr rvalue. + scoped_ptr& operator=(scoped_ptr&& rhs) { + impl_.TakeState(&rhs.impl_); + 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. // 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. element_type& operator[](size_t i) const { - assert(impl_.get() != NULL); + assert(impl_.get() != nullptr); return impl_.get()[i]; } element_type* get() const { return impl_.get(); } @@ -495,7 +516,9 @@ class scoped_ptr { scoped_ptr::*Testable; public: - operator Testable() const { return impl_.get() ? &scoped_ptr::impl_ : NULL; } + operator Testable() const { + return impl_.get() ? &scoped_ptr::impl_ : nullptr; + } // Comparison operators. // These return whether two scoped_ptr refer to the same object, not just to @@ -509,10 +532,9 @@ class scoped_ptr { } // 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. + // The return value is the current pointer held by this object. If this object + // holds a nullptr, the return value is nullptr. After this operation, this + // object will hold a nullptr, and will not own the object any more. element_type* release() WARN_UNUSED_RESULT { return impl_.release(); } @@ -563,4 +585,12 @@ bool operator!=(T* p1, const webrtc::scoped_ptr& p2) { return p1 != p2.get(); } +// A function to convert T* into scoped_ptr +// Doing e.g. make_scoped_ptr(new FooBarBaz(arg)) is a shorter notation +// for scoped_ptr >(new FooBarBaz(arg)) +template +webrtc::scoped_ptr make_scoped_ptr(T* ptr) { + return webrtc::scoped_ptr(ptr); +} + #endif // WEBRTC_SYSTEM_WRAPPERS_INTERFACE_SCOPED_PTR_H_ diff --git a/webrtc/system_wrappers/interface/scoped_vector.h b/webrtc/system_wrappers/interface/scoped_vector.h index 68db3a121..7b8c67880 100644 --- a/webrtc/system_wrappers/interface/scoped_vector.h +++ b/webrtc/system_wrappers/interface/scoped_vector.h @@ -13,10 +13,9 @@ #ifndef WEBRTC_SYSTEM_WRAPPERS_INTERFACE_SCOPED_VECTOR_H_ #define WEBRTC_SYSTEM_WRAPPERS_INTERFACE_SCOPED_VECTOR_H_ -#include -#include #include +#include "webrtc/base/checks.h" #include "webrtc/system_wrappers/interface/stl_util.h" #include "webrtc/system_wrappers/source/move.h" @@ -26,7 +25,7 @@ namespace webrtc { // destructor. template class ScopedVector { - WEBRTC_MOVE_ONLY_TYPE_FOR_CPP_03(ScopedVector, RValue) + RTC_MOVE_ONLY_TYPE_FOR_CPP_03(ScopedVector, RValue) public: typedef typename std::vector::allocator_type allocator_type; @@ -76,7 +75,7 @@ class ScopedVector { void push_back(T* elem) { v_.push_back(elem); } void pop_back() { - assert(!empty()); + DCHECK(!empty()); delete v_.back(); v_.pop_back(); } diff --git a/webrtc/system_wrappers/source/move.h b/webrtc/system_wrappers/source/move.h index 2e93641f4..80aed9a9e 100644 --- a/webrtc/system_wrappers/source/move.h +++ b/webrtc/system_wrappers/source/move.h @@ -10,8 +10,10 @@ // Borrowed from Chromium's src/base/move.h. -#ifndef WEBRTC_SYSTEM_WRAPPERS_INTEFACE_MOVE_H_ -#define WEBRTC_SYSTEM_WRAPPERS_INTEFACE_MOVE_H_ +#ifndef WEBRTC_SYSTEM_WRAPPERS_SOURCE_MOVE_H_ +#define WEBRTC_SYSTEM_WRAPPERS_SOURCE_MOVE_H_ + +#include "webrtc/typedefs.h" // Macro with the boilerplate that makes a type move-only in C++03. // @@ -209,7 +211,7 @@ // // The workaround is to explicitly declare your copy constructor. // -#define WEBRTC_MOVE_ONLY_TYPE_FOR_CPP_03(type, rvalue_type) \ +#define RTC_MOVE_ONLY_TYPE_FOR_CPP_03(type, rvalue_type) \ private: \ struct rvalue_type { \ explicit rvalue_type(type* object) : object(object) {} \ @@ -219,8 +221,17 @@ void operator=(type&); \ public: \ operator rvalue_type() { return rvalue_type(this); } \ - type Pass() { return type(rvalue_type(this)); } \ + type Pass() WARN_UNUSED_RESULT { return type(rvalue_type(this)); } \ typedef void MoveOnlyTypeForCPP03; \ private: -#endif // WEBRTC_SYSTEM_WRAPPERS_INTEFACE_MOVE_H_ +#define RTC_MOVE_ONLY_TYPE_WITH_MOVE_CONSTRUCTOR_FOR_CPP_03(type) \ + private: \ + type(type&); \ + void operator=(type&); \ + public: \ + type&& Pass() WARN_UNUSED_RESULT { return static_cast(*this); } \ + typedef void MoveOnlyTypeForCPP03; \ + private: + +#endif // WEBRTC_SYSTEM_WRAPPERS_SOURCE_MOVE_H_