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 <dang@redhat.com>

diff --git a/src/common/static_ptr.h b/src/common/static_ptr.h
index 63fce43424013918a1feb1582d9bb1b5e0680775..542f1e9a67a4208b4dd39fd60409044608df445e 100644 (file)
--- 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<typename T>
 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<T*>(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<Base>) : operate(rhs.operate) {
-    if (operate) {
-      operate(_mem::op::copy, &rhs.buf, &buf);
-    }
-  }
   static_ptr(static_ptr&& rhs)
     noexcept(std::is_nothrow_move_constructible_v<Base>) : operate(rhs.operate) {
     if (operate) {
@@ -150,14 +135,6 @@ public:
     }
   }
 
-  template<typename U, std::size_t S>
-  static_ptr(const static_ptr<U, S>& rhs)
-    noexcept(std::is_nothrow_copy_constructible_v<U>) : operate(rhs.operate) {
-    create_ward<U, S>();
-    if (operate) {
-      operate(_mem::op::copy, &rhs.buf, &buf);
-    }
-  }
   template<typename U, std::size_t S>
   static_ptr(static_ptr<U, S>&& rhs)
     noexcept(std::is_nothrow_move_constructible_v<U>) : operate(rhs.operate) {
@@ -167,16 +144,6 @@ public:
     }
   }
 
-  static_ptr& operator =(const static_ptr& rhs)
-    noexcept(std::is_nothrow_copy_constructible_v<Base>) {
-    reset();
-    if (rhs) {
-      operate = rhs.operate;
-      operate(_mem::op::copy,
-             const_cast<void*>(static_cast<const void*>(&rhs.buf)), &buf);
-    }
-    return *this;
-  }
   static_ptr& operator =(static_ptr&& rhs)
     noexcept(std::is_nothrow_move_constructible_v<Base>) {
     reset();
@@ -187,18 +154,6 @@ public:
     return *this;
   }
 
-  template<typename U, std::size_t S>
-  static_ptr& operator =(const static_ptr<U, S>& rhs)
-    noexcept(std::is_nothrow_copy_constructible_v<U>) {
-    create_ward<U, S>();
-    reset();
-    if (rhs) {
-      operate = rhs.operate;
-      operate(_mem::op::copy,
-             const_cast<void*>(static_cast<const void*>(&rhs.buf)), &buf);
-    }
-    return *this;
-  }
   template<typename U, std::size_t S>
   static_ptr& operator =(static_ptr<U, S>&& rhs)
     noexcept(std::is_nothrow_move_constructible_v<U>) {
@@ -300,20 +255,6 @@ public:
 // nice idiom. Having to release and reconstruct is obnoxious.
 //
 template<typename U, std::size_t Z, typename T, std::size_t S>
-static_ptr<U, Z> static_pointer_cast(const static_ptr<T, S>& p) {
-  static_assert(Z >= S,
-                "Value too large.");
-  static_ptr<U, Z> 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<U*>(p.get())) {
-    p.operate(_mem::op::copy, &p.buf, &r.buf);
-    r.operate = p.operate;
-  }
-  return r;
-}
-template<typename U, std::size_t Z, typename T, std::size_t S>
 static_ptr<U, Z> static_pointer_cast(static_ptr<T, S>&& p) {
   static_assert(Z >= S,
                 "Value too large.");
@@ -329,17 +270,6 @@ static_ptr<U, Z> static_pointer_cast(static_ptr<T, S>&& p) {
 // same behavior as dynamic_cast.
 //
 template<typename U, std::size_t Z, typename T, std::size_t S>
-static_ptr<U, Z> dynamic_pointer_cast(const static_ptr<T, S>& p) {
-  static_assert(Z >= S,
-                "Value too large.");
-  static_ptr<U, Z> r;
-  if (dynamic_cast<U*>(p.get())) {
-    p.operate(_mem::op::copy, &p.buf, &r.buf);
-    r.operate = p.operate;
-  }
-  return r;
-}
-template<typename U, std::size_t Z, typename T, std::size_t S>
 static_ptr<U, Z> dynamic_pointer_cast(static_ptr<T, S>&& p) {
   static_assert(Z >= S,
                 "Value too large.");
@@ -351,17 +281,6 @@ static_ptr<U, Z> dynamic_pointer_cast(static_ptr<T, S>&& p) {
   return r;
 }
 
-template<typename U, std::size_t Z, typename T, std::size_t S>
-static_ptr<U, Z> const_pointer_cast(const static_ptr<T, S>& p) {
-  static_assert(Z >= S,
-                "Value too large.");
-  static_ptr<U, Z> r;
-  if (const_cast<U*>(p.get())) {
-    p.operate(_mem::op::copy, &p.buf, &r.buf);
-    r.operate = p.operate;
-  }
-  return r;
-}
 template<typename U, std::size_t Z, typename T, std::size_t S>
 static_ptr<U, Z> const_pointer_cast(static_ptr<T, S>&& p) {
   static_assert(Z >= S,
@@ -378,15 +297,6 @@ static_ptr<U, Z> const_pointer_cast(static_ptr<T, S>&& p) {
 // where they might. It works, though!
 //
 template<typename U, std::size_t Z, typename T, std::size_t S>
-static_ptr<U, Z> reinterpret_pointer_cast(const static_ptr<T, S>& p) {
-  static_assert(Z >= S,
-                "Value too large.");
-  static_ptr<U, Z> r;
-  p.operate(_mem::op::copy, &p.buf, &r.buf);
-  r.operate = p.operate;
-  return r;
-}
-template<typename U, std::size_t Z, typename T, std::size_t S>
 static_ptr<U, Z> reinterpret_pointer_cast(static_ptr<T, S>&& p) {
   static_assert(Z >= S,
                 "Value too large.");
@@ -404,17 +314,6 @@ static_ptr<U, Z> reinterpret_pointer_cast(static_ptr<T, S>&& p) {
 // I follow cast semantics. Since this is a pointer-like type, it
 // returns a null value rather than throwing.
 template<typename U, std::size_t Z, typename T, std::size_t S>
-static_ptr<U, Z> resize_pointer_cast(const static_ptr<T, S>& p) {
-  static_assert(std::is_same_v<U, T>,
-                "resize_pointer_cast only changes size, not type.");
-  static_ptr<U, Z> 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<typename U, std::size_t Z, typename T, std::size_t S>
 static_ptr<U, Z> resize_pointer_cast(static_ptr<T, S>&& p) {
   static_assert(std::is_same_v<U, T>,
                 "resize_pointer_cast only changes size, not type.");
@@ -427,11 +326,19 @@ static_ptr<U, Z> resize_pointer_cast(static_ptr<T, S>&& p) {
 }
 
 template<typename Base, std::size_t Size>
-bool operator ==(static_ptr<Base, Size> s, std::nullptr_t) {
+bool operator ==(const static_ptr<Base, Size>& s, std::nullptr_t) {
+  return !s;
+}
+template<typename Base, std::size_t Size>
+bool operator ==(std::nullptr_t, const static_ptr<Base, Size>& s) {
+  return !s;
+}
+template<typename Base, std::size_t Size>
+bool operator ==(static_ptr<Base, Size>& s, std::nullptr_t) {
   return !s;
 }
 template<typename Base, std::size_t Size>
-bool operator ==(std::nullptr_t, static_ptr<Base, Size> s) {
+bool operator ==(std::nullptr_t, static_ptr<Base, Size>& s) {
   return !s;
 }
 

diff --git a/src/test/common/test_static_ptr.cc b/src/test/common/test_static_ptr.cc
index 4bfc77bb278d63c56955b49ec5d4a37112558055..f1c07c81b45fa12710911aa73f007dda8f7bcecb 100644 (file)
--- 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<base, sizeof(base)> p1(std::in_place_type_t<grandchild>{}, 3);
 
@@ -116,11 +116,6 @@ TEST(StaticPtr, CopyMove) {
   static_ptr<base, sizeof(grandchild)> p2(std::in_place_type_t<grandchild>{},
                                           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<base, sizeof(grandchild)> p1;
   static_ptr<sibling2, sizeof(grandchild)> p2(std::in_place_type_t<grandchild>{}, 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<base, sizeof(grandchild)> p1(std::in_place_type_t<grandchild>{}, 3);
   static_ptr<sibling2, sizeof(grandchild)> p2;
 
-  p2 = ceph::static_pointer_cast<sibling2, sizeof(grandchild)>(p1);
-  EXPECT_EQ(p2->func(), 9);
-  EXPECT_EQ(p2->call(10), 30);
-
   p2 = ceph::static_pointer_cast<sibling2, sizeof(grandchild)>(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<base, sz> p1(std::in_place_type_t<grandchild>{}, 3);
-    auto p2 = ceph::dynamic_pointer_cast<great_grandchild, sz>(p1);
-    EXPECT_FALSE(p2);
-  }
   {
     static_ptr<base, sz> p1(std::in_place_type_t<grandchild>{}, 3);
     auto p2 = ceph::dynamic_pointer_cast<great_grandchild, sz>(std::move(p1));
     EXPECT_FALSE(p2);
   }
 
-  {
-    static_ptr<base, sz> p1(std::in_place_type_t<grandchild>{}, 3);
-    auto p2 = ceph::dynamic_pointer_cast<grandchild, sz>(p1);
-    EXPECT_TRUE(p2);
-    EXPECT_EQ(p2->func(), 9);
-    EXPECT_EQ(p2->call(10), 30);
-  }
   {
     static_ptr<base, sz> p1(std::in_place_type_t<grandchild>{}, 3);
     auto p2 = ceph::dynamic_pointer_cast<grandchild, sz>(std::move(p1));
@@ -195,17 +171,6 @@ public:
 
 TEST(StaticPtr, ConstCast) {
   static constexpr auto sz = sizeof(constable);
-  {
-    auto p1 = make_static<const constable>();
-    static_assert(std::is_const<decltype(p1)::element_type>{},
-                  "Things are not as const as they ought to be.");
-    EXPECT_EQ(p1->foo(), 5);
-    auto p2 = ceph::const_pointer_cast<constable, sz>(p1);
-    static_assert(!std::is_const<decltype(p2)::element_type>{},
-                  "Things are more const than they ought to be.");
-    EXPECT_TRUE(p2);
-    EXPECT_EQ(p2->foo(), 2);
-  }
   {
     auto p1 = make_static<const constable>();
     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<grandchild>(3);
-    auto p2 = ceph::reinterpret_pointer_cast<constable, sz>(p1);
-    static_assert(std::is_same<decltype(p2)::element_type, constable>{},
-                  "Reinterpret is screwy.");
-    auto p3 = ceph::reinterpret_pointer_cast<grandchild, sz>(p2);
-    static_assert(std::is_same<decltype(p3)::element_type, grandchild>{},
-                  "Reinterpret is screwy.");
-    EXPECT_EQ(p3->func(), 9);
-    EXPECT_EQ(p3->call(10), 30);
-  }
   {
     auto p1 = make_static<grandchild>(3);
     auto p2 = ceph::reinterpret_pointer_cast<constable, sz>(std::move(p1));
@@ -255,6 +209,5 @@ struct exceptional {
 
 TEST(StaticPtr, Exceptional) {
   static_ptr<exceptional> p1(std::in_place_type_t<exceptional>{});
-  EXPECT_ANY_THROW(static_ptr<exceptional> p2(p1));
   EXPECT_ANY_THROW(static_ptr<exceptional> p2(std::move(p1)));
 }