LibJS: Implement WeakMap changes from 'Symbol as WeakMap Keys Proposal'

Tests/LibJS/test-js.cpp

@@ -68,10 +68,10 @@ TESTJS_GLOBAL_FUNCTION(mark_as_garbage, markAsGarbage)
 
     auto value = TRY(reference.get_value(global_object));
 
-    if (!value.is_object())
-        return vm.throw_completion<JS::TypeError>(global_object, JS::ErrorType::NotAnObject, String::formatted("Variable with name {}", variable_name.string()));
+    if (!can_be_held_weakly(value))
+        return vm.throw_completion<JS::TypeError>(global_object, JS::ErrorType::CannotBeHeldWeakly, String::formatted("Variable with name {}", variable_name.string()));
 
-    vm.heap().uproot_cell(&value.as_object());
+    vm.heap().uproot_cell(&value.as_cell());
     TRY(reference.delete_(global_object));
 
     return JS::js_undefined();

Userland/Libraries/LibJS/Runtime/ErrorTypes.h

@@ -19,6 +19,7 @@
     M(CallStackSizeExceeded, "Call stack size limit exceeded")                                                                          \
     M(CannotDeclareGlobalFunction, "Cannot declare global function of name '{}'")                                                       \
     M(CannotDeclareGlobalVariable, "Cannot declare global variable of name '{}'")                                                       \
+    M(CannotBeHeldWeakly, "{} cannot be held weakly")                                                                                   \
     M(ClassConstructorWithoutNew, "Class constructor {} must be called with 'new'")                                                     \
     M(ClassExtendsValueNotAConstructorOrNull, "Class extends value {} is not a constructor or null")                                    \
     M(ClassExtendsValueInvalidPrototype, "Class extends value has an invalid prototype {}")                                             \

Userland/Libraries/LibJS/Runtime/WeakMapPrototype.cpp

@@ -1,11 +1,12 @@
 /*
- * Copyright (c) 2021, Idan Horowitz <idan.horowitz@serenityos.org>
+ * Copyright (c) 2021-2022, Idan Horowitz <idan.horowitz@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
 #include <AK/HashTable.h>
 #include <AK/TypeCasts.h>
+#include <LibJS/Runtime/AbstractOperations.h>
 #include <LibJS/Runtime/WeakMapPrototype.h>
 
 namespace JS {
@@ -35,9 +36,9 @@ JS_DEFINE_NATIVE_FUNCTION(WeakMapPrototype::delete_)
 {
     auto* weak_map = 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_map->values().remove(&value.as_object()));
+    return Value(weak_map->values().remove(&value.as_cell()));
 }
 
 // 24.3.3.3 WeakMap.prototype.get ( key ), https://tc39.es/ecma262/#sec-weakmap.prototype.get
@@ -45,10 +46,10 @@ JS_DEFINE_NATIVE_FUNCTION(WeakMapPrototype::get)
 {
     auto* weak_map = TRY(typed_this_object(global_object));
     auto value = vm.argument(0);
-    if (!value.is_object())
+    if (!can_be_held_weakly(value))
         return js_undefined();
     auto& values = weak_map->values();
-    auto result = values.find(&value.as_object());
+    auto result = values.find(&value.as_cell());
     if (result == values.end())
         return js_undefined();
     return result->value;
@@ -59,10 +60,10 @@ JS_DEFINE_NATIVE_FUNCTION(WeakMapPrototype::has)
 {
     auto* weak_map = 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_map->values();
-    return Value(values.find(&value.as_object()) != values.end());
+    return Value(values.find(&value.as_cell()) != values.end());
 }
 
 // 24.3.3.5 WeakMap.prototype.set ( key, value ), https://tc39.es/ecma262/#sec-weakmap.prototype.set
@@ -70,9 +71,9 @@ JS_DEFINE_NATIVE_FUNCTION(WeakMapPrototype::set)
 {
     auto* weak_map = TRY(typed_this_object(global_object));
     auto value = vm.argument(0);
-    if (!value.is_object())
-        return vm.throw_completion<TypeError>(global_object, ErrorType::NotAnObject, value.to_string_without_side_effects());
-    weak_map->values().set(&value.as_object(), vm.argument(1));
+    if (!can_be_held_weakly(value))
+        return vm.throw_completion<TypeError>(global_object, ErrorType::CannotBeHeldWeakly, value.to_string_without_side_effects());
+    weak_map->values().set(&value.as_cell(), vm.argument(1));
     return weak_map;
 }
 

Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.js

@@ -37,6 +37,6 @@ describe("regressions", () => {
     test("missing key/value properties on iterable entry", () => {
         expect(() => {
             new WeakMap([{}]);
-        }).toThrowWithMessage(TypeError, "undefined is not an object");
+        }).toThrowWithMessage(TypeError, "undefined cannot be held weakly");
     });
 });

Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.prototype.delete.js

@@ -5,9 +5,12 @@ test("basic functionality", () => {
         [{ a: 1 }, 1],
         [{ a: 2 }, 2],
         [{ a: 3 }, 3],
+        [Symbol("foo"), "bar"],
     ];
     const weakMap = new WeakMap(original);
     expect(weakMap.delete(original[0][0])).toBeTrue();
     expect(weakMap.delete(original[0][0])).toBeFalse();
+    expect(weakMap.delete(original[3][0])).toBeTrue();
+    expect(weakMap.delete(original[3][0])).toBeFalse();
     expect(weakMap.delete(null)).toBeFalse();
 });

Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.prototype.get.js

@@ -5,8 +5,10 @@ test("basic functionality", () => {
         [{ a: 1 }, 1],
         [{ a: 2 }, 2],
         [{ a: 3 }, 3],
+        [Symbol("foo"), "bar"],
     ];
     const weakMap = new WeakMap(original);
     expect(weakMap.get(original[0][0])).toBe(original[0][1]);
+    expect(weakMap.get(original[3][0])).toBe(original[3][1]);
     expect(weakMap.get(null)).toBe(undefined);
 });

Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.prototype.has.js

@@ -7,10 +7,12 @@ test("basic functionality", () => {
         [{ a: 1 }, 1],
         [{ a: 2 }, 2],
         [{ a: 3 }, 3],
+        [Symbol("foo"), "bar"],
     ];
     var weakMap = new WeakMap(original);
 
     expect(new WeakMap().has()).toBeFalse();
     expect(weakMap.has(original[0][0])).toBeTrue();
+    expect(weakMap.has(original[3][0])).toBeTrue();
     expect(weakMap.has({ a: 1 })).toBeFalse();
 });

Userland/Libraries/LibJS/Tests/builtins/WeakMap/WeakMap.prototype.set.js

@@ -5,9 +5,11 @@ test("basic functionality", () => {
         [{ a: 1 }, 1],
         [{ a: 2 }, 2],
         [{ a: 3 }, 3],
+        [Symbol("foo"), "bar"],
     ]);
     expect(weakMap.set({ a: 4 }, 4)).toBe(weakMap);
     expect(weakMap.set({ a: 1 }, 2)).toBe(weakMap);
+    expect(weakMap.set(Symbol("hello"), "friends")).toBe(weakMap);
 });
 
 test("invalid values", () => {
@@ -15,18 +17,28 @@ test("invalid values", () => {
     [-100, Infinity, NaN, "hello", 152n].forEach(value => {
         expect(() => {
             weakMap.set(value, value);
-        }).toThrowWithMessage(TypeError, "is not an object");
+        }).toThrowWithMessage(TypeError, "cannot be held weakly");
     });
 });
 
 test("automatic removal of garbage-collected values", () => {
     const weakMap = new WeakMap();
-    const key = { e: 3 };
+    const objectKey = { e: 3 };
 
-    expect(weakMap.set(key, 1)).toBe(weakMap);
+    expect(weakMap.set(objectKey, 1)).toBe(weakMap);
     expect(getWeakMapSize(weakMap)).toBe(1);
 
-    markAsGarbage("key");
+    markAsGarbage("objectKey");
+    gc();
+
+    expect(getWeakMapSize(weakMap)).toBe(0);
+
+    const symbolKey = Symbol("foo");
+
+    expect(weakMap.set(symbolKey, "bar")).toBe(weakMap);
+    expect(getWeakMapSize(weakMap)).toBe(1);
+
+    markAsGarbage("symbolKey");
     gc();
 
     expect(getWeakMapSize(weakMap)).toBe(0);