kstd: ring_buffer: clear moved-from buffer

While the standard says that a move-from object is in a valid but
unspecified state, thus making it legal to leave the moved-from buffer
in a non-empty state, we can do better here. For one, standard
containers generally leave the moved-from object in an empty state.
Secondly, destroying the contents of a ring_buffer object has linear
complexity, as does moving the contents of a ring_buffer object. We can
therefore clear the moved-from object while we are moving from it,
without increasing the complexity of the move operation. At the same
time, we can thereby reduce the amount of work that needs to be done
when the moved-from object is finally destroyed.

Note that during the move assignment operation, the moved-from buffer is
in a somewhat peculiar state. Some of its elements have been destroyed,
but its size has not changed. This is okay, because we are essentially
still executing a transition from one valid state into another once. At
the end of the move operation, we ensure that the size of the moved-from
buffer is set to zero, thus reestablishing its invariant.

2 files changed, 42 insertions, 12 deletions

diff --git a/libs/kstd/kstd/ring_buffer.hpp b/libs/kstd/kstd/ring_buffer.hpp
index 8741c6db..66d870dd 100644
--- a/libs/kstd/kstd/ring_buffer.hpp
+++ b/libs/kstd/kstd/ring_buffer.hpp
@@ -11,6 +11,7 @@
 #include <memory>
 #include <ranges>
 #include <type_traits>
+#include <utility>
 
 namespace kstd
 {
@@ -190,8 +191,11 @@ namespace kstd
         : m_size{other.m_size}
         , m_read_index{}
     {
-      std::ranges::for_each(std::views::iota(0uz, other.size()),
-                            [&](auto const i) { m_storage.construct(i, std::move(other[i])); });
+      std::ranges::for_each(std::views::iota(0uz, other.size()), [&](auto const i) {
+        m_storage.construct(i, std::move(other[i]));
+        std::destroy_at(other.element_at(i));
+      });
+      other.m_size = 0;
     }
 
     //! Destroy this ring buffer.
@@ -268,12 +272,18 @@ namespace kstd
       }
 
       auto const to_move_assign = std::min(m_size, other.m_size);
-      std::ranges::move(std::views::take(other, to_move_assign), std::ranges::begin(*this));
+      std::ranges::for_each(std::views::enumerate(std::views::take(other, to_move_assign)), [&](auto const & entry) {
+        auto & [i, element] = entry;
+        (*this)[i] = std::move(element);
+        std::destroy_at(other.element_at(i));
+      });
 
       if (to_move_assign < other.m_size)
       {
-        std::ranges::for_each(std::views::iota(to_move_assign, other.m_size),
-                              [&](auto const i) { std::construct_at(element_at(i), std::move(other[i])); });
+        std::ranges::for_each(std::views::iota(to_move_assign, other.m_size), [&](auto const i) {
+          std::construct_at(element_at(i), std::move(other[i]));
+          std::destroy_at(other.element_at(i));
+        });
       }
       else if (to_move_assign < m_size)
       {
@@ -281,7 +291,7 @@ namespace kstd
                               [](auto & element) { std::destroy_at(&element); });
       }
 
-      m_size = other.m_size;
+      m_size = std::exchange(other.m_size, 0);
       return *this;
     }
 
diff --git a/libs/kstd/kstd/ring_buffer.tests.cpp b/libs/kstd/kstd/ring_buffer.tests.cpp
index 163d537c..d22633d6 100644
--- a/libs/kstd/kstd/ring_buffer.tests.cpp
+++ b/libs/kstd/kstd/ring_buffer.tests.cpp
@@ -331,6 +331,11 @@ SCENARIO("Ring Buffer initialization and construction", "[kstd][ring_buffer]")
       {
         REQUIRE(std::ranges::equal(moved, copy));
       }
+
+      THEN("the moved-from buffer is empty")
+      {
+        REQUIRE(buffer.empty());
+      }
     }
   }
 }
@@ -416,14 +421,19 @@ SCENARIO("Ring Buffer assignment", "[kstd][ring_buffer]")
       kstd::tests::static_copy_move_tracker::reset();
       large = std::move(small);
 
-      THEN("2 move assignments and 1 destruction occurs")
+      THEN("2 move assignments and 3 destructions occur")
       {
-        REQUIRE(kstd::tests::static_copy_move_tracker::dtor_call_count == 1);
+        REQUIRE(kstd::tests::static_copy_move_tracker::dtor_call_count == 3);
         REQUIRE(kstd::tests::static_copy_move_tracker::copy_ctor_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::copy_assignment_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::move_ctor_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::move_assignment_call_count == 2);
       }
+
+      THEN("the moved-from buffer is empty")
+      {
+        REQUIRE(small.empty());
+      }
     }
 
     WHEN("move assigning the large to the small one")
@@ -431,14 +441,19 @@ SCENARIO("Ring Buffer assignment", "[kstd][ring_buffer]")
       kstd::tests::static_copy_move_tracker::reset();
       small = std::move(large);
 
-      THEN("2 move assignments and 1 move construction occurs")
+      THEN("2 move assignments, 1 move construction, and 3 destructions occur")
       {
-        REQUIRE(kstd::tests::static_copy_move_tracker::dtor_call_count == 0);
+        REQUIRE(kstd::tests::static_copy_move_tracker::dtor_call_count == 3);
         REQUIRE(kstd::tests::static_copy_move_tracker::copy_ctor_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::copy_assignment_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::move_ctor_call_count == 1);
         REQUIRE(kstd::tests::static_copy_move_tracker::move_assignment_call_count == 2);
       }
+
+      THEN("the moved-from buffer is empty")
+      {
+        REQUIRE(large.empty());
+      }
     }
 
     WHEN("move assigning the small to the same size one")
@@ -446,14 +461,19 @@ SCENARIO("Ring Buffer assignment", "[kstd][ring_buffer]")
       kstd::tests::static_copy_move_tracker::reset();
       same = std::move(small);
 
-      THEN("2 move assignments occur")
+      THEN("2 move assignments and 2 destructions occur")
       {
-        REQUIRE(kstd::tests::static_copy_move_tracker::dtor_call_count == 0);
+        REQUIRE(kstd::tests::static_copy_move_tracker::dtor_call_count == 2);
         REQUIRE(kstd::tests::static_copy_move_tracker::copy_ctor_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::copy_assignment_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::move_ctor_call_count == 0);
         REQUIRE(kstd::tests::static_copy_move_tracker::move_assignment_call_count == 2);
       }
+
+      THEN("the moved-from buffer is empty")
+      {
+        REQUIRE(small.empty());
+      }
     }
 
     WHEN("move assigning a buffer to itself")