From 810999fadf41576204752113f33336a69603de4a Mon Sep 17 00:00:00 2001 From: Mikhail Pozdnyakov Date: Thu, 7 Feb 2019 12:13:19 +0200 Subject: [core] Simplify util::peer Remove custom vtable, base implementation on `std::unique_ptr`. --- include/mbgl/util/peer.hpp | 97 +++------------------------ platform/android/src/style/sources/source.cpp | 2 +- test/util/peer.test.cpp | 47 ++++++------- 3 files changed, 31 insertions(+), 115 deletions(-) diff --git a/include/mbgl/util/peer.hpp b/include/mbgl/util/peer.hpp index a4abea0e88..d56526b80d 100644 --- a/include/mbgl/util/peer.hpp +++ b/include/mbgl/util/peer.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -8,102 +9,22 @@ namespace util { class peer { public: - peer() = default; - peer(const peer&) = delete; - - peer(peer&& other) - : vtable(other.vtable) - { - if (vtable) { - vtable->move(other.storage, storage); - } - other.vtable = nullptr; - } + peer() noexcept : storage(nullptr, noop_deleter) {} template - peer(T&& value) { - using _Vt = std::decay_t; - vtable = get_vtable<_Vt>(); - new (&storage) _Vt(std::forward(value)); - } - - ~peer() { - reset(); - } - - peer& operator=(peer&& rhs) { - peer(std::move(rhs)).swap(*this); - return *this; - } - - void reset() { - if (vtable) { - vtable->destroy(storage); - vtable = nullptr; - } - } + peer(T&& value) noexcept : storage(new std::decay_t(std::forward(value)), cast_deleter>) {} - void swap(peer& rhs) { - if (this == &rhs) { - return; - } else { - peer tmp(std::move(rhs)); - rhs.vtable = vtable; - if (rhs.vtable) { - rhs.vtable->move(storage, rhs.storage); - } - vtable = tmp.vtable; - if (vtable) { - vtable->move(tmp.storage, storage); - } - } - } - - bool has_value() const { - return vtable != nullptr; - } - - template - T& get() { - return reinterpret_cast(storage); - } + bool has_value() const noexcept { return static_cast(storage); } template - T&& take() { - reset(); - return std::move(get()); - } + T& get() noexcept { return *reinterpret_cast(storage.get()); } private: - using storage_t = std::aligned_storage_t<2*sizeof(void*), alignof(void*)>; - - struct vtable { - virtual ~vtable() = default; - virtual void move(storage_t&, storage_t&) = 0; - virtual void destroy(storage_t&) = 0; - }; - - template - struct vtable_impl : public vtable { - static_assert(sizeof(T) <= sizeof(storage_t), "peer object is too big"); - - void move(storage_t& src, storage_t& dst) override { - new (&dst) T(std::move(reinterpret_cast(src))); - destroy(src); - } - - void destroy(storage_t& s) override { - reinterpret_cast(s).~T(); - } - }; - - template - static vtable* get_vtable() { - static vtable_impl vtable; - return &vtable; - } + template + static void cast_deleter(void* ptr) noexcept { delete reinterpret_cast(ptr); } + static void noop_deleter(void*) noexcept {} - vtable* vtable = nullptr; + using storage_t = std::unique_ptr; storage_t storage; }; diff --git a/platform/android/src/style/sources/source.cpp b/platform/android/src/style/sources/source.cpp index e13f55aff1..be4c0367fc 100644 --- a/platform/android/src/style/sources/source.cpp +++ b/platform/android/src/style/sources/source.cpp @@ -133,7 +133,7 @@ namespace android { // Release the peer relationships. These will be re-established when the source is added to a map assert(ownedSource->peer.has_value()); ownedSource->peer.get>().release(); - ownedSource->peer.reset(); + ownedSource->peer = util::peer(); // Release the strong reference to the java peer assert(javaPeer); diff --git a/test/util/peer.test.cpp b/test/util/peer.test.cpp index aa4dae5a88..aa5a4cc538 100644 --- a/test/util/peer.test.cpp +++ b/test/util/peer.test.cpp @@ -109,29 +109,25 @@ TEST(Peer, SmallType) { EXPECT_EQ(p, 0); } -// peer is not able to receive large types, unless we increase storage_t -// capacity. -//TEST(Peer, LargeType) { -// TestType t1; -// peer u1 = peer(std::move(t1)); -// EXPECT_TRUE(u1.has_value()); -// -// //TestType should be moved into owning peer -// EXPECT_EQ(u1.get().i1, 1); -// -// auto u2(std::move(u1)); -// EXPECT_FALSE(u1.has_value()); -// -// //TestType should not be moved when owning peer is moved; -// EXPECT_EQ(u2.get().i1, 1); -// -// //TestType should be moved out of owning peer -// // Note: two moves are involved in returning the moved value -// // First out of the peer, and then in the return statement -// auto t2 = std::move(u2.get()); -// EXPECT_FALSE(u2.has_value()); -// EXPECT_EQ(t2.i1, 3); -//} +TEST(Peer, LargeType) { + TestType t1; + peer u1 = peer(std::move(t1)); + EXPECT_TRUE(u1.has_value()); + + //TestType should be moved into owning peer + EXPECT_EQ(u1.get().i1, 1); + + auto u2(std::move(u1)); + EXPECT_FALSE(u1.has_value()); + + //TestType should not be moved when owning peer is moved; + EXPECT_EQ(u2.get().i1, 1); + + // TestType should not be moved out of owning peer + auto& t2 = u2.get(); + EXPECT_TRUE(u2.has_value()); + EXPECT_EQ(t2.i1, 1); +} TEST(Peer, Pointer) { auto t1 = new TestType(); @@ -163,9 +159,8 @@ TEST(Peer, UniquePtr) { EXPECT_EQ(t1.get(), nullptr); EXPECT_TRUE(u1.has_value()); - auto t2 = std::move(u1.take()); + u1 = peer(); EXPECT_FALSE(u1.has_value()); - (void)t2; peer u2; TestType * t3 = new TestType(); @@ -185,7 +180,7 @@ TEST(Peer, SharedPtr) { u1 = std::move(u2); EXPECT_EQ(weak.use_count(), 2); - u2.swap(u1); + std::swap(u2, u1); EXPECT_EQ(weak.use_count(), 2); u2 = 0; EXPECT_EQ(weak.use_count(), 1); -- cgit v1.2.1