Merge pull request 'Add an Option type and get rid of ENONE' (#19) from optionals-and-empty-errors into restart

Reviewed-on: #19

kernel/src/memory/Heap.cpp

@@ -94,14 +94,14 @@ static usize get_fair_offset_to_split_at(HeapBlock* block, usize min)
     return available + block->req_size;
 }
 
-static Result<HeapBlock*> split(HeapBlock* block, usize size)
+static Option<HeapBlock*> split(HeapBlock* block, usize size)
 {
     const usize available = space_available(block); // How much space can we steal from this block?
     const usize old_size =
         block->full_size; // Save the old value of this variable since we are going to use it after modifying it
 
     if (available < (size + sizeof(HeapBlock)))
-        return err(ENONE); // This block hasn't got enough free space to hold the requested size.
+        return {}; // This block hasn't got enough free space to hold the requested size.
 
     const usize offset = get_fair_offset_to_split_at(block, size + sizeof(HeapBlock));
     block->full_size = offset; // shrink the old block to fit this offset

kernel/src/thread/Scheduler.cpp

@@ -102,7 +102,7 @@ namespace Scheduler
         if (old->is_idle())
         {
             auto maybe_last = g_threads.last();
-            if (maybe_last.has_error()) // No threads!!
+            if (!maybe_last.has_value()) // No threads!!
                 return &g_idle;
             g_current = old = maybe_last.value();
         }
@@ -111,7 +111,7 @@ namespace Scheduler
 
         do {
             auto maybe_next = g_threads.next(g_current);
-            if (maybe_next.has_error()) g_current = g_threads.expect_first();
+            if (!maybe_next.has_value()) g_current = g_threads.expect_first();
             else
                 g_current = maybe_next.value();
 

luna/include/luna/LinkedList.h

@@ -1,9 +1,9 @@
 #pragma once
-#include <luna/Result.h>
+#include <luna/Option.h>
 
-template <typename T> inline Result<T*> nonnull_or_error(T* ptr)
+template <typename T> inline Option<T*> nonnull_or_error(T* ptr)
 {
-    if (ptr == nullptr) return err(ENONE);
+    if (ptr == nullptr) return {};
     else
         return ptr;
 }
@@ -101,7 +101,7 @@ template <typename T> class DoublyLinkedList
         return ptr;
     }
 
-    Result<T*> first()
+    Option<T*> first()
     {
         return nonnull_or_error((T*)m_start_node);
     }
@@ -111,7 +111,7 @@ template <typename T> class DoublyLinkedList
         return first().value();
     }
 
-    Result<T*> last()
+    Option<T*> last()
     {
         return nonnull_or_error((T*)m_end_node);
     }
@@ -121,12 +121,12 @@ template <typename T> class DoublyLinkedList
         return last().value();
     }
 
-    Result<T*> next(T* item)
+    Option<T*> next(T* item)
     {
         return nonnull_or_error((T*)extract_node(item)->get_next());
     }
 
-    Result<T*> previous(T* item)
+    Option<T*> previous(T* item)
     {
         return nonnull_or_error((T*)extract_node(item)->get_last());
     }

luna/include/luna/Option.h

@@ -0,0 +1,139 @@
+#pragma once
+#include <luna/Check.h>
+#include <luna/Move.h>
+#include <luna/PlacementNew.h>
+
+template <typename T> class Option
+{
+  public:
+    Option(const T& value)
+    {
+        m_storage.store_reference(value);
+        m_has_value = true;
+    }
+
+    Option(T&& value)
+    {
+        m_storage.store_moved_reference(move(value));
+        m_has_value = true;
+    }
+
+    Option(const Option<T>& other)
+    {
+        m_has_value = other.has_value();
+        if (m_has_value) { m_storage.store_reference(other.m_storage.fetch_reference()); }
+    }
+
+    Option(Option<T>&& other)
+    {
+        m_has_value = other.has_value();
+        other.m_has_value = false;
+        if (m_has_value) { m_storage.store_moved_reference(move(other.m_storage.fetch_reference())); }
+    }
+
+    Option()
+    {
+        m_has_value = false;
+    }
+
+    bool has_value() const
+    {
+        return m_has_value;
+    }
+
+    T value() const
+    {
+        expect(has_value(), "Option::value called on an empty Option");
+        return m_storage.fetch_reference();
+    }
+
+    T release_value()
+    {
+        expect(has_value(), "Option::release_value called on an empty Option");
+        T item = m_storage.fetch_reference();
+        m_has_value = false;
+        m_storage.destroy();
+        return move(item);
+    }
+
+    T value_or(const T& other) const
+    {
+        if (has_value()) return m_storage.fetch_reference();
+        return other;
+    }
+
+    bool try_set_value(T& ref) const
+    {
+        if (!has_value()) return false;
+        ref = m_storage.fetch_reference();
+        return true;
+    }
+
+    ~Option()
+    {
+        if (has_value()) m_storage.destroy();
+    }
+
+    struct ErrorHandle
+    {
+      private:
+        explicit ErrorHandle()
+        {
+        }
+    };
+
+    Option(ErrorHandle)
+    {
+        m_has_value = false;
+    }
+
+    // For compatibility with TRY()
+    ErrorHandle release_error()
+    {
+        expect(!has_value(), "Option::release_error called on a non-empty Option");
+        return ErrorHandle{};
+    }
+
+  private:
+    struct Storage
+    {
+        u8 buffer[sizeof(T)];
+
+        T* fetch_ptr()
+        {
+            return (T*)buffer;
+        }
+
+        T& fetch_reference()
+        {
+            return *fetch_ptr();
+        }
+
+        const T* fetch_ptr() const
+        {
+            return (const T*)buffer;
+        }
+
+        const T& fetch_reference() const
+        {
+            return *fetch_ptr();
+        }
+
+        void store_reference(const T& ref)
+        {
+            new (buffer) T(ref);
+        }
+
+        void store_moved_reference(T&& ref)
+        {
+            new (buffer) T(ref);
+        }
+
+        void destroy()
+        {
+            fetch_reference().~T();
+        }
+    };
+    Storage m_storage;
+    bool m_has_value{false};
+};

luna/include/luna/Result.h

@@ -1,6 +1,7 @@
 #pragma once
 #include <luna/Check.h>
 #include <luna/Move.h>
+#include <luna/Option.h>
 #include <luna/PlacementNew.h>
 #include <luna/SystemError.h>
 #include <luna/Types.h>
@@ -18,25 +19,22 @@ struct Error
 template <typename T> class Result
 {
   public:
-    Result(const T& value)
+    Result(const T& value) : m_value(value)
     {
-        m_storage.store_reference(value);
         m_has_value = true;
         m_has_error = false;
     }
 
-    Result(T&& value)
+    Result(T&& value) : m_value(value)
     {
-        m_storage.store_movable_reference(move(value));
         m_has_value = true;
         m_has_error = false;
     }
 
-    Result(const Result<T>& other)
+    Result(const Result<T>& other) : m_value(other.m_value)
     {
         if (!other.m_has_error)
         {
-            m_storage.store_reference(other.m_storage.fetch_reference());
             m_has_value = true;
             m_has_error = false;
         }
@@ -48,11 +46,10 @@ template <typename T> class Result
         }
     }
 
-    Result(Result<T>&& other)
+    Result(Result<T>&& other) : m_value(other.m_value)
     {
         if (!other.m_has_error)
         {
-            m_storage.store_movable_reference(move(other.m_storage.fetch_reference()));
             m_has_value = true;
             m_has_error = false;
         }
@@ -64,7 +61,7 @@ template <typename T> class Result
         }
     }
 
-    Result(const Error& err)
+    Result(const Error& err) : m_value()
     {
         m_error = err.error;
         m_has_error = true;
@@ -102,98 +99,39 @@ template <typename T> class Result
     T value()
     {
         expect(has_value(), "Result::value() called on a Result that holds an error");
-        return m_storage.fetch_reference();
+        return m_value.value();
     }
 
     T expect_value(const char* reason)
     {
         expect(has_value(), reason);
-        return m_storage.fetch_reference();
+        return move(m_value.release_value());
     }
 
-    T value_or(T other)
+    T value_or(const T& other)
     {
-        if (has_value()) return m_storage.fetch_reference();
+        return m_value.value_or(other);
-        return other;
     }
 
     bool try_set_value(T& ref)
     {
-        if (!has_value()) return false;
+        return m_value.try_set_value(ref);
-        ref = m_storage.fetch_reference();
-        return true;
     }
 
     T release_value()
     {
         expect(has_value(), "Result::release_value() called on a Result that holds an error");
-        T item = m_storage.fetch_reference();
+        return m_value.release_value();
-        m_has_value = false;
-        m_storage.destroy();
-        return move(item);
     }
 
     T expect_release_value(const char* reason)
     {
         expect(has_value(), reason);
-        T item = m_storage.fetch_reference();
+        return m_value.release_value();
-        m_has_value = false;
-        m_storage.destroy();
-        return move(item);
-    }
-
-    ~Result()
-    {
-        if (has_value()) m_storage.destroy();
     }
 
   private:
-    struct Storage
+    Option<T> m_value;
-    {
-        u8 buffer[sizeof(T)];
-
-        T* fetch_ptr()
-        {
-            return (T*)buffer;
-        }
-
-        T& fetch_reference()
-        {
-            return *fetch_ptr();
-        }
-
-        const T* fetch_ptr() const
-        {
-            return (const T*)buffer;
-        }
-
-        const T& fetch_reference() const
-        {
-            return *fetch_ptr();
-        }
-
-        void store_ptr(T* ptr)
-        {
-            new (buffer) T(*ptr);
-        }
-
-        void store_reference(const T& ref)
-        {
-            new (buffer) T(ref);
-        }
-
-        void store_movable_reference(T&& ref)
-        {
-            new (buffer) T(ref);
-        }
-
-        void destroy()
-        {
-            fetch_reference().~T();
-        }
-    };
-
-    Storage m_storage;
     int m_error;
     bool m_has_error;
     bool m_has_value;
@@ -289,6 +227,6 @@ template <> class Result<void>
 #define TRY(expr)                                                                                                      \
     ({                                                                                                                 \
         auto _expr_rc = (expr);                                                                                        \
-        if (_expr_rc.has_error()) return _expr_rc.release_error();                                                     \
+        if (!_expr_rc.has_value()) return _expr_rc.release_error();                                                    \
         _expr_rc.expect_release_value("sanity check failed: has_error() returned false, yet result has no value");     \
     })

luna/include/luna/SystemError.h

@@ -52,8 +52,7 @@
 #define ETIMEDOUT 110   // Connection timed out
 #define EALREADY 114    // Operation already in progress
 
-// These ones are Luna-specific.
+// This one is Luna-specific.
 #define EFIXME 342 // Functionality not yet implemented
-#define ENONE 343  // Internal or insignificant error
 
 const char* error_string(int error);

luna/src/SystemError.cpp

@@ -55,7 +55,6 @@ const char* error_string(int error)
     case ETIMEDOUT: return "Connection timed out";
     case EALREADY: return "Operation already in progress";
     case EFIXME: return "Functionality not yet implemented";
-    case ENONE: return "Internal or insignificant error";
     default: return "Unknown error";
     }
 }