From e3013cb0910805d87ba0b43eb7edeeb12684e1c9 Mon Sep 17 00:00:00 2001 From: Daniel Gryniewicz Date: Thu, 24 Sep 2020 12:45:31 -0400 Subject: [PATCH] Remove copy from ceph::static_ptr The copy functionality of ceph::static_ptr is unused, and it means that nothing containing a std::unique_ptr (or any other non-copyable type) can be put into a static_ptr. Remove the copy functionality, so that unique_ptr can be used in static_ptr. Signed-off-by: Daniel Gryniewicz --- src/common/static_ptr.h | 115 +++-------------------------- src/test/common/test_static_ptr.cc | 49 +----------- 2 files changed, 12 insertions(+), 152 deletions(-) diff --git a/src/common/static_ptr.h b/src/common/static_ptr.h index 63fce43424013..542f1e9a67a42 100644 --- a/src/common/static_ptr.h +++ b/src/common/static_ptr.h @@ -33,7 +33,7 @@ namespace _mem { // of the same arguments (which is not true for function type erasure) // it's a pretty good one. enum class op { - copy, move, destroy, size + move, destroy, size }; template static std::size_t op_fun(op oper, void* p1, void* p2) @@ -41,15 +41,6 @@ static std::size_t op_fun(op oper, void* p1, void* p2) auto me = static_cast(p1); switch (oper) { - case op::copy: - // One conspicuous downside is that immovable/uncopyable functions - // kill compilation right here, even if nobody ever calls the move - // or copy methods. Working around this is a pain, since we'd need - // four operator functions and a top-level class to - // provide/withhold copy/move operations as appropriate. - new (p2) T(*me); - break; - case op::move: new (p2) T(std::move(*me)); break; @@ -137,12 +128,6 @@ public: // Set from another static pointer. // // Since the templated versions don't count for overriding the defaults - static_ptr(const static_ptr& rhs) - noexcept(std::is_nothrow_copy_constructible_v) : operate(rhs.operate) { - if (operate) { - operate(_mem::op::copy, &rhs.buf, &buf); - } - } static_ptr(static_ptr&& rhs) noexcept(std::is_nothrow_move_constructible_v) : operate(rhs.operate) { if (operate) { @@ -150,14 +135,6 @@ public: } } - template - static_ptr(const static_ptr& rhs) - noexcept(std::is_nothrow_copy_constructible_v) : operate(rhs.operate) { - create_ward(); - if (operate) { - operate(_mem::op::copy, &rhs.buf, &buf); - } - } template static_ptr(static_ptr&& rhs) noexcept(std::is_nothrow_move_constructible_v) : operate(rhs.operate) { @@ -167,16 +144,6 @@ public: } } - static_ptr& operator =(const static_ptr& rhs) - noexcept(std::is_nothrow_copy_constructible_v) { - reset(); - if (rhs) { - operate = rhs.operate; - operate(_mem::op::copy, - const_cast(static_cast(&rhs.buf)), &buf); - } - return *this; - } static_ptr& operator =(static_ptr&& rhs) noexcept(std::is_nothrow_move_constructible_v) { reset(); @@ -187,18 +154,6 @@ public: return *this; } - template - static_ptr& operator =(const static_ptr& rhs) - noexcept(std::is_nothrow_copy_constructible_v) { - create_ward(); - reset(); - if (rhs) { - operate = rhs.operate; - operate(_mem::op::copy, - const_cast(static_cast(&rhs.buf)), &buf); - } - return *this; - } template static_ptr& operator =(static_ptr&& rhs) noexcept(std::is_nothrow_move_constructible_v) { @@ -300,20 +255,6 @@ public: // nice idiom. Having to release and reconstruct is obnoxious. // template -static_ptr static_pointer_cast(const static_ptr& p) { - static_assert(Z >= S, - "Value too large."); - static_ptr r; - // Really, this is always true because static_cast either succeeds - // or fails to compile, but it prevents an unused variable warning - // and should be optimized out. - if (static_cast(p.get())) { - p.operate(_mem::op::copy, &p.buf, &r.buf); - r.operate = p.operate; - } - return r; -} -template static_ptr static_pointer_cast(static_ptr&& p) { static_assert(Z >= S, "Value too large."); @@ -329,17 +270,6 @@ static_ptr static_pointer_cast(static_ptr&& p) { // same behavior as dynamic_cast. // template -static_ptr dynamic_pointer_cast(const static_ptr& p) { - static_assert(Z >= S, - "Value too large."); - static_ptr r; - if (dynamic_cast(p.get())) { - p.operate(_mem::op::copy, &p.buf, &r.buf); - r.operate = p.operate; - } - return r; -} -template static_ptr dynamic_pointer_cast(static_ptr&& p) { static_assert(Z >= S, "Value too large."); @@ -351,17 +281,6 @@ static_ptr dynamic_pointer_cast(static_ptr&& p) { return r; } -template -static_ptr const_pointer_cast(const static_ptr& p) { - static_assert(Z >= S, - "Value too large."); - static_ptr r; - if (const_cast(p.get())) { - p.operate(_mem::op::copy, &p.buf, &r.buf); - r.operate = p.operate; - } - return r; -} template static_ptr const_pointer_cast(static_ptr&& p) { static_assert(Z >= S, @@ -378,15 +297,6 @@ static_ptr const_pointer_cast(static_ptr&& p) { // where they might. It works, though! // template -static_ptr reinterpret_pointer_cast(const static_ptr& p) { - static_assert(Z >= S, - "Value too large."); - static_ptr r; - p.operate(_mem::op::copy, &p.buf, &r.buf); - r.operate = p.operate; - return r; -} -template static_ptr reinterpret_pointer_cast(static_ptr&& p) { static_assert(Z >= S, "Value too large."); @@ -404,17 +314,6 @@ static_ptr reinterpret_pointer_cast(static_ptr&& p) { // I follow cast semantics. Since this is a pointer-like type, it // returns a null value rather than throwing. template -static_ptr resize_pointer_cast(const static_ptr& p) { - static_assert(std::is_same_v, - "resize_pointer_cast only changes size, not type."); - static_ptr r; - if (Z >= p.operate(_mem::op::size, &p.buf, nullptr)) { - p.operate(_mem::op::copy, &p.buf, &r.buf); - r.operate = p.operate; - } - return r; -} -template static_ptr resize_pointer_cast(static_ptr&& p) { static_assert(std::is_same_v, "resize_pointer_cast only changes size, not type."); @@ -427,11 +326,19 @@ static_ptr resize_pointer_cast(static_ptr&& p) { } template -bool operator ==(static_ptr s, std::nullptr_t) { +bool operator ==(const static_ptr& s, std::nullptr_t) { + return !s; +} +template +bool operator ==(std::nullptr_t, const static_ptr& s) { + return !s; +} +template +bool operator ==(static_ptr& s, std::nullptr_t) { return !s; } template -bool operator ==(std::nullptr_t, static_ptr s) { +bool operator ==(std::nullptr_t, static_ptr& s) { return !s; } diff --git a/src/test/common/test_static_ptr.cc b/src/test/common/test_static_ptr.cc index 4bfc77bb278d6..f1c07c81b45fa 100644 --- a/src/test/common/test_static_ptr.cc +++ b/src/test/common/test_static_ptr.cc @@ -108,7 +108,7 @@ TEST(StaticPtr, CreateEmplace) { EXPECT_EQ(p->func(), 9); } -TEST(StaticPtr, CopyMove) { +TEST(StaticPtr, Move) { // Won't compile. Good. // static_ptr p1(std::in_place_type_t{}, 3); @@ -116,11 +116,6 @@ TEST(StaticPtr, CopyMove) { static_ptr p2(std::in_place_type_t{}, 3); - // This also does not compile. Good. - // p1 = p2; - p2 = p1; - EXPECT_EQ(p1->func(), 0); - p2 = std::move(p1); EXPECT_EQ(p1->func(), 0); } @@ -129,9 +124,6 @@ TEST(StaticPtr, ImplicitUpcast) { static_ptr p1; static_ptr p2(std::in_place_type_t{}, 3); - p1 = p2; - EXPECT_EQ(p1->func(), 9); - p1 = std::move(p2); EXPECT_EQ(p1->func(), 9); @@ -145,10 +137,6 @@ TEST(StaticPtr, StaticCast) { static_ptr p1(std::in_place_type_t{}, 3); static_ptr p2; - p2 = ceph::static_pointer_cast(p1); - EXPECT_EQ(p2->func(), 9); - EXPECT_EQ(p2->call(10), 30); - p2 = ceph::static_pointer_cast(std::move(p1)); EXPECT_EQ(p2->func(), 9); EXPECT_EQ(p2->call(10), 30); @@ -156,24 +144,12 @@ TEST(StaticPtr, StaticCast) { TEST(StaticPtr, DynamicCast) { static constexpr auto sz = sizeof(great_grandchild); - { - static_ptr p1(std::in_place_type_t{}, 3); - auto p2 = ceph::dynamic_pointer_cast(p1); - EXPECT_FALSE(p2); - } { static_ptr p1(std::in_place_type_t{}, 3); auto p2 = ceph::dynamic_pointer_cast(std::move(p1)); EXPECT_FALSE(p2); } - { - static_ptr p1(std::in_place_type_t{}, 3); - auto p2 = ceph::dynamic_pointer_cast(p1); - EXPECT_TRUE(p2); - EXPECT_EQ(p2->func(), 9); - EXPECT_EQ(p2->call(10), 30); - } { static_ptr p1(std::in_place_type_t{}, 3); auto p2 = ceph::dynamic_pointer_cast(std::move(p1)); @@ -195,17 +171,6 @@ public: TEST(StaticPtr, ConstCast) { static constexpr auto sz = sizeof(constable); - { - auto p1 = make_static(); - static_assert(std::is_const{}, - "Things are not as const as they ought to be."); - EXPECT_EQ(p1->foo(), 5); - auto p2 = ceph::const_pointer_cast(p1); - static_assert(!std::is_const{}, - "Things are more const than they ought to be."); - EXPECT_TRUE(p2); - EXPECT_EQ(p2->foo(), 2); - } { auto p1 = make_static(); EXPECT_EQ(p1->foo(), 5); @@ -219,17 +184,6 @@ TEST(StaticPtr, ConstCast) { TEST(StaticPtr, ReinterpretCast) { static constexpr auto sz = sizeof(grandchild); - { - auto p1 = make_static(3); - auto p2 = ceph::reinterpret_pointer_cast(p1); - static_assert(std::is_same{}, - "Reinterpret is screwy."); - auto p3 = ceph::reinterpret_pointer_cast(p2); - static_assert(std::is_same{}, - "Reinterpret is screwy."); - EXPECT_EQ(p3->func(), 9); - EXPECT_EQ(p3->call(10), 30); - } { auto p1 = make_static(3); auto p2 = ceph::reinterpret_pointer_cast(std::move(p1)); @@ -255,6 +209,5 @@ struct exceptional { TEST(StaticPtr, Exceptional) { static_ptr p1(std::in_place_type_t{}); - EXPECT_ANY_THROW(static_ptr p2(p1)); EXPECT_ANY_THROW(static_ptr p2(std::move(p1))); } -- 2.39.5