From 0cea80218df874bb3370f13c5e03ab22b230be1b Mon Sep 17 00:00:00 2001
From: Andreas Kling <awesomekling@gmail.com>
Date: Wed, 23 Oct 2019 12:20:45 +0200
Subject: [PATCH] AK: Make it possible to store complex types in a
 CircularQueue

Previously we would not run destructors for items in a CircularQueue,
which would lead to memory leaks.

This patch fixes that, and also adds a basic unit test for the class.
---
 AK/CircularDeque.h             |  5 ++--
 AK/CircularQueue.h             | 36 ++++++++++++++++++------
 AK/Tests/Makefile              |  6 +++-
 AK/Tests/TestCircularQueue.cpp | 51 ++++++++++++++++++++++++++++++++++
 4 files changed, 87 insertions(+), 11 deletions(-)
 create mode 100644 AK/Tests/TestCircularQueue.cpp

diff --git a/AK/CircularDeque.h b/AK/CircularDeque.h
index 7f7f4d7f19f..fafe4e65ca8 100644
--- a/AK/CircularDeque.h
+++ b/AK/CircularDeque.h
@@ -8,12 +8,13 @@ namespace AK {
 
 template<typename T, int Capacity>
 class CircularDeque : public CircularQueue<T, Capacity> {
-
 public:
     T dequeue_end()
     {
         ASSERT(!this->is_empty());
-        T value = this->m_elements[(this->m_head + this->m_size - 1) % Capacity];
+        auto& slot = this->elements()[(this->m_head + this->m_size - 1) % Capacity];
+        T value = move(slot);
+        slot.~T();
         this->m_size--;
         return value;
     }
diff --git a/AK/CircularQueue.h b/AK/CircularQueue.h
index e48249bf5e3..605472c8af6 100644
--- a/AK/CircularQueue.h
+++ b/AK/CircularQueue.h
@@ -10,12 +10,18 @@ class CircularQueue {
 public:
     CircularQueue()
     {
-        for (int i = 0; i < Capacity; ++i)
-            m_elements[i] = T();
+    }
+
+    ~CircularQueue()
+    {
+        clear();
     }
 
     void clear()
     {
+        for (int i = 0; i < m_size; ++i)
+            elements()[(m_head + i) % Capacity].~T();
+
         m_head = 0;
         m_size = 0;
     }
@@ -25,25 +31,36 @@ public:
 
     int capacity() const { return Capacity; }
 
-    void enqueue(const T& t)
+    void enqueue(T&& value)
     {
-        m_elements[(m_head + m_size) % Capacity] = t;
+        auto& slot = elements()[(m_head + m_size) % Capacity];
+        if (m_size == Capacity)
+            slot.~T();
+
+        new (&slot) T(value);
         if (m_size == Capacity)
             m_head = (m_head + 1) % Capacity;
         else
             ++m_size;
     }
 
+    void enqueue(const T& value)
+    {
+        enqueue(T(value));
+    }
+
     T dequeue()
     {
         ASSERT(!is_empty());
-        T value = m_elements[m_head];
+        auto& slot = elements()[m_head];
+        T value = move(slot);
+        slot.~T();
         m_head = (m_head + 1) % Capacity;
         --m_size;
         return value;
     }
 
-    const T& at(int index) const { return m_elements[(m_head + index) % Capacity]; }
+    const T& at(int index) const { return elements()[(m_head + index) % Capacity]; }
 
     const T& first() const { return at(0); }
     const T& last() const { return at(size() - 1); }
@@ -59,7 +76,7 @@ public:
             return *this;
         }
 
-        const T& operator*() const { return m_queue.m_elements[m_index]; }
+        const T& operator*() const { return m_queue.elements()[m_index]; }
 
     private:
         friend class CircularQueue;
@@ -78,8 +95,11 @@ public:
     int head_index() const { return m_head; }
 
 protected:
+    T* elements() { return reinterpret_cast<T*>(m_storage); }
+    const T* elements() const { return reinterpret_cast<const T*>(m_storage); }
+
     friend class ConstIterator;
-    T m_elements[Capacity];
+    alignas(T) u8 m_storage[sizeof(T) * Capacity];
     int m_size { 0 };
     int m_head { 0 };
 };
diff --git a/AK/Tests/Makefile b/AK/Tests/Makefile
index 50f5339a23a..77f8957daf0 100644
--- a/AK/Tests/Makefile
+++ b/AK/Tests/Makefile
@@ -1,4 +1,4 @@
-PROGRAMS = TestAtomic TestString TestQueue TestVector TestHashMap TestJSON TestWeakPtr TestNonnullRefPtr TestRefPtr TestFixedArray TestFileSystemPath TestURL TestStringView TestUtf8
+PROGRAMS = TestAtomic TestString TestQueue TestVector TestHashMap TestJSON TestWeakPtr TestNonnullRefPtr TestRefPtr TestFixedArray TestFileSystemPath TestURL TestStringView TestUtf8 TestCircularQueue
 
 CXXFLAGS = -std=c++17 -Wall -Wextra -ggdb3 -O2 -I../ -I../../
 
@@ -70,6 +70,10 @@ TestStringView: TestStringView.o $(SHARED_TEST_OBJS)
 TestUtf8: TestUtf8.o $(SHARED_TEST_OBJS)
 	$(PRE_CXX) $(CXX) $(CXXFLAGS) -o $@ TestUtf8.o $(SHARED_TEST_OBJS)
 
+TestCircularQueue: TestCircularQueue.o $(SHARED_TEST_OBJS)
+	$(PRE_CXX) $(CXX) $(CXXFLAGS) -o $@ TestCircularQueue.o $(SHARED_TEST_OBJS)
+
+
 clean:
 	rm -f $(SHARED_TEST_OBJS)
 	rm -f $(PROGRAMS)
diff --git a/AK/Tests/TestCircularQueue.cpp b/AK/Tests/TestCircularQueue.cpp
new file mode 100644
index 00000000000..fd49b88dcc3
--- /dev/null
+++ b/AK/Tests/TestCircularQueue.cpp
@@ -0,0 +1,51 @@
+#include <AK/TestSuite.h>
+#include <AK/String.h>
+#include <AK/CircularQueue.h>
+
+TEST_CASE(basic)
+{
+    CircularQueue<int, 3> ints;
+    EXPECT(ints.is_empty());
+    ints.enqueue(1);
+    ints.enqueue(2);
+    ints.enqueue(3);
+    EXPECT_EQ(ints.size(), 3);
+
+    ints.enqueue(4);
+    EXPECT_EQ(ints.size(), 3);
+    EXPECT_EQ(ints.dequeue(), 2);
+    EXPECT_EQ(ints.dequeue(), 3);
+    EXPECT_EQ(ints.dequeue(), 4);
+    EXPECT_EQ(ints.size(), 0);
+}
+
+TEST_CASE(complex_type)
+{
+    CircularQueue<String, 2> strings;
+
+    strings.enqueue("ABC");
+    strings.enqueue("DEF");
+
+    EXPECT_EQ(strings.size(), 2);
+
+    strings.enqueue("abc");
+    strings.enqueue("def");
+
+    EXPECT_EQ(strings.dequeue(), "abc");
+    EXPECT_EQ(strings.dequeue(), "def");
+}
+
+TEST_CASE(complex_type_clear)
+{
+    CircularQueue<String, 5> strings;
+    strings.enqueue("xxx");
+    strings.enqueue("xxx");
+    strings.enqueue("xxx");
+    strings.enqueue("xxx");
+    strings.enqueue("xxx");
+    EXPECT_EQ(strings.size(), 5);
+    strings.clear();
+    EXPECT_EQ(strings.size(), 0);
+}
+
+TEST_MAIN(CircularQueue)