adt - extend circular map unit test to test the container with move-only types

lib/include/srsran/adt/circular_map.h

@@ -13,6 +13,7 @@
 #ifndef SRSRAN_ID_MAP_H
 #define SRSRAN_ID_MAP_H
 
+#include "detail/type_storage.h"
 #include "expected.h"
 #include "srsran/common/srsran_assert.h"
 #include <array>
@@ -25,7 +26,6 @@ class static_circular_map
   static_assert(std::is_integral<K>::value and std::is_unsigned<K>::value, "Map key must be an unsigned integer");
 
   using obj_t = std::pair<K, T>;
-  using obj_storage_t = typename std::aligned_storage<sizeof(obj_t), alignof(obj_t)>::type;
 
 public:
   class iterator
@@ -48,23 +48,23 @@ public:
 
     obj_t& operator*()
     {
-      srsran_assert(idx < ptr->buffer.size(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->buffer.size());
+      srsran_assert(idx < ptr->capacity(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->capacity());
       return ptr->get_obj_(idx);
     }
     obj_t* operator->()
     {
-      srsran_assert(idx < ptr->buffer.size(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->buffer.size());
+      srsran_assert(idx < ptr->capacity(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->capacity());
       return &ptr->get_obj_(idx);
     }
     const obj_t* operator*() const
     {
-      srsran_assert(idx < ptr->buffer.size(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->buffer.size());
+      srsran_assert(idx < ptr->capacity(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->capacity());
-      return ptr->buffer[idx];
+      return &ptr->get_obj_(idx);
     }
     const obj_t* operator->() const
     {
-      srsran_assert(idx < ptr->buffer.size(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->buffer.size());
+      srsran_assert(idx < ptr->capacity(), "Iterator out-of-bounds (%zd >= %zd)", idx, ptr->capacity());
-      return ptr->buffer[idx];
+      return &ptr->get_obj_(idx);
     }
 
     bool operator==(const iterator& other) const { return ptr == other.ptr and idx == other.idx; }
@@ -88,8 +88,8 @@ public:
       return *this;
     }
 
-    const obj_t* operator*() const { return ptr->buffer[idx]; }
+    const obj_t* operator*() const { return &ptr->buffer[idx].get(); }
-    const obj_t* operator->() const { return ptr->buffer[idx]; }
+    const obj_t* operator->() const { return &ptr->buffer[idx].get(); }
 
     bool operator==(const const_iterator& other) const { return ptr == other.ptr and idx == other.idx; }
     bool operator!=(const const_iterator& other) const { return not(*this == other); }
@@ -103,17 +103,17 @@ public:
   static_circular_map() { std::fill(present.begin(), present.end(), false); }
   static_circular_map(const static_circular_map<K, T, N>& other) : present(other.present), count(other.count)
   {
-    for (size_t idx = 0; idx < other.size(); ++idx) {
+    for (size_t idx = 0; idx < other.capacity(); ++idx) {
       if (present[idx]) {
-        new (&buffer[idx]) obj_t(other.get_obj_(idx));
+        buffer[idx].template construct(other.get_obj_(idx));
       }
     }
   }
   static_circular_map(static_circular_map<K, T, N>&& other) noexcept : present(other.present), count(other.count)
   {
-    for (size_t idx = 0; idx < other.size(); ++idx) {
+    for (size_t idx = 0; idx < other.capacity(); ++idx) {
       if (present[idx]) {
-        new (&buffer[idx]) obj_t(std::move(other.get_obj_(idx)));
+        buffer[idx].template construct(std::move(other.get_obj_(idx)));
       }
     }
     other.clear();
@@ -124,26 +124,21 @@ public:
     if (this == &other) {
       return *this;
     }
-    clear();
+    for (size_t idx = 0; idx < other.capacity(); ++idx) {
+      copy_if_present_helper(buffer[idx], other.buffer[idx], present[idx], other.present[idx]);
+    }
     count   = other.count;
     present = other.present;
-    for (size_t idx = 0; idx < other.size(); ++idx) {
-      if (present[idx]) {
-        new (&buffer[idx]) obj_t(other.get_obj_(idx));
-      }
-    }
   }
   static_circular_map& operator=(static_circular_map<K, T, N>&& other) noexcept
   {
-    clear();
+    for (size_t idx = 0; idx < other.capacity(); ++idx) {
+      move_if_present_helper(buffer[idx], other.buffer[idx], present[idx], other.present[idx]);
+    }
     count   = other.count;
     present = other.present;
-    for (size_t idx = 0; idx < other.size(); ++idx) {
+    other.clear();
-      if (present[idx]) {
+    return *this;
-        new (&buffer[idx]) obj_t(std::move(other.get_obj_(idx)));
-      }
-    }
-    clear();
   }
 
   bool contains(K id)
@@ -158,7 +153,7 @@ public:
     if (present[idx]) {
       return false;
     }
-    new (&buffer[idx]) obj_t(id, obj);
+    buffer[idx].template construct(id, obj);
     present[idx] = true;
     count++;
     return true;
@@ -169,7 +164,7 @@ public:
     if (present[idx]) {
       return srsran::expected<iterator, T>(std::move(obj));
     }
-    new (&buffer[idx]) obj_t(id, std::move(obj));
+    buffer[idx].template construct(id, std::move(obj));
     present[idx] = true;
     count++;
     return iterator(this, idx);
@@ -242,10 +237,10 @@ public:
   }
 
 private:
-  obj_t&       get_obj_(size_t idx) { return reinterpret_cast<obj_t&>(buffer[idx]); }
+  obj_t&       get_obj_(size_t idx) { return buffer[idx].get(); }
-  const obj_t& get_obj_(size_t idx) const { return reinterpret_cast<obj_t&>(buffer[idx]); }
+  const obj_t& get_obj_(size_t idx) const { return buffer[idx].get(); }
 
-  std::array<obj_storage_t, N> buffer;
+  std::array<type_storage<obj_t>, N> buffer;
   std::array<bool, N>                present;
   size_t                             count = 0;
 };

lib/include/srsran/adt/detail/type_storage.h

@@ -0,0 +1,76 @@
+/**
+ *
+ * \section COPYRIGHT
+ *
+ * Copyright 2013-2021 Software Radio Systems Limited
+ *
+ * By using this file, you agree to the terms and conditions set
+ * forth in the LICENSE file which can be found at the top level of
+ * the distribution.
+ *
+ */
+
+#ifndef SRSRAN_TYPE_STORAGE_H
+#define SRSRAN_TYPE_STORAGE_H
+
+#include <type_traits>
+#include <utility>
+
+namespace srsran {
+
+template <typename T>
+struct type_storage {
+  template <typename... Args>
+  void construct(Args&&... args)
+  {
+    new (&buffer) T(std::forward<Args>(args)...);
+  }
+  void destroy() { get().~T(); }
+  void copy_ctor(const type_storage<T>& other) { buffer.get() = other.get(); }
+  void move_ctor(type_storage<T>&& other) { buffer.get() = std::move(other.get()); }
+  void copy_assign(const type_storage<T>& other)
+  {
+    if (this == &other) {
+      return;
+    }
+    get() = other.get();
+  }
+  void move_assign(type_storage<T>&& other) { get() = std::move(other.get()); }
+
+  T&       get() { return reinterpret_cast<T&>(buffer); }
+  const T& get() const { return reinterpret_cast<const T&>(buffer); }
+
+  typename std::aligned_storage<sizeof(T), alignof(T)>::type buffer;
+};
+
+template <typename T>
+void copy_if_present_helper(type_storage<T>& lhs, const type_storage<T>& rhs, bool lhs_present, bool rhs_present)
+{
+  if (lhs_present and rhs_present) {
+    lhs.get() = rhs.get();
+  }
+  if (lhs_present) {
+    lhs.destroy();
+  }
+  if (rhs_present) {
+    lhs.template construct(rhs.get());
+  }
+}
+
+template <typename T>
+void move_if_present_helper(type_storage<T>& lhs, type_storage<T>& rhs, bool lhs_present, bool rhs_present)
+{
+  if (lhs_present and rhs_present) {
+    lhs.move_assign(std::move(rhs));
+  }
+  if (lhs_present) {
+    lhs.destroy();
+  }
+  if (rhs_present) {
+    lhs.template construct(std::move(rhs.get()));
+  }
+}
+
+} // namespace srsran
+
+#endif // SRSRAN_TYPE_STORAGE_H

lib/test/adt/circular_map_test.cc

@@ -15,7 +15,7 @@
 
 namespace srsran {
 
-int test_id_map()
+void test_id_map()
 {
   static_circular_map<uint32_t, std::string, 16> myobj;
   TESTASSERT(myobj.size() == 0 and myobj.empty() and not myobj.full());
@@ -57,11 +57,9 @@ int test_id_map()
   TESTASSERT(myobj.size() == 2 and not myobj.empty() and not myobj.full());
   myobj.clear();
   TESTASSERT(myobj.size() == 0 and myobj.empty());
-
-  return SRSRAN_SUCCESS;
 }
 
-int test_id_map_wraparound()
+void test_id_map_wraparound()
 {
   static_circular_map<uint32_t, std::string, 4> mymap;
 
@@ -81,8 +79,56 @@ int test_id_map_wraparound()
   TESTASSERT(not mymap.full());
   TESTASSERT(mymap.insert(4, "4"));
   TESTASSERT(mymap.full());
+}
 
-  return SRSRAN_SUCCESS;
+struct C {
+  C() { count++; }
+  ~C() { count--; }
+  C(C&&) { count++; }
+  C(const C&) = delete;
+  C& operator=(C&&) = default;
+
+  static size_t count;
+};
+size_t C::count = 0;
+
+void test_correct_destruction()
+{
+  TESTASSERT(C::count == 0);
+  {
+    static_circular_map<uint32_t, C, 4> circ_buffer;
+    TESTASSERT(C::count == 0);
+    TESTASSERT(circ_buffer.insert(0, C{}));
+    TESTASSERT(C::count == 1);
+    TESTASSERT(circ_buffer.insert(1, C{}));
+    TESTASSERT(circ_buffer.insert(2, C{}));
+    TESTASSERT(circ_buffer.insert(3, C{}));
+    TESTASSERT(C::count == 4);
+    TESTASSERT(not circ_buffer.insert(4, C{}));
+    TESTASSERT(C::count == 4);
+    TESTASSERT(circ_buffer.erase(1));
+    TESTASSERT(C::count == 3);
+    TESTASSERT(not circ_buffer.contains(1));
+
+    std::array<uint32_t, 3> content{0, 2, 3};
+    size_t                  i = 0;
+    for (auto& e : circ_buffer) {
+      TESTASSERT(content[i] == e.first);
+      i++;
+    }
+
+    TESTASSERT(C::count == 3);
+    static_circular_map<uint32_t, C, 4> circ_buffer2;
+    circ_buffer2 = std::move(circ_buffer);
+    TESTASSERT(C::count == 3);
+
+    static_circular_map<uint32_t, C, 4> circ_buffer3;
+    TESTASSERT(circ_buffer3.insert(1, C{}));
+    TESTASSERT(C::count == 4);
+    circ_buffer2 = std::move(circ_buffer3);
+    TESTASSERT(C::count == 1);
+  }
+  TESTASSERT(C::count == 0);
 }
 
 } // namespace srsran
@@ -94,7 +140,10 @@ int main(int argc, char** argv)
 
   srsran::test_init(argc, argv);
 
-  TESTASSERT(srsran::test_id_map() == SRSRAN_SUCCESS);
+  srsran::test_id_map();
-  TESTASSERT(srsran::test_id_map_wraparound() == SRSRAN_SUCCESS);
+  srsran::test_id_map_wraparound();
+  srsran::test_correct_destruction();
+
+  printf("Success\n");
   return SRSRAN_SUCCESS;
 }