From 53ed8decaf3bd6704b2aa40d8b2b9c2e861045dc Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Wed, 22 Jun 2022 23:09:59 +0300 Subject: [PATCH] LibJS: Implement WeakRef changes from 'Symbol as WeakMap Keys Proposal' --- Userland/Libraries/LibJS/Runtime/WeakRef.cpp | 34 +++++++++++++------ Userland/Libraries/LibJS/Runtime/WeakRef.h | 12 ++++--- .../LibJS/Runtime/WeakRefConstructor.cpp | 12 ++++--- .../LibJS/Runtime/WeakRefPrototype.cpp | 6 ++-- .../LibJS/Tests/builtins/WeakRef/WeakRef.js | 7 +++- .../WeakRef/WeakRef.prototype.deref.js | 24 ++++++++++--- 6 files changed, 69 insertions(+), 26 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/WeakRef.cpp b/Userland/Libraries/LibJS/Runtime/WeakRef.cpp index 2958282a8d..e03b5adb13 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRef.cpp +++ b/Userland/Libraries/LibJS/Runtime/WeakRef.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Idan Horowitz + * Copyright (c) 2021-2022, Idan Horowitz * * SPDX-License-Identifier: BSD-2-Clause */ @@ -8,26 +8,38 @@ namespace JS { -WeakRef* WeakRef::create(GlobalObject& global_object, Object* object) +WeakRef* WeakRef::create(GlobalObject& global_object, Object& value) { - return global_object.heap().allocate(global_object, object, *global_object.weak_ref_prototype()); + return global_object.heap().allocate(global_object, value, *global_object.weak_ref_prototype()); } -WeakRef::WeakRef(Object* object, Object& prototype) +WeakRef* WeakRef::create(GlobalObject& global_object, Symbol& value) +{ + return global_object.heap().allocate(global_object, value, *global_object.weak_ref_prototype()); +} + +WeakRef::WeakRef(Object& value, Object& prototype) : Object(prototype) , WeakContainer(heap()) - , m_value(object) + , m_value(&value) + , m_last_execution_generation(vm().execution_generation()) +{ +} + +WeakRef::WeakRef(Symbol& value, Object& prototype) + : Object(prototype) + , WeakContainer(heap()) + , m_value(&value) , m_last_execution_generation(vm().execution_generation()) { } void WeakRef::remove_dead_cells(Badge) { - VERIFY(m_value); - if (m_value->state() == Cell::State::Live) + if (m_value.visit([](Cell* cell) -> bool { return cell->state() == Cell::State::Live; }, [](Empty) -> bool { VERIFY_NOT_REACHED(); })) return; - m_value = nullptr; + m_value = Empty {}; // This is an optimization, we deregister from the garbage collector early (even if we were not garbage collected ourself yet) // to reduce the garbage collection overhead, which we can do because a cleared weak ref cannot be reused. WeakContainer::deregister(); @@ -37,8 +49,10 @@ void WeakRef::visit_edges(Visitor& visitor) { Base::visit_edges(visitor); - if (vm().execution_generation() == m_last_execution_generation) - visitor.visit(m_value); + if (vm().execution_generation() == m_last_execution_generation) { + auto* cell = m_value.visit([](Cell* cell) -> Cell* { return cell; }, [](Empty) -> Cell* { return nullptr; }); + visitor.visit(cell); + } } } diff --git a/Userland/Libraries/LibJS/Runtime/WeakRef.h b/Userland/Libraries/LibJS/Runtime/WeakRef.h index 327973c10f..b3aeaf2d8d 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRef.h +++ b/Userland/Libraries/LibJS/Runtime/WeakRef.h @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Idan Horowitz + * Copyright (c) 2021-2022, Idan Horowitz * * SPDX-License-Identifier: BSD-2-Clause */ @@ -18,12 +18,14 @@ class WeakRef final JS_OBJECT(WeakRef, Object); public: - static WeakRef* create(GlobalObject&, Object*); + static WeakRef* create(GlobalObject&, Object&); + static WeakRef* create(GlobalObject&, Symbol&); - explicit WeakRef(Object*, Object& prototype); + explicit WeakRef(Object&, Object& prototype); + explicit WeakRef(Symbol&, Object& prototype); virtual ~WeakRef() override = default; - Object* value() const { return m_value; }; + auto const& value() const { return m_value; }; void update_execution_generation() { m_last_execution_generation = vm().execution_generation(); }; @@ -32,7 +34,7 @@ public: private: virtual void visit_edges(Visitor&) override; - Object* m_value { nullptr }; + Variant m_value; u32 m_last_execution_generation { 0 }; }; diff --git a/Userland/Libraries/LibJS/Runtime/WeakRefConstructor.cpp b/Userland/Libraries/LibJS/Runtime/WeakRefConstructor.cpp index ced1ec3e35..6dd6241cf2 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRefConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/WeakRefConstructor.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Idan Horowitz + * Copyright (c) 2021-2022, Idan Horowitz * * SPDX-License-Identifier: BSD-2-Clause */ @@ -42,9 +42,13 @@ ThrowCompletionOr WeakRefConstructor::construct(FunctionObject& new_tar auto& global_object = this->global_object(); auto target = vm.argument(0); - if (!target.is_object()) - return vm.throw_completion(global_object, ErrorType::NotAnObject, target.to_string_without_side_effects()); - return TRY(ordinary_create_from_constructor(global_object, new_target, &GlobalObject::weak_ref_prototype, &target.as_object())); + if (!can_be_held_weakly(target)) + return vm.throw_completion(global_object, ErrorType::CannotBeHeldWeakly, target.to_string_without_side_effects()); + + if (target.is_object()) + return TRY(ordinary_create_from_constructor(global_object, new_target, &GlobalObject::weak_ref_prototype, target.as_object())); + VERIFY(target.is_symbol()); + return TRY(ordinary_create_from_constructor(global_object, new_target, &GlobalObject::weak_ref_prototype, target.as_symbol())); } } diff --git a/Userland/Libraries/LibJS/Runtime/WeakRefPrototype.cpp b/Userland/Libraries/LibJS/Runtime/WeakRefPrototype.cpp index ab94b071db..dee76df167 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakRefPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/WeakRefPrototype.cpp @@ -1,5 +1,5 @@ /* - * Copyright (c) 2021, Idan Horowitz + * Copyright (c) 2021-2022, Idan Horowitz * * SPDX-License-Identifier: BSD-2-Clause */ @@ -30,7 +30,9 @@ JS_DEFINE_NATIVE_FUNCTION(WeakRefPrototype::deref) auto* weak_ref = TRY(typed_this_object(global_object)); weak_ref->update_execution_generation(); - return weak_ref->value() ?: js_undefined(); + return weak_ref->value().visit( + [](Empty) -> Value { return js_undefined(); }, + [](auto* value) -> Value { return value; }); } } diff --git a/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.js b/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.js index d098bf2ed6..07cd01986a 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.js +++ b/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.js @@ -8,7 +8,7 @@ describe("errors", () => { [-100, Infinity, NaN, 152n, undefined].forEach(value => { expect(() => { new WeakRef(value); - }).toThrowWithMessage(TypeError, "is not an object"); + }).toThrowWithMessage(TypeError, "cannot be held weakly"); }); }); test("called without new", () => { @@ -27,4 +27,9 @@ describe("normal behavior", () => { var a = new WeakRef({}); expect(a instanceof WeakRef).toBeTrue(); }); + + test("constructor with single symbol argument", () => { + var a = new WeakRef(Symbol()); + expect(a instanceof WeakRef).toBeTrue(); + }); }); diff --git a/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.prototype.deref.js b/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.prototype.deref.js index e616cff77a..80dfe9ccca 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.prototype.deref.js +++ b/Userland/Libraries/LibJS/Tests/builtins/WeakRef/WeakRef.prototype.deref.js @@ -3,13 +3,18 @@ test("length is 0", () => { }); test("basic functionality", () => { - var original = { a: 1 }; - var weakRef = new WeakRef(original); + var originalObject = { a: 1 }; + var objectWeakRef = new WeakRef(originalObject); - expect(weakRef.deref()).toBe(original); + expect(objectWeakRef.deref()).toBe(originalObject); + + var originalSymbol = { a: 1 }; + var symbolWeakRef = new WeakRef(originalSymbol); + + expect(symbolWeakRef.deref()).toBe(originalSymbol); }); -test("kept alive for current synchronous execution sequence", () => { +test("object kept alive for current synchronous execution sequence", () => { var weakRef; { weakRef = new WeakRef({ a: 1 }); @@ -19,3 +24,14 @@ test("kept alive for current synchronous execution sequence", () => { // This is fine 🔥 expect(weakRef.deref()).not.toBe(undefined); }); + +test("symbol kept alive for current synchronous execution sequence", () => { + var weakRef; + { + weakRef = new WeakRef(Symbol("foo")); + } + weakRef.deref(); + gc(); + // This is fine 🔥 + expect(weakRef.deref()).not.toBe(undefined); +});