Allow constructing {Interface,Struct}Ptr from nullptr
This allows spelling a null InterfacePtr or StructPtr parameter
'nullptr' instead of namespace::FooPtr(). To do so, this uses C++11
rvalue reference semantics for move support and adds constructor and
assignment operator overloads that take the type decltype(nullptr). We
still use a macro for move-only types to hide the copy and assignment
operators, add Pass(), and add the misleading named type tag
MoveOnlyForCPP03 for compatibility with Chromium's base/callback (and
anything else that may want to use it).
R=darin@chromium.org
Review URL: https://codereview.chromium.org/828373008
diff --git a/mojo/public/cpp/bindings/array.h b/mojo/public/cpp/bindings/array.h
index 2544abb..ca4e9cc 100644
--- a/mojo/public/cpp/bindings/array.h
+++ b/mojo/public/cpp/bindings/array.h
@@ -20,7 +20,7 @@
template <typename T>
class Array {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(Array, RValue)
+ MOJO_MOVE_ONLY_TYPE(Array)
public:
typedef internal::ArrayTraits<T, internal::IsMoveOnlyType<T>::value> Traits;
typedef typename Traits::ConstRefType ConstRefType;
@@ -37,9 +37,9 @@
}
~Array() { Traits::Finalize(&vec_); }
- Array(RValue other) : is_null_(true) { Take(other.object); }
- Array& operator=(RValue other) {
- Take(other.object);
+ Array(Array&& other) : is_null_(true) { Take(&other); }
+ Array& operator=(Array&& other) {
+ Take(&other);
return *this;
}
diff --git a/mojo/public/cpp/bindings/interface_ptr.h b/mojo/public/cpp/bindings/interface_ptr.h
index c56e770..a2ede4d 100644
--- a/mojo/public/cpp/bindings/interface_ptr.h
+++ b/mojo/public/cpp/bindings/interface_ptr.h
@@ -18,16 +18,22 @@
// InterfacePtr represents a proxy to a remote instance of an interface.
template <typename Interface>
class InterfacePtr {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(InterfacePtr, RValue)
+ MOJO_MOVE_ONLY_TYPE(InterfacePtr)
public:
InterfacePtr() {}
+ InterfacePtr(decltype(nullptr)) {}
- InterfacePtr(RValue other) {
- internal_state_.Swap(&other.object->internal_state_);
+ InterfacePtr(InterfacePtr&& other) {
+ internal_state_.Swap(&other.internal_state_);
}
- InterfacePtr& operator=(RValue other) {
+ InterfacePtr& operator=(InterfacePtr&& other) {
reset();
- internal_state_.Swap(&other.object->internal_state_);
+ internal_state_.Swap(&other.internal_state_);
+ return *this;
+ }
+
+ InterfacePtr& operator=(decltype(nullptr)) {
+ reset();
return *this;
}
diff --git a/mojo/public/cpp/bindings/interface_request.h b/mojo/public/cpp/bindings/interface_request.h
index 0a6dc2c..1723330 100644
--- a/mojo/public/cpp/bindings/interface_request.h
+++ b/mojo/public/cpp/bindings/interface_request.h
@@ -12,13 +12,13 @@
// Used in methods that return instances of remote objects.
template <typename Interface>
class InterfaceRequest {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(InterfaceRequest, RValue)
+ MOJO_MOVE_ONLY_TYPE(InterfaceRequest)
public:
InterfaceRequest() {}
- InterfaceRequest(RValue other) { handle_ = other.object->handle_.Pass(); }
- InterfaceRequest& operator=(RValue other) {
- handle_ = other.object->handle_.Pass();
+ InterfaceRequest(InterfaceRequest&& other) { handle_ = other.handle_.Pass(); }
+ InterfaceRequest& operator=(InterfaceRequest&& other) {
+ handle_ = other.handle_.Pass();
return *this;
}
diff --git a/mojo/public/cpp/bindings/lib/filter_chain.cc b/mojo/public/cpp/bindings/lib/filter_chain.cc
index 6634562..d32eb78 100644
--- a/mojo/public/cpp/bindings/lib/filter_chain.cc
+++ b/mojo/public/cpp/bindings/lib/filter_chain.cc
@@ -14,14 +14,14 @@
FilterChain::FilterChain(MessageReceiver* sink) : sink_(sink) {
}
-FilterChain::FilterChain(RValue other) : sink_(other.object->sink_) {
- other.object->sink_ = nullptr;
- filters_.swap(other.object->filters_);
+FilterChain::FilterChain(FilterChain&& other) : sink_(other.sink_) {
+ other.sink_ = nullptr;
+ filters_.swap(other.filters_);
}
-FilterChain& FilterChain::operator=(RValue other) {
- std::swap(sink_, other.object->sink_);
- filters_.swap(other.object->filters_);
+FilterChain& FilterChain::operator=(FilterChain&& other) {
+ std::swap(sink_, other.sink_);
+ filters_.swap(other.filters_);
return *this;
}
diff --git a/mojo/public/cpp/bindings/lib/filter_chain.h b/mojo/public/cpp/bindings/lib/filter_chain.h
index fc66642..bd7f9f5 100644
--- a/mojo/public/cpp/bindings/lib/filter_chain.h
+++ b/mojo/public/cpp/bindings/lib/filter_chain.h
@@ -15,17 +15,15 @@
namespace internal {
class FilterChain {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(FilterChain, RValue)
+ MOJO_MOVE_ONLY_TYPE(FilterChain)
public:
// Doesn't take ownership of |sink|. Therefore |sink| has to stay alive while
// this object is alive.
explicit FilterChain(MessageReceiver* sink = nullptr);
- // Move-only constructor and operator=.
- FilterChain(RValue other);
- FilterChain& operator=(RValue other);
-
+ FilterChain(FilterChain&& other);
+ FilterChain& operator=(FilterChain&& other);
~FilterChain();
template <typename FilterType>
diff --git a/mojo/public/cpp/bindings/map.h b/mojo/public/cpp/bindings/map.h
index 7aa7b6f..5149bb0 100644
--- a/mojo/public/cpp/bindings/map.h
+++ b/mojo/public/cpp/bindings/map.h
@@ -13,7 +13,8 @@
template <typename Key, typename Value>
class Map {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(Map, RValue)
+ MOJO_MOVE_ONLY_TYPE(Map)
+
public:
// Map keys can not be move only classes.
static_assert(!internal::IsMoveOnlyType<Key>::value,
@@ -45,9 +46,9 @@
~Map() { Traits::Finalize(&map_); }
- Map(RValue other) : is_null_(true) { Take(other.object); }
- Map& operator=(RValue other) {
- Take(other.object);
+ Map(Map&& other) : is_null_(true) { Take(&other); }
+ Map& operator=(Map&& other) {
+ Take(&other);
return *this;
}
diff --git a/mojo/public/cpp/bindings/struct_ptr.h b/mojo/public/cpp/bindings/struct_ptr.h
index efcf255..a420fa5 100644
--- a/mojo/public/cpp/bindings/struct_ptr.h
+++ b/mojo/public/cpp/bindings/struct_ptr.h
@@ -27,16 +27,23 @@
template <typename Struct>
class StructPtr {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(StructPtr, RValue);
+ MOJO_MOVE_ONLY_TYPE(StructPtr)
public:
StructPtr() : ptr_(nullptr) {}
+ StructPtr(decltype(nullptr)) : ptr_(nullptr) {}
+
~StructPtr() { delete ptr_; }
- StructPtr(RValue other) : ptr_(nullptr) { Take(other.object); }
- StructPtr& operator=(RValue other) {
- Take(other.object);
+ StructPtr& operator=(decltype(nullptr)) {
+ reset();
+ return *this;
+ }
+
+ StructPtr(StructPtr&& other) : ptr_(nullptr) { Take(&other); }
+ StructPtr& operator=(StructPtr&& other) {
+ Take(&other);
return *this;
}
@@ -101,16 +108,23 @@
// Designed to be used when Struct is small and copyable.
template <typename Struct>
class InlinedStructPtr {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(InlinedStructPtr, RValue);
+ MOJO_MOVE_ONLY_TYPE(InlinedStructPtr);
public:
InlinedStructPtr() : is_null_(true) {}
+ InlinedStructPtr(decltype(nullptr)) : is_null_(true) {}
+
~InlinedStructPtr() {}
- InlinedStructPtr(RValue other) : is_null_(true) { Take(other.object); }
- InlinedStructPtr& operator=(RValue other) {
- Take(other.object);
+ InlinedStructPtr& operator=(decltype(nullptr)) {
+ reset();
+ return *this;
+ }
+
+ InlinedStructPtr(InlinedStructPtr&& other) : is_null_(true) { Take(&other); }
+ InlinedStructPtr& operator=(InlinedStructPtr&& other) {
+ Take(&other);
return *this;
}
diff --git a/mojo/public/cpp/bindings/tests/container_test_util.cc b/mojo/public/cpp/bindings/tests/container_test_util.cc
index 12fdaba..e8377c4 100644
--- a/mojo/public/cpp/bindings/tests/container_test_util.cc
+++ b/mojo/public/cpp/bindings/tests/container_test_util.cc
@@ -32,14 +32,14 @@
num_instances_++;
}
-MoveOnlyType::MoveOnlyType(RValue other)
- : moved_(true), ptr_(other.object->ptr()) {
+MoveOnlyType::MoveOnlyType(MoveOnlyType&& other)
+ : moved_(true), ptr_(other.ptr()) {
num_instances_++;
}
-MoveOnlyType& MoveOnlyType::operator=(RValue other) {
+MoveOnlyType& MoveOnlyType::operator=(MoveOnlyType&& other) {
moved_ = true;
- ptr_ = other.object->ptr();
+ ptr_ = other.ptr();
return *this;
}
diff --git a/mojo/public/cpp/bindings/tests/container_test_util.h b/mojo/public/cpp/bindings/tests/container_test_util.h
index 1c3e5ee..1e29d22 100644
--- a/mojo/public/cpp/bindings/tests/container_test_util.h
+++ b/mojo/public/cpp/bindings/tests/container_test_util.h
@@ -28,12 +28,12 @@
};
class MoveOnlyType {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(MoveOnlyType, RValue)
+ MOJO_MOVE_ONLY_TYPE(MoveOnlyType)
public:
typedef MoveOnlyType Data_;
MoveOnlyType();
- MoveOnlyType(RValue other);
- MoveOnlyType& operator=(RValue other);
+ MoveOnlyType(MoveOnlyType&& other);
+ MoveOnlyType& operator=(MoveOnlyType&& other);
~MoveOnlyType();
bool moved() const { return moved_; }
diff --git a/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc b/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc
index 91ce84b..f90dc3d 100644
--- a/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/interface_ptr_unittest.cc
@@ -327,12 +327,8 @@
sample::ServicePtr proxy;
ReentrantServiceImpl* impl = BindToProxy(new ReentrantServiceImpl(), &proxy);
- proxy->Frobinate(sample::FooPtr(),
- sample::Service::BAZ_OPTIONS_REGULAR,
- sample::PortPtr());
- proxy->Frobinate(sample::FooPtr(),
- sample::Service::BAZ_OPTIONS_REGULAR,
- sample::PortPtr());
+ proxy->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr);
+ proxy->Frobinate(nullptr, sample::Service::BAZ_OPTIONS_REGULAR, nullptr);
PumpMessages();
diff --git a/mojo/public/cpp/bindings/tests/struct_unittest.cc b/mojo/public/cpp/bindings/tests/struct_unittest.cc
index 8f7580d..52d1313 100644
--- a/mojo/public/cpp/bindings/tests/struct_unittest.cc
+++ b/mojo/public/cpp/bindings/tests/struct_unittest.cc
@@ -43,11 +43,21 @@
EXPECT_TRUE(!rect);
EXPECT_FALSE(rect);
+ rect = nullptr;
+ EXPECT_TRUE(rect.is_null());
+ EXPECT_TRUE(!rect);
+ EXPECT_FALSE(rect);
+
rect = MakeRect();
EXPECT_FALSE(rect.is_null());
EXPECT_FALSE(!rect);
EXPECT_TRUE(rect);
+ RectPtr null_rect = nullptr;
+ EXPECT_TRUE(null_rect.is_null());
+ EXPECT_TRUE(!null_rect);
+ EXPECT_FALSE(null_rect);
+
CheckRect(*rect);
}
@@ -96,6 +106,20 @@
CheckRect(*rect2);
}
+// Construction of a struct with struct pointers from null.
+TEST_F(StructTest, Construction_StructPointers) {
+ RectPairPtr pair;
+ EXPECT_TRUE(pair.is_null());
+
+ pair = RectPair::New();
+ EXPECT_FALSE(pair.is_null());
+ EXPECT_TRUE(pair->first.is_null());
+ EXPECT_TRUE(pair->first.is_null());
+
+ pair = nullptr;
+ EXPECT_TRUE(pair.is_null());
+}
+
// Serialization test of a struct with struct pointers.
TEST_F(StructTest, Serialization_StructPointers) {
RectPairPtr pair(RectPair::New());
diff --git a/mojo/public/cpp/system/handle.h b/mojo/public/cpp/system/handle.h
index 211d895..0c5adc7 100644
--- a/mojo/public/cpp/system/handle.h
+++ b/mojo/public/cpp/system/handle.h
@@ -84,7 +84,7 @@
// like the C++11 |unique_ptr|.
template <class HandleType>
class ScopedHandleBase {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(ScopedHandleBase, RValue)
+ MOJO_MOVE_ONLY_TYPE(ScopedHandleBase)
public:
ScopedHandleBase() {}
@@ -96,11 +96,11 @@
: handle_(other.release()) {}
// Move-only constructor and operator=.
- ScopedHandleBase(RValue other) : handle_(other.object->release()) {}
- ScopedHandleBase& operator=(RValue other) {
- if (other.object != this) {
+ ScopedHandleBase(ScopedHandleBase&& other) : handle_(other.release()) {}
+ ScopedHandleBase& operator=(ScopedHandleBase&& other) {
+ if (&other != this) {
CloseIfNecessary();
- handle_ = other.object->release();
+ handle_ = other.release();
}
return *this;
}
diff --git a/mojo/public/cpp/system/macros.h b/mojo/public/cpp/system/macros.h
index 564b6be..f2bd0bc 100644
--- a/mojo/public/cpp/system/macros.h
+++ b/mojo/public/cpp/system/macros.h
@@ -29,21 +29,17 @@
} // namespace mojo
#define MOJO_ARRAYSIZE(array) (sizeof(::mojo::ArraySizeHelper(array)))
-// Used to make a type move-only in C++03. See Chromium's base/move.h for more
-// details.
-#define MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(type, rvalue_type) \
- private: \
- struct rvalue_type { \
- explicit rvalue_type(type* object) : object(object) {} \
- type* object; \
- }; \
- type(type&); \
- void operator=(type&); \
- \
- public: \
- operator rvalue_type() { return rvalue_type(this); } \
- type Pass() { return type(rvalue_type(this)); } \
- typedef void MoveOnlyTypeForCPP03; \
- \
+// Used to make a type move-only. See Chromium's base/move.h for more
+// details. The MoveOnlyTypeForCPP03 typedef is for Chromium's base/callback to
+// tell that this type is move-only.
+#define MOJO_MOVE_ONLY_TYPE(type) \
+ private: \
+ type(type&); \
+ void operator=(type&); \
+ \
+ public: \
+ type&& Pass() MOJO_WARN_UNUSED_RESULT { return static_cast<type&&>(*this); } \
+ typedef void MoveOnlyTypeForCPP03; \
+ \
private:
#endif // MOJO_PUBLIC_CPP_SYSTEM_MACROS_H_
diff --git a/mojo/public/cpp/system/tests/macros_unittest.cc b/mojo/public/cpp/system/tests/macros_unittest.cc
index 89dd764..27a61bd 100644
--- a/mojo/public/cpp/system/tests/macros_unittest.cc
+++ b/mojo/public/cpp/system/tests/macros_unittest.cc
@@ -76,7 +76,7 @@
// Note: MSVS is very strict (and arguably buggy) about warnings for classes
// defined in a local scope, so define these globally.
class MoveOnlyInt {
- MOJO_MOVE_ONLY_TYPE_FOR_CPP_03(MoveOnlyInt, RValue)
+ MOJO_MOVE_ONLY_TYPE(MoveOnlyInt)
public:
MoveOnlyInt() : is_set_(false), value_() {}
@@ -84,12 +84,12 @@
~MoveOnlyInt() {}
// Move-only constructor and operator=.
- MoveOnlyInt(RValue other) { *this = other; }
- MoveOnlyInt& operator=(RValue other) {
- if (other.object != this) {
- is_set_ = other.object->is_set_;
- value_ = other.object->value_;
- other.object->is_set_ = false;
+ MoveOnlyInt(MoveOnlyInt&& other) { *this = other.Pass(); }
+ MoveOnlyInt& operator=(MoveOnlyInt&& other) {
+ if (&other != this) {
+ is_set_ = other.is_set_;
+ value_ = other.value_;
+ other.is_set_ = false;
}
return *this;
}
