switch to raw ptr, clean moved objects, destructor

c_os/user/lib/ArrayList.h

@@ -6,9 +6,9 @@
 //       ArrayList instead, without additional effort.
 
 #include "user/lib/List.h"
+#include "user/lib/Logger.h"
 #include "user/lib/mem/UniquePointer.h"
 #include <cstddef>
-#include <type_traits>
 #include <utility>
 
 // I put most of the implementation in the header because the templating makes it cumbersome to split
@@ -23,16 +23,13 @@ private:
     static constexpr const std::size_t default_cap = 10;  // Arbitrary but very small because this isn't a real OS :(
     static constexpr const std::size_t min_cap = 5;       // Slots to allocate extra when array full
 
-    bse::unique_ptr<Type[]> buf;  // Heap allocated as size needs to change during runtime
+    Type* buf = nullptr;  // Heap allocated as size needs to change during runtime
                           // Can't use Array for the same reason so we use a C Style array
-                                  // NOTE: I wouldn't normally use smart pointers for low level datastructures
-                                  //       but here it doesn't hurt as the unique_ptr basically has no overhead
-                                  //       (And it saved 2 deletes ¯\_(ツ)_/¯)
     std::size_t buf_pos = 0;
     std::size_t buf_cap = 0;
 
     void init() {
-        buf = bse::make_unique<Type[]>(ArrayList::default_cap);
+        buf = new Type[ArrayList::default_cap];
         buf_cap = ArrayList::default_cap;
     }
 
@@ -43,7 +40,7 @@ private:
     // Enlarges the buffer if we run out of space
     std::size_t expand() {
         // Init if necessary
-        if (!buf) {
+        if (buf == nullptr) {
             init();
             return buf_cap;  // Dont have to realloc after init
         }
@@ -53,15 +50,17 @@ private:
             std::size_t new_cap = buf_cap + min_cap;
 
             // Alloc new array
-            bse::unique_ptr<Type[]> new_buf = bse::make_unique<Type[]>(new_cap);
+            Type* new_buf = new Type[new_cap];
 
             // Swap current elements to new array
             for (std::size_t i = 0; i < size(); ++i) {
-                new_buf[i] = std::move(buf[i]);  // Should I have just used a regular pointer?
+                new_buf[i] = std::move(buf[i]);
+                buf[i].~Type();
             }
 
             // Move new array to buf, deleting the old array
-            buf = std::move(new_buf);
+            delete[] buf;
+            buf = new_buf;
             buf_cap = new_cap;
         }
 
@@ -92,6 +91,7 @@ private:
             //         pos = 4                     pos = 5
             for (std::size_t idx = size(); idx > i; --idx) {  // idx > i so idx - 1 is never < 0
                 buf[idx] = std::move(buf[idx - 1]);
+                buf[idx - 1].~Type();
             }
 
             // Only change pos if elements were copied
@@ -118,13 +118,19 @@ private:
         //       pos = 3                 pos = 2
         for (std::size_t idx = i; idx < size(); ++idx) {  // idx < pos so idx + 1 is never outside of size limit
             buf[idx] = std::move(buf[idx + 1]);
+            buf[idx + 1].~Type();
         }
 
         return size();
     }
 
 public:
-    ~ArrayList() = default;  // Buffer deletes itself (I hope)
+    ~ArrayList() {
+        for (std::size_t i; i < size(); ++i) {
+            buf[i].~Type();
+        }
+        delete[] buf;
+    }
 
     Iterator begin() override {
         return Iterator(&buf[0]);
@@ -135,6 +141,7 @@ public:
     }
 
     // Returns new pos
+    // NOTE: Insert copies
     std::size_t insert_at(Type e, std::size_t i) override {
         if (i > size()) {
             // Error: Space between elements
@@ -146,8 +153,12 @@ public:
             return insert_last(std::forward<Type>(e));
         }
 
+        if constexpr (std::is_copy_assignable_v<Type>) {
             copy_right(i);  // Changes pos
             buf[i] = e;
+        } else {
+            return -1;
+        }
 
         return size();
     }
@@ -157,21 +168,35 @@ public:
     }
 
     std::size_t insert_last(Type e) override {
-        expand();
+        if (!buf) {
+            init();
+        }
+
+        if constexpr (std::is_copy_assignable_v<Type>) {
             buf[size()] = e;
             ++buf_pos;
+            expand();
+        } else {
+            return -1;
+        }
 
         return size();
     }
 
     // Returns removed element
+    // NOTE: Remove moves
     std::optional<Type> remove_at(std::size_t i) override {
         if (i >= size()) {
             // ERROR: No element here
             return std::nullopt;
         }
 
+        // Move moves the data if possible, copies otherwise
+        // It does not destroy the original object, an intact
+        // invariant is left so we have to destruct the old object
         Type e = std::move(buf[i]);
+        buf[i].~Type();  // Cleanup the leftovers from the move
+
         copy_left(i);
         return e;
     }
@@ -204,9 +229,14 @@ public:
             return std::nullopt;
         }
 
+        // TODO: assignable or constructable?
+        if constexpr (std::is_copy_assignable_v<Type>) {
             return buf[i];
         }
 
+        return std::nullopt;
+    }
+
     std::optional<Type> first() const override {
         return get(0);
     }
@@ -225,7 +255,7 @@ public:
 
     void print(OutStream& out) const override {
         // Our stream cannot print all types so enable this only for debugging purposes (only int)
-        if constexpr (std::is_same<Type, int>::value) {
+        if constexpr (std::is_same_v<Type, int>) {
             if (empty()) {
                 out << "Print List (0 elements)" << endl;
                 return;