Bladeren bron

LibJS: Resolve a circular include problem between HeapBlock and Cell

Cell::heap() and Cell::vm() needed to access member functions from
HeapBlock, and wanted to be inline, so they were moved to VM.h.
That approach will no longer work with VM.h not being included in every
file (starting from the next commit), so this commit fixes that circular
import issue by introducing secondary base classes to host the
references to Heap and VM, respectively.
Ali Mohammad Pur 2 jaren geleden
bovenliggende
commit
392b5c3b19

+ 3 - 2
Userland/Libraries/LibJS/Heap/Cell.h

@@ -13,6 +13,7 @@
 #include <AK/StringView.h>
 #include <LibJS/Forward.h>
 #include <LibJS/Heap/GCPtr.h>
+#include <LibJS/Heap/Internals.h>
 #include <LibJS/Runtime/Completion.h>
 #include <LibJS/Runtime/Value.h>
 
@@ -105,8 +106,8 @@ public:
 
     bool overrides_must_survive_garbage_collection(Badge<Heap>) const { return m_overrides_must_survive_garbage_collection; }
 
-    Heap& heap() const;
-    VM& vm() const;
+    ALWAYS_INLINE Heap& heap() const { return HeapBlockBase::from_cell(this)->heap(); }
+    ALWAYS_INLINE VM& vm() const { return bit_cast<HeapBase*>(&heap())->vm(); }
 
 protected:
     Cell() = default;

+ 1 - 1
Userland/Libraries/LibJS/Heap/Heap.cpp

@@ -39,7 +39,7 @@ static int gc_perf_string_id;
 static __thread HashMap<FlatPtr*, size_t>* s_custom_ranges_for_conservative_scan = nullptr;
 
 Heap::Heap(VM& vm)
-    : m_vm(vm)
+    : HeapBase(vm)
 {
 #ifdef AK_OS_SERENITY
     auto gc_signpost_string = "Garbage collection"sv;

+ 2 - 5
Userland/Libraries/LibJS/Heap/Heap.h

@@ -19,12 +19,13 @@
 #include <LibJS/Heap/Cell.h>
 #include <LibJS/Heap/CellAllocator.h>
 #include <LibJS/Heap/Handle.h>
+#include <LibJS/Heap/Internals.h>
 #include <LibJS/Heap/MarkedVector.h>
 #include <LibJS/Runtime/WeakContainer.h>
 
 namespace JS {
 
-class Heap {
+class Heap : public HeapBase {
     AK_MAKE_NONCOPYABLE(Heap);
     AK_MAKE_NONMOVABLE(Heap);
 
@@ -57,8 +58,6 @@ public:
 
     void collect_garbage(CollectionType = CollectionType::CollectGarbage, bool print_report = false);
 
-    VM& vm() { return m_vm; }
-
     bool should_collect_on_every_allocation() const { return m_should_collect_on_every_allocation; }
     void set_should_collect_on_every_allocation(bool b) { m_should_collect_on_every_allocation = b; }
 
@@ -106,8 +105,6 @@ private:
 
     bool m_should_collect_on_every_allocation { false };
 
-    VM& m_vm;
-
     Vector<NonnullOwnPtr<CellAllocator>> m_allocators;
 
     HandleImpl::List m_handles;

+ 1 - 1
Userland/Libraries/LibJS/Heap/HeapBlock.cpp

@@ -32,7 +32,7 @@ NonnullOwnPtr<HeapBlock> HeapBlock::create_with_cell_size(Heap& heap, size_t cel
 }
 
 HeapBlock::HeapBlock(Heap& heap, size_t cell_size)
-    : m_heap(heap)
+    : HeapBlockBase(heap)
     , m_cell_size(cell_size)
 {
     VERIFY(cell_size >= sizeof(FreelistEntry));

+ 4 - 6
Userland/Libraries/LibJS/Heap/HeapBlock.h

@@ -12,6 +12,7 @@
 #include <AK/Types.h>
 #include <LibJS/Forward.h>
 #include <LibJS/Heap/Cell.h>
+#include <LibJS/Heap/Internals.h>
 
 #ifdef HAS_ADDRESS_SANITIZER
 #    include <sanitizer/asan_interface.h>
@@ -19,12 +20,12 @@
 
 namespace JS {
 
-class HeapBlock {
+class HeapBlock : public HeapBlockBase {
     AK_MAKE_NONCOPYABLE(HeapBlock);
     AK_MAKE_NONMOVABLE(HeapBlock);
 
 public:
-    static constexpr size_t block_size = 16 * KiB;
+    using HeapBlockBase::block_size;
     static NonnullOwnPtr<HeapBlock> create_with_cell_size(Heap&, size_t);
 
     size_t cell_size() const { return m_cell_size; }
@@ -66,11 +67,9 @@ public:
         });
     }
 
-    Heap& heap() { return m_heap; }
-
     static HeapBlock* from_cell(Cell const* cell)
     {
-        return reinterpret_cast<HeapBlock*>((FlatPtr)cell & ~(block_size - 1));
+        return static_cast<HeapBlock*>(HeapBlockBase::from_cell(cell));
     }
 
     Cell* cell_from_possible_pointer(FlatPtr pointer)
@@ -107,7 +106,6 @@ private:
         return reinterpret_cast<Cell*>(&m_storage[index * cell_size()]);
     }
 
-    Heap& m_heap;
     size_t m_cell_size { 0 };
     size_t m_next_lazy_freelist_index { 0 };
     GCPtr<FreelistEntry> m_freelist;

+ 53 - 0
Userland/Libraries/LibJS/Heap/Internals.h

@@ -0,0 +1,53 @@
+/*
+ * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2020-2023, the SerenityOS developers.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#pragma once
+
+#include <AK/Types.h>
+#include <LibJS/Forward.h>
+
+namespace JS {
+
+class HeapBase {
+    AK_MAKE_NONCOPYABLE(HeapBase);
+    AK_MAKE_NONMOVABLE(HeapBase);
+
+public:
+    VM& vm() { return m_vm; }
+
+protected:
+    HeapBase(VM& vm)
+        : m_vm(vm)
+    {
+    }
+
+    VM& m_vm;
+};
+
+class HeapBlockBase {
+    AK_MAKE_NONMOVABLE(HeapBlockBase);
+    AK_MAKE_NONCOPYABLE(HeapBlockBase);
+
+public:
+    static constexpr auto block_size = 16 * KiB;
+    static HeapBlockBase* from_cell(Cell const* cell)
+    {
+        return reinterpret_cast<HeapBlockBase*>(bit_cast<FlatPtr>(cell) & ~(HeapBlockBase::block_size - 1));
+    }
+
+    Heap& heap() { return m_heap; }
+
+protected:
+    HeapBlockBase(Heap& heap)
+        : m_heap(heap)
+    {
+    }
+
+    Heap& m_heap;
+};
+
+}

+ 0 - 10
Userland/Libraries/LibJS/Runtime/VM.h

@@ -342,14 +342,4 @@ private:
     OwnPtr<Bytecode::Interpreter> m_bytecode_interpreter;
 };
 
-ALWAYS_INLINE Heap& Cell::heap() const
-{
-    return HeapBlock::from_cell(this)->heap();
-}
-
-ALWAYS_INLINE VM& Cell::vm() const
-{
-    return heap().vm();
-}
-
 }