From dbd0110721baf5c9dcb94fd138a6c4072a4fc0b7 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Wed, 22 Jun 2022 23:09:31 +0300 Subject: [PATCH] LibJS: Implement WeakSet changes from 'Symbol as WeakMap Keys Proposal' --- .../LibJS/Runtime/WeakSetPrototype.cpp | 17 ++++++++------- .../LibJS/Tests/builtins/WeakSet/WeakSet.js | 2 +- .../builtins/WeakSet/WeakSet.prototype.add.js | 21 ++++++++++++++----- .../WeakSet/WeakSet.prototype.delete.js | 4 +++- .../builtins/WeakSet/WeakSet.prototype.has.js | 4 +++- 5 files changed, 32 insertions(+), 16 deletions(-) diff --git a/Userland/Libraries/LibJS/Runtime/WeakSetPrototype.cpp b/Userland/Libraries/LibJS/Runtime/WeakSetPrototype.cpp index 85e982f19e..cd6a36655e 100644 --- a/Userland/Libraries/LibJS/Runtime/WeakSetPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/WeakSetPrototype.cpp @@ -1,11 +1,12 @@ /* - * Copyright (c) 2021, Idan Horowitz + * Copyright (c) 2021-2022, Idan Horowitz * * SPDX-License-Identifier: BSD-2-Clause */ #include #include +#include #include namespace JS { @@ -34,9 +35,9 @@ JS_DEFINE_NATIVE_FUNCTION(WeakSetPrototype::add) { auto* weak_set = TRY(typed_this_object(global_object)); auto value = vm.argument(0); - if (!value.is_object()) - return vm.throw_completion(global_object, ErrorType::NotAnObject, value.to_string_without_side_effects()); - weak_set->values().set(&value.as_object(), AK::HashSetExistingEntryBehavior::Keep); + if (!can_be_held_weakly(value)) + return vm.throw_completion(global_object, ErrorType::CannotBeHeldWeakly, value.to_string_without_side_effects()); + weak_set->values().set(&value.as_cell(), AK::HashSetExistingEntryBehavior::Keep); return weak_set; } @@ -45,9 +46,9 @@ JS_DEFINE_NATIVE_FUNCTION(WeakSetPrototype::delete_) { auto* weak_set = TRY(typed_this_object(global_object)); auto value = vm.argument(0); - if (!value.is_object()) + if (!can_be_held_weakly(value)) return Value(false); - return Value(weak_set->values().remove(&value.as_object())); + return Value(weak_set->values().remove(&value.as_cell())); } // 24.4.3.4 WeakSet.prototype.has ( value ), https://tc39.es/ecma262/#sec-weakset.prototype.has @@ -55,10 +56,10 @@ JS_DEFINE_NATIVE_FUNCTION(WeakSetPrototype::has) { auto* weak_set = TRY(typed_this_object(global_object)); auto value = vm.argument(0); - if (!value.is_object()) + if (!can_be_held_weakly(value)) return Value(false); auto& values = weak_set->values(); - return Value(values.find(&value.as_object()) != values.end()); + return Value(values.find(&value.as_cell()) != values.end()); } } diff --git a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.js b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.js index 61400a048d..02ac097b21 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.js +++ b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.js @@ -24,7 +24,7 @@ describe("normal behavior", () => { }); test("constructor with single array argument", () => { - var a = new WeakSet([{ a: 1 }, { a: 2 }, { a: 3 }]); + var a = new WeakSet([{ a: 1 }, { a: 2 }, { a: 3 }, Symbol("foo")]); expect(a instanceof WeakSet).toBeTrue(); }); }); diff --git a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.add.js b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.add.js index f9b40e0869..0d5fdda12c 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.add.js +++ b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.add.js @@ -1,9 +1,10 @@ test("basic functionality", () => { expect(WeakSet.prototype.add).toHaveLength(1); - const weakSet = new WeakSet([{ a: 1 }, { a: 2 }, { a: 3 }]); + const weakSet = new WeakSet([{ a: 1 }, { a: 2 }, { a: 3 }, Symbol("foo")]); expect(weakSet.add({ a: 4 })).toBe(weakSet); expect(weakSet.add({ a: 1 })).toBe(weakSet); + expect(weakSet.add(Symbol("bar"))).toBe(weakSet); }); test("invalid values", () => { @@ -11,18 +12,28 @@ test("invalid values", () => { [-100, Infinity, NaN, "hello", 152n].forEach(value => { expect(() => { weakSet.add(value); - }).toThrowWithMessage(TypeError, "is not an object"); + }).toThrowWithMessage(TypeError, "cannot be held weakly"); }); }); test("automatic removal of garbage-collected values", () => { const weakSet = new WeakSet(); - const item = { a: 1 }; + const objectItem = { a: 1 }; - expect(weakSet.add(item)).toBe(weakSet); + expect(weakSet.add(objectItem)).toBe(weakSet); expect(getWeakSetSize(weakSet)).toBe(1); - markAsGarbage("item"); + markAsGarbage("objectItem"); + gc(); + + expect(getWeakSetSize(weakSet)).toBe(0); + + const symbolItem = Symbol("foo"); + + expect(weakSet.add(symbolItem)).toBe(weakSet); + expect(getWeakSetSize(weakSet)).toBe(1); + + markAsGarbage("symbolItem"); gc(); expect(getWeakSetSize(weakSet)).toBe(0); diff --git a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.delete.js b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.delete.js index adfff46188..c40a75bf78 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.delete.js +++ b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.delete.js @@ -1,9 +1,11 @@ test("basic functionality", () => { expect(WeakSet.prototype.delete).toHaveLength(1); - var original = [{ a: 1 }, { a: 2 }, { a: 3 }]; + var original = [{ a: 1 }, { a: 2 }, { a: 3 }, Symbol("foo")]; const weakSet = new WeakSet(original); expect(weakSet.delete(original[0])).toBeTrue(); expect(weakSet.delete(original[0])).toBeFalse(); + expect(weakSet.delete(original[3])).toBeTrue(); + expect(weakSet.delete(original[3])).toBeFalse(); expect(weakSet.delete(null)).toBeFalse(); }); diff --git a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.has.js b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.has.js index 3a8cfe021d..fb2dd7cc88 100644 --- a/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.has.js +++ b/Userland/Libraries/LibJS/Tests/builtins/WeakSet/WeakSet.prototype.has.js @@ -3,10 +3,12 @@ test("length is 1", () => { }); test("basic functionality", () => { - var original = [{ a: 1 }, { a: 2 }, { a: 3 }]; + var original = [{ a: 1 }, { a: 2 }, { a: 3 }, Symbol("foo")]; var weakSet = new WeakSet(original); expect(new WeakSet().has()).toBeFalse(); expect(weakSet.has(original[0])).toBeTrue(); + expect(weakSet.has(original[3])).toBeTrue(); expect(weakSet.has({ a: 1 })).toBeFalse(); + expect(weakSet.has(Symbol("foo"))).toBeFalse(); });