AK+Kernel: Make IntrusiveList capable of holding non-raw pointers

This should allow creating intrusive lists that have smart pointers,
while remaining free (compared to the impl before this commit) when
holding raw pointers :^)
As a sidenote, this also adds a `RawPtr<T>` type, which is just
equivalent to `T*`.
Note that this does not actually use such functionality, but is only
expected to pave the way for #6369, to replace NonnullRefPtrVector<T>
with intrusive lists.

As it is with zero-cost things, this makes the interface a bit less nice
by requiring the type name of what an `IntrusiveListNode` holds (and
optionally its container, if not RawPtr), and also requiring the type of
the container (normally `RawPtr`) on the `IntrusiveList` instance.

AK/IntrusiveList.h

@@ -28,20 +28,25 @@
 
 #include <AK/Assertions.h>
 #include <AK/BitCast.h>
+#include <AK/Forward.h>
+#include <AK/StdLibExtras.h>
 
 namespace AK {
 
+template<typename T, typename Container = RawPtr<T>>
 class IntrusiveListNode;
+
+template<typename T, typename Container>
 class IntrusiveListStorage {
 private:
-    friend class IntrusiveListNode;
-    template<class T, IntrusiveListNode T::*member>
+    friend class IntrusiveListNode<T, Container>;
+    template<class T_, typename Container_, IntrusiveListNode<T_, Container_> T_::*member>
     friend class IntrusiveList;
-    IntrusiveListNode* m_first { nullptr };
-    IntrusiveListNode* m_last { nullptr };
+    IntrusiveListNode<T, Container>* m_first { nullptr };
+    IntrusiveListNode<T, Container>* m_last { nullptr };
 };
 
-template<class T, IntrusiveListNode T::*member>
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
 class IntrusiveList {
 public:
     IntrusiveList();
@@ -53,27 +58,29 @@ public:
     void prepend(T& n);
     void remove(T& n);
     [[nodiscard]] bool contains(const T&) const;
-    [[nodiscard]] T* first() const;
-    [[nodiscard]] T* last() const;
+    [[nodiscard]] Container first() const;
+    [[nodiscard]] Container last() const;
 
-    [[nodiscard]] T* take_first();
-    [[nodiscard]] T* take_last();
+    [[nodiscard]] Container take_first();
+    [[nodiscard]] Container take_last();
 
     class Iterator {
     public:
         Iterator() = default;
         Iterator(T* value)
-            : m_value(value)
+            : m_value(move(value))
         {
         }
 
-        T& operator*() const { return *m_value; }
-        T* operator->() const { return m_value; }
+        const T& operator*() const { return *m_value; }
+        auto operator->() const { return m_value; }
+        T& operator*() { return *m_value; }
+        auto operator->() { return m_value; }
         bool operator==(const Iterator& other) const { return other.m_value == m_value; }
         bool operator!=(const Iterator& other) const { return !(*this == other); }
         Iterator& operator++()
         {
-            m_value = IntrusiveList<T, member>::next(m_value);
+            m_value = IntrusiveList<T, Container, member>::next(m_value);
             return *this;
         }
         Iterator& erase();
@@ -94,12 +101,12 @@ public:
         }
 
         const T& operator*() const { return *m_value; }
-        const T* operator->() const { return m_value; }
+        auto operator->() const { return m_value; }
         bool operator==(const ConstIterator& other) const { return other.m_value == m_value; }
         bool operator!=(const ConstIterator& other) const { return !(*this == other); }
         ConstIterator& operator++()
         {
-            m_value = IntrusiveList<T, member>::next(const_cast<T*>(m_value));
+            m_value = IntrusiveList<T, Container, member>::next(m_value);
             return *this;
         }
 
@@ -112,68 +119,81 @@ public:
 
 private:
     static T* next(T* current);
-    static T* node_to_value(IntrusiveListNode& node);
-    IntrusiveListStorage m_storage;
+    static const T* next(const T* current);
+    static T* node_to_value(IntrusiveListNode<T, Container>& node);
+    IntrusiveListStorage<T, Container> m_storage;
 };
 
+template<typename Contained, bool _IsRaw>
+struct SelfReferenceIfNeeded {
+    Contained reference = nullptr;
+};
+template<typename Contained>
+struct SelfReferenceIfNeeded<Contained, true> {
+};
+
+template<typename T, typename Container>
 class IntrusiveListNode {
 public:
     ~IntrusiveListNode();
     void remove();
     bool is_in_list() const;
 
+    static constexpr bool IsRaw = IsPointer<Container>;
+
 private:
-    template<class T, IntrusiveListNode T::*member>
+    template<class T_, typename Container_, IntrusiveListNode<T_, Container_> T_::*member>
     friend class IntrusiveList;
-    IntrusiveListStorage* m_storage = nullptr;
-    IntrusiveListNode* m_next = nullptr;
-    IntrusiveListNode* m_prev = nullptr;
+    IntrusiveListStorage<T, Container>* m_storage = nullptr;
+    IntrusiveListNode<T, Container>* m_next = nullptr;
+    IntrusiveListNode<T, Container>* m_prev = nullptr;
+    SelfReferenceIfNeeded<Container, IsRaw> m_self;
 };
 
-template<class T, IntrusiveListNode T::*member>
-inline typename IntrusiveList<T, member>::Iterator& IntrusiveList<T, member>::Iterator::erase()
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline typename IntrusiveList<T, Container, member>::Iterator& IntrusiveList<T, Container, member>::Iterator::erase()
 {
-    T* old = m_value;
-    m_value = IntrusiveList<T, member>::next(m_value);
+    auto old = m_value;
+    m_value = IntrusiveList<T, Container, member>::next(m_value);
     (old->*member).remove();
     return *this;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline IntrusiveList<T, member>::IntrusiveList()
-
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline IntrusiveList<T, Container, member>::IntrusiveList()
 {
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline IntrusiveList<T, member>::~IntrusiveList()
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline IntrusiveList<T, Container, member>::~IntrusiveList()
 {
     clear();
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline void IntrusiveList<T, member>::clear()
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline void IntrusiveList<T, Container, member>::clear()
 {
     while (m_storage.m_first)
         m_storage.m_first->remove();
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline bool IntrusiveList<T, member>::is_empty() const
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline bool IntrusiveList<T, Container, member>::is_empty() const
 {
     return m_storage.m_first == nullptr;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline void IntrusiveList<T, member>::append(T& n)
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline void IntrusiveList<T, Container, member>::append(T& n)
 {
-    auto& nnode = n.*member;
-    if (nnode.m_storage)
-        nnode.remove();
+    remove(n);
 
+    auto& nnode = n.*member;
     nnode.m_storage = &m_storage;
     nnode.m_prev = m_storage.m_last;
     nnode.m_next = nullptr;
+    if constexpr (!RemoveReference<decltype(nnode)>::IsRaw)
+        nnode.m_self.reference = &n; // Note: Self-reference ensures that the object will keep a ref to itself when the Container is a smart pointer.
 
     if (m_storage.m_last)
         m_storage.m_last->m_next = &nnode;
@@ -182,8 +202,8 @@ inline void IntrusiveList<T, member>::append(T& n)
         m_storage.m_first = &nnode;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline void IntrusiveList<T, member>::prepend(T& n)
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline void IntrusiveList<T, Container, member>::prepend(T& n)
 {
     auto& nnode = n.*member;
     if (nnode.m_storage)
@@ -192,6 +212,8 @@ inline void IntrusiveList<T, member>::prepend(T& n)
     nnode.m_storage = &m_storage;
     nnode.m_prev = nullptr;
     nnode.m_next = m_storage.m_first;
+    if constexpr (!RemoveReference<decltype(nnode)>::IsRaw)
+        nnode.m_self.reference = &n;
 
     if (m_storage.m_first)
         m_storage.m_first->m_prev = &nnode;
@@ -200,29 +222,29 @@ inline void IntrusiveList<T, member>::prepend(T& n)
         m_storage.m_last = &nnode;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline void IntrusiveList<T, member>::remove(T& n)
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline void IntrusiveList<T, Container, member>::remove(T& n)
 {
     auto& nnode = n.*member;
     if (nnode.m_storage)
         nnode.remove();
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline bool IntrusiveList<T, member>::contains(const T& n) const
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline bool IntrusiveList<T, Container, member>::contains(const T& n) const
 {
     auto& nnode = n.*member;
     return nnode.m_storage == &m_storage;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline T* IntrusiveList<T, member>::first() const
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline Container IntrusiveList<T, Container, member>::first() const
 {
     return m_storage.m_first ? node_to_value(*m_storage.m_first) : nullptr;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline T* IntrusiveList<T, member>::take_first()
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline Container IntrusiveList<T, Container, member>::take_first()
 {
     if (auto* ptr = first()) {
         remove(*ptr);
@@ -231,8 +253,8 @@ inline T* IntrusiveList<T, member>::take_first()
     return nullptr;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline T* IntrusiveList<T, member>::take_last()
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline Container IntrusiveList<T, Container, member>::take_last()
 {
     if (auto* ptr = last()) {
         remove(*ptr);
@@ -241,34 +263,42 @@ inline T* IntrusiveList<T, member>::take_last()
     return nullptr;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline T* IntrusiveList<T, member>::last() const
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline Container IntrusiveList<T, Container, member>::last() const
 {
     return m_storage.m_last ? node_to_value(*m_storage.m_last) : nullptr;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline T* IntrusiveList<T, member>::next(T* current)
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline const T* IntrusiveList<T, Container, member>::next(const T* current)
+{
+    auto& nextnode = (current->*member).m_next;
+    const T* nextstruct = nextnode ? node_to_value(*nextnode) : nullptr;
+    return nextstruct;
+}
+
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline T* IntrusiveList<T, Container, member>::next(T* current)
 {
     auto& nextnode = (current->*member).m_next;
     T* nextstruct = nextnode ? node_to_value(*nextnode) : nullptr;
     return nextstruct;
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline typename IntrusiveList<T, member>::Iterator IntrusiveList<T, member>::begin()
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline typename IntrusiveList<T, Container, member>::Iterator IntrusiveList<T, Container, member>::begin()
 {
     return m_storage.m_first ? Iterator(node_to_value(*m_storage.m_first)) : Iterator();
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline typename IntrusiveList<T, member>::ConstIterator IntrusiveList<T, member>::begin() const
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline typename IntrusiveList<T, Container, member>::ConstIterator IntrusiveList<T, Container, member>::begin() const
 {
     return m_storage.m_first ? ConstIterator(node_to_value(*m_storage.m_first)) : ConstIterator();
 }
 
-template<class T, IntrusiveListNode T::*member>
-inline T* IntrusiveList<T, member>::node_to_value(IntrusiveListNode& node)
+template<class T, typename Container, IntrusiveListNode<T, Container> T::*member>
+inline T* IntrusiveList<T, Container, member>::node_to_value(IntrusiveListNode<T, Container>& node)
 {
     // Note: Since this might seem odd, here's an explanation on what this function actually does:
     //       `node` is a reference that resides in some part of the actual value (of type T), the
@@ -279,13 +309,15 @@ inline T* IntrusiveList<T, member>::node_to_value(IntrusiveListNode& node)
     return bit_cast<T*>(bit_cast<unsigned char*>(&node) - bit_cast<unsigned char*>(member));
 }
 
-inline IntrusiveListNode::~IntrusiveListNode()
+template<typename T, typename Container>
+inline IntrusiveListNode<T, Container>::~IntrusiveListNode()
 {
     if (m_storage)
         remove();
 }
 
-inline void IntrusiveListNode::remove()
+template<typename T, typename Container>
+inline void IntrusiveListNode<T, Container>::remove()
 {
     VERIFY(m_storage);
     if (m_storage->m_first == this)
@@ -299,13 +331,33 @@ inline void IntrusiveListNode::remove()
     m_prev = nullptr;
     m_next = nullptr;
     m_storage = nullptr;
+    if constexpr (!IsRaw)
+        m_self.reference = nullptr;
 }
 
-inline bool IntrusiveListNode::is_in_list() const
+template<typename T, typename Container>
+inline bool IntrusiveListNode<T, Container>::is_in_list() const
 {
     return m_storage != nullptr;
 }
 
+// Specialise IntrusiveList(Node) for NonnullRefPtr
+// By default, intrusive lists cannot contain null entries anyway, so switch to RefPtr
+// and just make the user-facing functions deref the pointers.
+template<typename T>
+struct IntrusiveListNode<T, NonnullRefPtr<T>> : public IntrusiveListNode<T, RefPtr<T>> {
+};
+
+template<class T, IntrusiveListNode<T, NonnullRefPtr<T>> T::*member>
+class IntrusiveList<T, NonnullRefPtr<T>, member> : public IntrusiveList<T, RefPtr<T>, member> {
+public:
+    [[nodiscard]] NonnullRefPtr<T> first() const { return *IntrusiveList<T, RefPtr<T>, member>::first(); }
+    [[nodiscard]] NonnullRefPtr<T> last() const { return *IntrusiveList<T, RefPtr<T>, member>::last(); }
+
+    [[nodiscard]] NonnullRefPtr<T> take_first() { return *IntrusiveList<T, RefPtr<T>, member>::take_first(); }
+    [[nodiscard]] NonnullRefPtr<T> take_last() { return *IntrusiveList<T, RefPtr<T>, member>::take_last(); }
+};
+
 }
 
 using AK::IntrusiveList;

AK/StdLibExtras.h

@@ -49,6 +49,13 @@ constexpr T&& move(T& arg)
 
 using std::move;
 
+namespace AK::Detail {
+template<typename T>
+struct _RawPtr {
+    using Type = T*;
+};
+}
+
 namespace AK {
 
 template<typename T>
@@ -122,6 +129,9 @@ constexpr T exchange(T& slot, U&& value)
     return old_value;
 }
 
+template<typename T>
+using RawPtr = typename Detail::_RawPtr<T>::Type;
+
 }
 
 using AK::array_size;
@@ -132,4 +142,5 @@ using AK::exchange;
 using AK::forward;
 using AK::max;
 using AK::min;
+using AK::RawPtr;
 using AK::swap;

Kernel/FileSystem/BlockBasedFileSystem.cpp

@@ -32,7 +32,7 @@
 namespace Kernel {
 
 struct CacheEntry {
-    IntrusiveListNode list_node;
+    IntrusiveListNode<CacheEntry> list_node;
     BlockBasedFS::BlockIndex block_index { 0 };
     u8* data { nullptr };
     bool has_data { false };
@@ -117,8 +117,8 @@ private:
     BlockBasedFS& m_fs;
     size_t m_entry_count { 10000 };
     mutable HashMap<BlockBasedFS::BlockIndex, CacheEntry*> m_hash;
-    mutable IntrusiveList<CacheEntry, &CacheEntry::list_node> m_clean_list;
-    mutable IntrusiveList<CacheEntry, &CacheEntry::list_node> m_dirty_list;
+    mutable IntrusiveList<CacheEntry, RawPtr<CacheEntry>, &CacheEntry::list_node> m_clean_list;
+    mutable IntrusiveList<CacheEntry, RawPtr<CacheEntry>, &CacheEntry::list_node> m_dirty_list;
     KBuffer m_cached_block_data;
     KBuffer m_entries;
     bool m_dirty { false };

Kernel/Process.h

@@ -120,7 +120,7 @@ protected:
     mode_t m_umask { 022 };
     VirtualAddress m_signal_trampoline;
     Atomic<u32> m_thread_count { 0 };
-    IntrusiveList<Thread, &Thread::m_process_thread_list_node> m_thread_list;
+    IntrusiveList<Thread, RawPtr<Thread>, &Thread::m_process_thread_list_node> m_thread_list;
     u8 m_termination_status { 0 };
     u8 m_termination_signal { 0 };
 };

Kernel/Scheduler.cpp

@@ -73,7 +73,7 @@ Atomic<bool> g_finalizer_has_work { false };
 READONLY_AFTER_INIT static Process* s_colonel_process;
 
 struct ThreadReadyQueue {
-    IntrusiveList<Thread, &Thread::m_ready_queue_node> thread_list;
+    IntrusiveList<Thread, RawPtr<Thread>, &Thread::m_ready_queue_node> thread_list;
 };
 static SpinLock<u8> g_ready_queues_lock;
 static u32 g_ready_queues_mask;

Kernel/Thread.h

@@ -88,7 +88,7 @@ class Thread
     friend class Process;
     friend class ProtectedProcessBase;
     friend class Scheduler;
-    friend class ThreadReadyQueue;
+    friend struct ThreadReadyQueue;
 
     static SpinLock<u8> g_tid_map_lock;
     static HashMap<ThreadID, Thread*>* g_tid_map;
@@ -1129,7 +1129,7 @@ public:
 private:
     Thread(NonnullRefPtr<Process>, NonnullOwnPtr<Region> kernel_stack_region);
 
-    IntrusiveListNode m_process_thread_list_node;
+    IntrusiveListNode<Thread> m_process_thread_list_node;
     int m_runnable_priority { -1 };
 
     friend class WaitQueue;
@@ -1202,7 +1202,7 @@ private:
     TSS m_tss {};
     TrapFrame* m_current_trap { nullptr };
     u32 m_saved_critical { 1 };
-    IntrusiveListNode m_ready_queue_node;
+    IntrusiveListNode<Thread> m_ready_queue_node;
     Atomic<u32> m_cpu { 0 };
     u32 m_cpu_affinity { THREAD_AFFINITY_DEFAULT };
     u32 m_ticks_left { 0 };

Kernel/WorkQueue.h

@@ -75,7 +75,7 @@ public:
 
 private:
     struct WorkItem {
-        IntrusiveListNode m_node;
+        IntrusiveListNode<WorkItem> m_node;
         void (*function)(void*);
         void* data;
         void (*free_data)(void*);
@@ -86,7 +86,7 @@ private:
 
     RefPtr<Thread> m_thread;
     WaitQueue m_wait_queue;
-    IntrusiveList<WorkItem, &WorkItem::m_node> m_items;
+    IntrusiveList<WorkItem, RawPtr<WorkItem>, &WorkItem::m_node> m_items;
     SpinLock<u8> m_lock;
 };
 

Userland/Libraries/LibCore/Object.cpp

@@ -34,9 +34,9 @@
 
 namespace Core {
 
-IntrusiveList<Object, &Object::m_all_objects_list_node>& Object::all_objects()
+IntrusiveList<Object, RawPtr<Object>, &Object::m_all_objects_list_node>& Object::all_objects()
 {
-    static IntrusiveList<Object, &Object::m_all_objects_list_node> objects;
+    static IntrusiveList<Object, RawPtr<Object>, &Object::m_all_objects_list_node> objects;
     return objects;
 }
 

Userland/Libraries/LibCore/Object.h

@@ -67,7 +67,7 @@ class Object
     AK_MAKE_NONCOPYABLE(Object);
     AK_MAKE_NONMOVABLE(Object);
 
-    IntrusiveListNode m_all_objects_list_node;
+    IntrusiveListNode<Object> m_all_objects_list_node;
 
 public:
     virtual ~Object();
@@ -124,7 +124,7 @@ public:
     JsonValue property(const StringView& name) const;
     const HashMap<String, NonnullOwnPtr<Property>>& properties() const { return m_properties; }
 
-    static IntrusiveList<Object, &Object::m_all_objects_list_node>& all_objects();
+    static IntrusiveList<Object, RawPtr<Object>, &Object::m_all_objects_list_node>& all_objects();
 
     void dispatch_event(Core::Event&, Object* stay_within = nullptr);
 

Userland/Libraries/LibJS/Heap/Allocator.h

@@ -63,7 +63,7 @@ public:
 private:
     const size_t m_cell_size;
 
-    typedef IntrusiveList<HeapBlock, &HeapBlock::m_list_node> BlockList;
+    typedef IntrusiveList<HeapBlock, RawPtr<HeapBlock>, &HeapBlock::m_list_node> BlockList;
     BlockList m_full_blocks;
     BlockList m_usable_blocks;
 };

Userland/Libraries/LibJS/Heap/HeapBlock.h

@@ -86,7 +86,7 @@ public:
         return cell_from_possible_pointer((FlatPtr)cell);
     }
 
-    IntrusiveListNode m_list_node;
+    IntrusiveListNode<HeapBlock> m_list_node;
 
 private:
     HeapBlock(Heap&, size_t cell_size);