From 9d4c08747cee857a8153b66aed34a9d0d0e72023 Mon Sep 17 00:00:00 2001
From: Andrew Noyes <andrew@weaselab.dev>
Date: Thu, 14 Aug 2025 12:06:42 -0400
Subject: [PATCH] Require types used with construct are trivially destructible

---
 src/arena_allocator.hpp        | 27 +++++++++++++++++-----
 tests/test_arena_allocator.cpp | 41 +++++++++++++++++++++++-----------
 2 files changed, 49 insertions(+), 19 deletions(-)

diff --git a/src/arena_allocator.hpp b/src/arena_allocator.hpp
index 884c618..2006ee1 100644
--- a/src/arena_allocator.hpp
+++ b/src/arena_allocator.hpp
@@ -5,6 +5,7 @@
 #include <cstdint>
 #include <cstdlib>
 #include <new>
+#include <type_traits>
 #include <utility>
 
 /**
@@ -36,8 +37,9 @@
  * ArenaAllocator arena(1024);
  * void* ptr = arena.allocate(100);
  *
- * // Construct objects in-place
- * MyClass* obj = arena.construct<MyClass>(arg1, arg2);
+ * // Construct trivially destructible objects in-place
+ * int* num = arena.construct<int>(42);
+ * MyPOD* obj = arena.construct<MyPOD>(arg1, arg2);  // If MyPOD is trivial
  *
  * // Track memory usage
  * size_t total = arena.total_allocated();
@@ -231,23 +233,36 @@ public:
    * This is a convenience method that combines allocation with in-place
    * construction. It properly handles alignment requirements for type T.
    *
-   * @tparam T The type of object to construct
+   * @tparam T The type of object to construct (must be trivially destructible)
    * @tparam Args Types of constructor arguments
    * @param args Arguments to forward to T's constructor
    * @return Pointer to the constructed object
    * @throws std::bad_alloc if memory allocation fails
    *
+   * ## Type Requirements:
+   * T must be trivially destructible (std::is_trivially_destructible_v<T>).
+   * This prevents subtle bugs since destructors are never called for objects
+   * constructed in the arena.
+   *
    * ## Example:
    * ```cpp
-   * std::string* str = arena.construct<std::string>("Hello, World!");
-   * MyClass* obj = arena.construct<MyClass>(arg1, arg2, arg3);
+   * int* num = arena.construct<int>(42);                    // ✓ Trivially
+   * destructible MyPOD* pod = arena.construct<MyPOD>(arg1, arg2);       // ✓ If
+   * MyPOD is trivial std::string* str = arena.construct<std::string>("hi"); //
+   * ✗ Compile error!
    * ```
    *
    * ## Note:
    * Objects constructed this way cannot be individually destroyed.
-   * Their destructors will NOT be called automatically.
+   * Their destructors will NOT be called automatically - hence the requirement
+   * for trivially destructible types.
    */
   template <typename T, typename... Args> T *construct(Args &&...args) {
+    static_assert(
+        std::is_trivially_destructible_v<T>,
+        "ArenaAllocator::construct requires trivially destructible types. "
+        "Objects constructed in the arena will not have their destructors "
+        "called.");
     void *ptr = allocate(sizeof(T), alignof(T));
     return new (ptr) T(std::forward<Args>(args)...);
   }
diff --git a/tests/test_arena_allocator.cpp b/tests/test_arena_allocator.cpp
index 73a1c3a..708b7a8 100644
--- a/tests/test_arena_allocator.cpp
+++ b/tests/test_arena_allocator.cpp
@@ -1,5 +1,6 @@
 #define DOCTEST_CONFIG_IMPLEMENT_WITH_MAIN
 #include "arena_allocator.hpp"
+#include <cstring>
 #include <doctest/doctest.h>
 #include <string>
 #include <vector>
@@ -106,10 +107,17 @@ TEST_CASE("ArenaAllocator construct template") {
     CHECK(*ptr == 42);
   }
 
-  SUBCASE("construct string") {
-    std::string *ptr = arena.construct<std::string>("hello world");
+  SUBCASE("construct array") {
+    struct FixedString {
+      char data[16];
+      FixedString(const char *str) {
+        std::strncpy(data, str, sizeof(data) - 1);
+        data[sizeof(data) - 1] = '\0';
+      }
+    };
+    FixedString *ptr = arena.construct<FixedString>("hello world");
     CHECK(ptr != nullptr);
-    CHECK(*ptr == "hello world");
+    CHECK(std::strcmp(ptr->data, "hello world") == 0);
   }
 
   SUBCASE("construct multiple objects") {
@@ -122,10 +130,15 @@ TEST_CASE("ArenaAllocator construct template") {
   }
 
   SUBCASE("construct with multiple arguments") {
-    auto *ptr = arena.construct<std::pair<int, std::string>>(42, "test");
+    struct IntPair {
+      int first;
+      int second;
+      IntPair(int a, int b) : first(a), second(b) {}
+    };
+    auto *ptr = arena.construct<IntPair>(42, 24);
     CHECK(ptr != nullptr);
     CHECK(ptr->first == 42);
-    CHECK(ptr->second == "test");
+    CHECK(ptr->second == 24);
   }
 }
 
@@ -266,27 +279,29 @@ TEST_CASE("ArenaAllocator edge cases") {
   }
 }
 
-struct TestObject {
+struct TestPOD {
   int value;
-  std::string name;
+  char name[16];
 
-  TestObject(int v, const std::string &n) : value(v), name(n) {}
-  ~TestObject() = default;
+  TestPOD(int v, const char *n) : value(v) {
+    std::strncpy(name, n, sizeof(name) - 1);
+    name[sizeof(name) - 1] = '\0';
+  }
 };
 
 TEST_CASE("ArenaAllocator with custom objects") {
   ArenaAllocator arena;
 
-  TestObject *obj1 = arena.construct<TestObject>(42, "first");
-  TestObject *obj2 = arena.construct<TestObject>(84, "second");
+  TestPOD *obj1 = arena.construct<TestPOD>(42, "first");
+  TestPOD *obj2 = arena.construct<TestPOD>(84, "second");
 
   CHECK(obj1 != nullptr);
   CHECK(obj2 != nullptr);
   CHECK(obj1 != obj2);
   CHECK(obj1->value == 42);
-  CHECK(obj1->name == "first");
+  CHECK(std::strcmp(obj1->name, "first") == 0);
   CHECK(obj2->value == 84);
-  CHECK(obj2->name == "second");
+  CHECK(std::strcmp(obj2->name, "second") == 0);
 }
 
 TEST_CASE("ArenaAllocator geometric growth policy") {