Allocator deallocate is void rather than returning FudStatus.

11 files changed, 147 insertions, 64 deletions

diff --git a/include/fud_allocator.hpp b/include/fud_allocator.hpp
index e4078ac..9b90deb 100644
--- a/include/fud_allocator.hpp
+++ b/include/fud_allocator.hpp
@@ -32,7 +32,7 @@ class alignas(std::max_align_t) Allocator {
 
     virtual Result<std::byte*, FudStatus> allocate(size_t bytes, size_t alignment = alignof(std::max_align_t)) = 0;
 
-    virtual FudStatus deallocate(std::byte* pointer, size_t bytes) = 0;
+    virtual void deallocate(std::byte* pointer, size_t bytes) = 0;
 
     virtual bool isEqual(const Allocator& rhs) const = 0;
 };
@@ -48,7 +48,7 @@ class FudAllocator : public Allocator {
 
     virtual Result<std::byte*, FudStatus> allocate(size_t bytes, size_t alignment = alignof(std::max_align_t)) override;
 
-    virtual FudStatus deallocate(std::byte* pointer, size_t bytes) override;
+    virtual void deallocate(std::byte* pointer, size_t bytes) override;
 
     virtual bool isEqual(const Allocator& rhs) const override;
 };
@@ -61,7 +61,7 @@ class NullAllocator : public Allocator {
 
     virtual Result<std::byte*, FudStatus> allocate(size_t bytes, size_t alignment = alignof(std::max_align_t)) override;
 
-    virtual FudStatus deallocate(std::byte* pointer, size_t bytes) override;
+    virtual void deallocate(std::byte* pointer, size_t bytes) override;
 
     virtual bool isEqual(const Allocator& rhs) const override;
 };
diff --git a/include/fud_c_string.hpp b/include/fud_c_string.hpp
index d55ba30..a1ab51a 100644
--- a/include/fud_c_string.hpp
+++ b/include/fud_c_string.hpp
@@ -20,14 +20,17 @@
 
 #include <climits>
 #include <cstddef>
+#include <limits>
 #include <sys/types.h>
 
 
 namespace fud {
 
+constexpr ssize_t MAX_C_STRING_LENGTH = std::numeric_limits<ssize_t>::max() - 1;
+
 constexpr ssize_t cStringLength(const char* str, size_t maxLength)
 {
-    if (str == nullptr || maxLength > (SSIZE_MAX - 1)) {
+    if (str == nullptr || maxLength > MAX_C_STRING_LENGTH) {
         return -1;
     }
 
@@ -46,13 +49,14 @@ constexpr ssize_t cStringLength(const char* str, size_t maxLength)
 
 constexpr ssize_t cStringLength(const char* str)
 {
-    constexpr auto maxLength = SSIZE_MAX - 1;
-    return cStringLength(str, maxLength);
+    return cStringLength(str, MAX_C_STRING_LENGTH);
 }
 
 constexpr ssize_t cStringLength(const char8_t* str, size_t maxLength)
 {
-    if (str == nullptr || maxLength > (SSIZE_MAX - 1)) {
+    // Cannot cast str to const char* without breaking constexpr
+    // return cStringLength(reinterpret_cast<const char*>(str), maxLength);
+    if (str == nullptr || maxLength > MAX_C_STRING_LENGTH) {
         return -1;
     }
 
@@ -71,8 +75,7 @@ constexpr ssize_t cStringLength(const char8_t* str, size_t maxLength)
 
 constexpr ssize_t cStringLength(const char8_t* str)
 {
-    constexpr auto maxLength = SSIZE_MAX - 1;
-    return cStringLength(str, maxLength);
+    return cStringLength(str, MAX_C_STRING_LENGTH);
 }
 
 } // namespace fud
diff --git a/include/fud_file.hpp b/include/fud_file.hpp
index 51826a6..7a53468 100644
--- a/include/fud_file.hpp
+++ b/include/fud_file.hpp
@@ -165,10 +165,17 @@ class RegularFile {
     FileAccessMode m_modeFlags{};
 };
 
+enum class ReadPolicy {
+    Unbuffered,
+    ReadAhead,
+};
+
 class BufferedRegularFile {
 public:
-    DrainResult write(const void* source, size_t sourceSize, size_t length, size_t offset);
-    // DrainResult read(void* sink, );
+    /** \brief Write from source to file as sink. */
+    DrainResult write(const std::byte* source, size_t sourceSize, size_t length, size_t offset);
+    /** \brief Read from file as source to sink. */
+    DrainResult read(std::byte* sink, size_t sinkSize, size_t length, size_t offset);
 private:
     Vector<std::byte> m_readBuffer{Vector<std::byte>::NullVector()};
     Vector<std::byte> m_writeBuffer{Vector<std::byte>::NullVector()};
diff --git a/include/fud_string.hpp b/include/fud_string.hpp
index 226bb89..59c434a 100644
--- a/include/fud_string.hpp
+++ b/include/fud_string.hpp
@@ -60,6 +60,8 @@ class String {
      * \returns FudStatus::AllocFailure if the allocator fails.
      */
     static StringResult makeFromCString(const char* cString);
+
+    /** @copydoc String::makeFromCString(const char* cString) */
     static StringResult makeFromCString(const char8_t* cString);
 
     /** \brief Create a string from a C String, specifying the allocator.
@@ -74,6 +76,8 @@ class String {
      * \returns FudStatus::AllocFailure if the allocator fails.
      */
     static StringResult makeFromCString(const char* cString, Allocator* allocator);
+
+    /** @copydoc String::makeFromCString(const char* cString, Allocator* allocator) */
     static StringResult makeFromCString(const char8_t* cString, Allocator* allocator);
 
     /** \brief Create a string from concatenating multiple C Strings.
@@ -187,20 +191,33 @@ class String {
      * fud allocator. */
     String() noexcept = default;
 
+    /* The copy constructor is deleted because it is fallible. */
     String(const String& rhs) = delete;
 
+    /** \brief Infallibly moves the string. */
     String(String&& rhs) noexcept;
 
-    ~String();
+    /* Destructors need no documentation. */
+    ~String() noexcept;
 
+    /* The copy assignment operator not deleted because it is fallible. */
     String& operator=(const String& rhs) = delete;
 
+    /** \brief Takes ownership of rhs, destroying the contents of this string in
+     * the process. The allocator is taken from rhs.
+     */
     String& operator=(String&& rhs) noexcept;
 
-    static StringResult from(const String& rhs);
+    /** \brief Create a String by copying from an existing rhs, optionally
+     * specifying a different allocator. If allocatorOption is NullOpt, the
+     * allocator from rhs is used.
+     */
+    static StringResult from(const String& rhs, Option<Allocator*> allocatorOption = NullOpt);
 
+    /** \brief Create a String by copying from a view, with the specified allocator. */
     static StringResult from(StringView view, Allocator* allocator = &globalFudAllocator);
 
+    /** \brief Copy the contents of rhs, without modifying rhs. */
     FudStatus copy(const String& rhs);
 
     /** \brief The raw length of the string's data, excluding the null terminator. */
@@ -246,12 +263,19 @@ class String {
         return reinterpret_cast<const char*>(data());
     }
 
+    /** \brief Indicates if the contents of the string form a valid sequence of
+     * UTF8 code points. */
     [[nodiscard]] bool utf8Valid() const;
 
+    /** \brief Attempts to reserve newCapacity bytes of storage. */
     FudStatus reserve(size_t newCapacity);
 
+    /** \brief Returns the last character in the sequence if the length is
+     * greater than zero. */
     [[nodiscard]] Option<utf8> back();
 
+    /** \brief Returns the remaining capacity for characters excluding the null
+     * terminating byte. */
     [[nodiscard]] size_t remainingLength() const
     {
         if (length() > capacity()) {
@@ -312,7 +336,9 @@ class String {
 
     Allocator* allocator() const
     {
-        return reinterpret_cast<Allocator*>(m_allocator & allocatorMask);
+        auto* allocPtr = reinterpret_cast<Allocator*>(m_allocator & allocatorMask);
+        fudAssert(allocPtr != nullptr);
+        return allocPtr;
     }
 
     [[nodiscard]] bool nullTerminated() const;
diff --git a/include/fud_vector.hpp b/include/fud_vector.hpp
index 52876fd..53b2625 100644
--- a/include/fud_vector.hpp
+++ b/include/fud_vector.hpp
@@ -39,6 +39,7 @@ class Vector {
 
   public:
     constexpr Vector() noexcept = default;
+    constexpr explicit Vector(Allocator& allocator) noexcept : m_allocator{&allocator} {}
     constexpr Vector(const Vector<T>& rhs) = delete;
     constexpr Vector(Vector<T>&& rhs) noexcept :
         m_allocator(rhs.m_allocator), m_data(rhs.m_data), m_length{rhs.m_length}, m_capacity{rhs.m_capacity}
@@ -121,12 +122,12 @@ class Vector {
         Vector<T> output{};
         auto status = initializeWithCapacity(output, count, allocator);
         if (status != FudStatus::Success) {
-            return status;
+            return FudError{status};
         }
-        return output;
+        return Okay<Vector<T>>{std::move(output)};
     }
 
-    static Result<Vector<T>, FudStatus> initializeWithSize(
+    static FudStatus initializeWithSize(
         Vector<T>& output,
         size_t count,
         Allocator* allocator = &globalFudAllocator)
@@ -145,7 +146,8 @@ class Vector {
             const auto* ptr = new (output.m_data + index) T();
             fudAssert(ptr != nullptr);
         }
-        return output;
+
+        return FudStatus::Success;
     }
 
     template <typename Builder>
@@ -157,20 +159,20 @@ class Vector {
         Vector<T> output{};
         auto status = initializeWithSizeFallible(output, count, std::forward<Builder>(builder), allocator);
         if (status != FudStatus::Success) {
-            return status;
+            return FudError{status};
         }
-        return output;
+        return Okay<Vector<T>>{std::move(output)};
     }
 
     template <typename Builder>
-    static Result<Vector<T>, FudStatus> initializeWithSizeFallible(
+    static FudStatus initializeWithSizeFallible(
         Vector<T>& output,
         size_t count,
         Builder&& builder,
         Allocator* allocator = &globalFudAllocator)
     {
-        using BuilderResult = decltype(std::forward<Builder>(builder)());
-        static_assert(std::is_same_v<BuilderResult, FudStatus>());
+        // using BuilderResult = decltype(std::forward<Builder>(builder)(T{}));
+        // static_assert(std::is_same_v<BuilderResult, FudStatus>);
 
         auto status = Vector::initializeWithCapacity(output, count, allocator);
         if (status != FudStatus::Success) {
@@ -180,13 +182,13 @@ class Vector {
         output.m_length = count;
 
         for (size_t index = 0; index < count; ++index) {
-            auto builderResult{std::forward<Builder>(builder)(output.m_data[index])};
-            if (builderResult.isError()) {
-                return builderResult.takeError();
+            auto builderStatus{std::forward<Builder>(builder)(output.m_data[index])};
+            if (builderStatus != FudStatus::Success) {
+                return builderStatus;
             }
         }
 
-        return output;
+        return FudStatus::Success;
     }
 
     static Result<Vector<T>, FudStatus> from(const Vector<T>& rhs, Option<Allocator*> allocatorOption = NullOpt)
@@ -356,7 +358,7 @@ class Vector {
 
         auto status = FudStatus::Success;
         if (m_capacity > 0) {
-            status = m_allocator->deallocate(reinterpret_cast<std::byte*>(m_data), m_capacity);
+            m_allocator->deallocate(reinterpret_cast<std::byte*>(m_data), m_capacity);
         }
 
         m_data = dataPtr;
@@ -672,10 +674,7 @@ class Vector {
         auto status = clear();
 
         if (m_data != nullptr && m_allocator != nullptr) {
-            auto deallocStatus = m_allocator->deallocate(reinterpret_cast<std::byte*>(m_data), m_capacity);
-            if (status == FudStatus::Success) {
-                status = deallocStatus;
-            }
+            m_allocator->deallocate(reinterpret_cast<std::byte*>(m_data), m_capacity);
         }
 
         m_allocator = nullptr;
diff --git a/source/fud_allocator.cpp b/source/fud_allocator.cpp
index d5127fa..3f58fb0 100644
--- a/source/fud_allocator.cpp
+++ b/source/fud_allocator.cpp
@@ -30,13 +30,12 @@ Result<std::byte*, FudStatus> FudAllocator::allocate(size_t bytes, size_t alignm
     return RetType::okay(pointer);
 }
 
-FudStatus FudAllocator::deallocate(std::byte* pointer, size_t bytes)
+void FudAllocator::deallocate(std::byte* pointer, size_t bytes)
 {
     if (pointer == nullptr || bytes == 0) {
-        return FudStatus::ArgumentInvalid;
+        return;
     }
     fudFree(pointer);
-    return FudStatus::Success;
 }
 
 bool FudAllocator::isEqual(const Allocator& rhs) const
@@ -50,14 +49,13 @@ Result<std::byte*, FudStatus> NullAllocator::allocate(size_t bytes, size_t align
 {
     static_cast<void>(bytes);
     static_cast<void>(alignment);
-    return FudError{FudStatus::Failure};
+    return FudError{FudStatus::AllocFailure};
 }
 
-FudStatus NullAllocator::deallocate(std::byte* pointer, size_t bytes)
+void NullAllocator::deallocate(std::byte* pointer, size_t bytes)
 {
     static_cast<void>(pointer);
     static_cast<void>(bytes);
-    return FudStatus::Failure;
 }
 
 bool NullAllocator::isEqual(const Allocator& rhs) const
diff --git a/source/fud_string.cpp b/source/fud_string.cpp
index 4a20630..4dddedb 100644
--- a/source/fud_string.cpp
+++ b/source/fud_string.cpp
@@ -23,7 +23,8 @@
 
 namespace fud {
 
-StringResult String::makeFromCString(const char8_t* cString) {
+StringResult String::makeFromCString(const char8_t* cString)
+{
     return makeFromCString(reinterpret_cast<const char*>(cString));
 }
 
@@ -85,14 +86,19 @@ StringResult String::makeFromCString(const char* cString, Allocator* allocator)
     return StringResult::okay(std::move(output));
 }
 
-StringResult String::from(const String& rhs)
+StringResult String::from(const String& rhs, Option<Allocator*> allocatorOption)
 {
     if (!rhs.valid()) {
         return StringResult::error(FudStatus::ArgumentInvalid);
     }
 
+    auto* allocator = allocatorOption.valueOr(rhs.allocator());
+    if (allocator == nullptr || !allocatorValid(allocator)) {
+        return StringResult::error(FudStatus::ArgumentInvalid);
+    }
+
     String output{};
-    output.m_allocator = rhs.m_allocator;
+    output.m_allocator = reinterpret_cast<uintptr_t>(allocator);
     utf8* outputData{nullptr};
     size_t outputCapacity{0};
     size_t outputLength{0};
@@ -154,12 +160,11 @@ StringResult String::from(StringView view, Allocator* allocator)
 
 String::String(String&& rhs) noexcept : m_allocator{rhs.m_allocator}, m_repr{std::move(rhs.m_repr)}
 {
-    if (isLarge()) {
-        rhs.m_repr.large.data = nullptr;
-    }
+    rhs.setSmall();
+    rhs.m_repr.small.length = 0;
 }
 
-String::~String()
+String::~String() noexcept
 {
     cleanup();
 }
@@ -184,7 +189,7 @@ FudStatus String::copy(const String& rhs)
     size_t outputLength{};
 
     if (isLarge()) {
-        auto allocResult = allocator()->allocate(m_repr.large.capacity);
+        auto allocResult = allocator()->allocate(m_repr.large.capacity, 1);
         if (allocResult.isError()) {
             return allocResult.takeError();
         }
@@ -223,6 +228,7 @@ String& String::operator=(String&& rhs) noexcept
     if (isLarge()) {
         rhs.m_repr.large.data = nullptr;
     }
+
     return *this;
 }
 
@@ -230,8 +236,7 @@ void String::cleanup()
 {
     const auto* allocPtr = allocator();
     if (isLarge() && m_repr.large.data != nullptr && allocPtr != nullptr) {
-        auto deallocStatus = allocator()->deallocate(reinterpret_cast<std::byte*>(m_repr.large.data), m_repr.large.capacity);
-        static_cast<void>(deallocStatus);
+        allocator()->deallocate(reinterpret_cast<std::byte*>(m_repr.large.data), m_repr.large.capacity);
         m_repr.large.data = nullptr;
     }
 }
@@ -258,16 +263,16 @@ FudStatus String::resize(size_t newCapacity)
         auto copyResult = copyMem(dataMut(), temp.size(), temp.data(), len);
         fudAssert(copyResult == FudStatus::Success);
 
-        auto deallocStatus = allocator()->deallocate(reinterpret_cast<std::byte*>(m_repr.large.data), m_repr.large.capacity);
+        allocator()->deallocate(reinterpret_cast<std::byte*>(m_repr.large.data), m_repr.large.capacity);
         setSmall();
         m_repr.small.length = len & smallStringLengthMask;
         copyMem(m_repr.small.buffer, temp);
         m_repr.small.buffer[len] = '\0';
 
-        return deallocStatus != FudStatus::Success ? FudStatus::DeallocFailure : FudStatus::Success;
+        return FudStatus::Success;
     }
 
-    auto allocResult = allocator()->allocate(newCapacity);
+    auto allocResult = allocator()->allocate(newCapacity, 1);
     if (allocResult.isError()) {
         return allocResult.takeError();
     }
@@ -277,9 +282,8 @@ FudStatus String::resize(size_t newCapacity)
     auto copyResult = copyMem(newData, newCapacity, data(), length());
     fudAssert(copyResult == FudStatus::Success);
 
-    auto deallocStatus = FudStatus::Success;
     if (isLarge()) {
-        deallocStatus = allocator()->deallocate(reinterpret_cast<std::byte*>(m_repr.large.data), m_repr.large.capacity);
+        allocator()->deallocate(reinterpret_cast<std::byte*>(m_repr.large.data), m_repr.large.capacity);
     }
 
     size_t len = length();
@@ -291,7 +295,7 @@ FudStatus String::resize(size_t newCapacity)
     m_repr.large.data[m_repr.large.length] = '\0';
     fudAssert(valid());
 
-    return deallocStatus != FudStatus::Success ? FudStatus::DeallocFailure : FudStatus::Success;
+    return FudStatus::Success;
 }
 
 bool String::nullTerminated() const
@@ -750,7 +754,7 @@ FudStatus String::makeLarge(size_t cap, size_t len, utf8*& outputData)
 {
     m_repr.large.capacity = cap;
     m_repr.large.length = len;
-    auto dataResult = allocator()->allocate(cap);
+    auto dataResult = allocator()->allocate(cap, 1);
     if (dataResult.isError()) {
         return dataResult.getError();
     }
diff --git a/test/test_allocator.cpp b/test/test_allocator.cpp
index 6382463..fa32a96 100644
--- a/test/test_allocator.cpp
+++ b/test/test_allocator.cpp
@@ -39,10 +39,10 @@ TEST(AllocatorTest, TestFudAllocator)
     ASSERT_TRUE(allocResult.isOkay());
     auto* mem = allocResult.getOkay();
     ASSERT_NE(mem, nullptr);
-    ASSERT_EQ(fudAllocator.deallocate(mem, 0), FudStatus::ArgumentInvalid);
-    ASSERT_EQ(fudAllocator.deallocate(mem, 1), FudStatus::Success);
+    fudAllocator.deallocate(mem, 0);
+    fudAllocator.deallocate(mem, 1);
     mem = nullptr;
-    ASSERT_EQ(fudAllocator.deallocate(mem, 1), FudStatus::ArgumentInvalid);
+    fudAllocator.deallocate(mem, 1);
 
     FailAllocator failAllocator{};
     globalMockFudAlloc.m_allocator = &failAllocator;
diff --git a/test/test_common.cpp b/test/test_common.cpp
index 778b4b5..07a0088 100644
--- a/test/test_common.cpp
+++ b/test/test_common.cpp
@@ -32,7 +32,7 @@ std::byte* MockFudAlloc::operator()(size_t size)
 
 void MockFudDealloc::operator()(std::byte* pointer)
 {
-    return free(pointer);
+    free(pointer);
 }
 
 MockFudAlloc globalDefaultMockAlloc{};
diff --git a/test/test_utf8.cpp b/test/test_utf8.cpp
index 69a1643..e5cb89a 100644
--- a/test/test_utf8.cpp
+++ b/test/test_utf8.cpp
@@ -147,11 +147,10 @@ TEST(Utf8Test, Utf8MultiByte)
             return RetType::okay(data);
         }
 
-        virtual FudStatus deallocate(std::byte* pointer, size_t bytes) override final
+        virtual void deallocate(std::byte* pointer, size_t bytes) override final
         {
             static_cast<void>(pointer);
             static_cast<void>(bytes);
-            return FudStatus::Success;
         }
 
         virtual bool isEqual(const Allocator& rhs) const override final
diff --git a/test/test_vector.cpp b/test/test_vector.cpp
index cadeaa6..ba0272e 100644
--- a/test/test_vector.cpp
+++ b/test/test_vector.cpp
@@ -16,11 +16,44 @@
  */
 
 #include "fud_vector.hpp"
+#include "fud_array.hpp"
 
 #include "gtest/gtest.h"
 
 namespace fud {
 
+template <size_t Size>
+struct TestLinearAllocator : public Allocator {
+    virtual ~TestLinearAllocator() override = default;
+
+    virtual Result<std::byte*, FudStatus> allocate(size_t bytes, size_t alignment = alignof(std::max_align_t)) override
+    {
+        auto allocIndex = m_next;
+        if (allocIndex % alignment != 0) {
+            allocIndex += alignment - allocIndex % alignment;
+        }
+        if ((allocIndex + bytes) > Size) {
+            return FudError{FudStatus::AllocFailure};
+        }
+        m_next = allocIndex + bytes;
+        return Okay<std::byte*>{m_backing.data() + allocIndex};
+    }
+
+    virtual void deallocate(std::byte* pointer, size_t bytes) override
+    {
+        static_cast<void>(pointer);
+        static_cast<void>(bytes);
+    }
+
+    virtual bool isEqual(const Allocator& rhs) const override {
+        return &rhs == static_cast<const Allocator*>(this);
+    }
+
+    Array<std::byte, Size> m_backing{Array<std::byte, Size>::constFill({})};
+    size_t m_next{0};
+};
+
+
 TEST(VectorTest, TrivialVector)
 {
     Vector<int> intVector{};
@@ -61,7 +94,8 @@ struct NonTrivial {
         }
     }
     NonTrivial& operator=(const NonTrivial& rhs) = delete;
-    NonTrivial& operator=(NonTrivial&& rhs) {
+    NonTrivial& operator=(NonTrivial&& rhs)
+    {
         value = rhs.value;
         destroyed = rhs.destroyed;
         rhs.destroyed = true;
@@ -75,9 +109,11 @@ int thread_local NonTrivial::counter = 0;
 
 TEST(VectorTest, NonTrivialVector)
 {
+    constexpr size_t testAllocSize = sizeof(NonTrivial) * 30;
+    TestLinearAllocator<testAllocSize> testLinearAllocator{};
     auto& counter = NonTrivial::counter;
     counter = 0;
-    Vector<NonTrivial> nonTrivialVector{};
+    Vector<NonTrivial> nonTrivialVector{testLinearAllocator};
     ASSERT_EQ(nonTrivialVector.size(), 0);
     ASSERT_EQ(nonTrivialVector.capacity(), 0);
     ASSERT_TRUE(nonTrivialVector.ref(0).isError());
@@ -114,7 +150,7 @@ TEST(VectorTest, NonTrivialVector)
     ASSERT_EQ(counter, 9);
 
     int val = 1;
-    for (auto& element: nonTrivialVector) {
+    for (auto& element : nonTrivialVector) {
         element.value = val;
         val++;
     }
@@ -136,4 +172,15 @@ TEST(VectorTest, NonTrivialVector)
     ASSERT_EQ(counter, nonTrivialVector.size());
 }
 
+TEST(VectorTest, NestedVector)
+{
+    struct FallibleObject {};
+    auto intVectorVectorResult{Vector<Vector<int>>::withSizeFallible(10, [](auto& vec) {
+        return Vector<int>::initializeWithSize(vec, 100, &globalNullAllocator);
+    })};
+    EXPECT_TRUE(intVectorVectorResult.isError());
+    EXPECT_EQ(intVectorVectorResult.getErrorOr(FudStatus::Success), FudStatus::AllocFailure);
+    // Result<Vector<Vector<FallibleObject>>, FudStatus>
+}
+
 } // namespace fud