Allow symbols as WeakMap and WeakSet keys (#58)

quickjs.c

@@ -446,6 +446,7 @@ struct JSString {
     uint32_t hash : 30;
     uint8_t atom_type : 2; /* != 0 if atom, JS_ATOM_TYPE_x */
     uint32_t hash_next; /* atom_index for JS_ATOM_TYPE_SYMBOL */
+    struct JSMapRecord *first_weak_ref;
 #ifdef DUMP_LEAKS
     struct list_head link; /* string list */
 #endif
@@ -1052,7 +1053,7 @@ static int JS_CreateProperty(JSContext *ctx, JSObject *p,
                              JSValueConst getter, JSValueConst setter,
                              int flags);
 static int js_string_memcmp(const JSString *p1, const JSString *p2, int len);
-static void reset_weak_ref(JSRuntime *rt, JSObject *p);
+static void reset_weak_ref(JSRuntime *rt, struct JSMapRecord **first_weak_ref);
 static JSValue js_array_buffer_constructor3(JSContext *ctx,
                                             JSValueConst new_target,
                                             uint64_t len, JSClassID class_id,
@@ -2624,6 +2625,7 @@ static JSAtom __JS_NewAtom(JSRuntime *rt, JSString *str, int atom_type)
     p->hash = h;
     p->hash_next = i;   /* atom_index */
     p->atom_type = atom_type;
+    p->first_weak_ref = NULL;
 
     rt->atom_count++;
 
@@ -2718,6 +2720,9 @@ static void JS_FreeAtomStruct(JSRuntime *rt, JSAtomStruct *p)
     /* insert in free atom list */
     rt->atom_array[i] = atom_set_free(rt->atom_free_index);
     rt->atom_free_index = i;
+    if (unlikely(p->first_weak_ref)) {
+        reset_weak_ref(rt, &p->first_weak_ref);
+    }
     /* free the string structure */
 #ifdef DUMP_LEAKS
     list_del(&p->link);
@@ -5184,7 +5189,7 @@ static void free_object(JSRuntime *rt, JSObject *p)
     p->prop = NULL;
 
     if (unlikely(p->first_weak_ref)) {
-        reset_weak_ref(rt, p);
+        reset_weak_ref(rt, &p->first_weak_ref);
     }
 
     finalizer = rt->class_array[p->class_id].finalizer;
@@ -43529,11 +43534,31 @@ static void map_hash_resize(JSContext *ctx, JSMapState *s)
     s->record_count_threshold = new_hash_size * 2;
 }
 
+static JSMapRecord **get_first_weak_ref(JSValueConst key)
+{
+        switch (JS_VALUE_GET_TAG(key)) {
+        case JS_TAG_OBJECT:
+            {
+                JSObject *p = JS_VALUE_GET_OBJ(key);
+                return &p->first_weak_ref;
+            }
+            break;
+        case JS_TAG_SYMBOL:
+            {
+                JSAtomStruct *p = JS_VALUE_GET_PTR(key);
+                return &p->first_weak_ref;
+            }
+            break;
+        default:
+            abort();
+        }
+}
+
 static JSMapRecord *map_add_record(JSContext *ctx, JSMapState *s,
                                    JSValueConst key)
 {
     uint32_t h;
-    JSMapRecord *mr;
+    JSMapRecord *mr, **pmr;
 
     mr = js_malloc(ctx, sizeof(*mr));
     if (!mr)
@@ -43542,10 +43567,10 @@ static JSMapRecord *map_add_record(JSContext *ctx, JSMapState *s,
     mr->map = s;
     mr->empty = FALSE;
     if (s->is_weak) {
-        JSObject *p = JS_VALUE_GET_OBJ(key);
+        pmr = get_first_weak_ref(key);
         /* Add the weak reference */
-        mr->next_weak_ref = p->first_weak_ref;
+        mr->next_weak_ref = *pmr;
-        p->first_weak_ref = mr;
+        *pmr = mr;
     } else {
         JS_DupValue(ctx, key);
     }
@@ -43567,10 +43592,8 @@ static JSMapRecord *map_add_record(JSContext *ctx, JSMapState *s,
 static void delete_weak_ref(JSRuntime *rt, JSMapRecord *mr)
 {
     JSMapRecord **pmr, *mr1;
-    JSObject *p;
 
-    p = JS_VALUE_GET_OBJ(mr->key);
+    pmr = get_first_weak_ref(mr->key);
-    pmr = &p->first_weak_ref;
     for(;;) {
         mr1 = *pmr;
         assert(mr1 != NULL);
@@ -43614,14 +43637,14 @@ static void map_decref_record(JSRuntime *rt, JSMapRecord *mr)
     }
 }
 
-static void reset_weak_ref(JSRuntime *rt, JSObject *p)
+static void reset_weak_ref(JSRuntime *rt, struct JSMapRecord **first_weak_ref)
 {
     JSMapRecord *mr, *mr_next;
     JSMapState *s;
 
     /* first pass to remove the records from the WeakMap/WeakSet
        lists */
-    for(mr = p->first_weak_ref; mr != NULL; mr = mr->next_weak_ref) {
+    for(mr = *first_weak_ref; mr != NULL; mr = mr->next_weak_ref) {
         s = mr->map;
         assert(s->is_weak);
         assert(!mr->empty); /* no iterator on WeakMap/WeakSet */
@@ -43631,13 +43654,13 @@ static void reset_weak_ref(JSRuntime *rt, JSObject *p)
 
     /* second pass to free the values to avoid modifying the weak
        reference list while traversing it. */
-    for(mr = p->first_weak_ref; mr != NULL; mr = mr_next) {
+    for(mr = *first_weak_ref; mr != NULL; mr = mr_next) {
         mr_next = mr->next_weak_ref;
         JS_FreeValueRT(rt, mr->value);
         js_free_rt(rt, mr);
     }
 
-    p->first_weak_ref = NULL; /* fail safe */
+    *first_weak_ref = NULL; /* fail safe */
 }
 
 static JSValue js_map_set(JSContext *ctx, JSValueConst this_val,
@@ -43646,13 +43669,29 @@ static JSValue js_map_set(JSContext *ctx, JSValueConst this_val,
     JSMapState *s = JS_GetOpaque2(ctx, this_val, JS_CLASS_MAP + magic);
     JSMapRecord *mr;
     JSValueConst key, value;
+    JSAtomStruct *p;
+    int is_set;
 
     if (!s)
         return JS_EXCEPTION;
+    is_set = (magic & MAGIC_SET);
     key = map_normalize_key(ctx, argv[0]);
-    if (s->is_weak && !JS_IsObject(key))
+    if (s->is_weak) {
-        return JS_ThrowTypeErrorNotAnObject(ctx);
+        switch (JS_VALUE_GET_TAG(key)) {
-    if (magic & MAGIC_SET)
+        case JS_TAG_OBJECT:
+            break;
+        case JS_TAG_SYMBOL:
+            // Per spec: prohibit symbols registered with Symbol.for()
+            p = JS_VALUE_GET_PTR(key);
+            if (p->atom_type != JS_ATOM_TYPE_GLOBAL_SYMBOL)
+                break;
+            // fallthru
+        default:
+            return JS_ThrowTypeError(ctx, "invalid value used as %s key",
+                                     is_set ? "WeakSet" : "WeakMap");
+        }
+    }
+    if (is_set)
         value = JS_UNDEFINED;
     else
         value = argv[1];

test262_errors.txt

@@ -336,38 +336,6 @@ test262/test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds
 test262/test/built-ins/TypedArrayConstructors/internals/Set/key-is-out-of-bounds.js:22: strict mode: Test262Error: Reflect.set(sample, "-1", 1) must return true Expected SameValue(«false», «true») to be true (Testing with Float64Array.)
 test262/test/built-ins/TypedArrayConstructors/internals/Set/tonumber-value-detached-buffer.js:39: Test262Error: Expected SameValue(«false», «true») to be true (Testing with Float64Array.)
 test262/test/built-ins/TypedArrayConstructors/internals/Set/tonumber-value-detached-buffer.js:39: strict mode: Test262Error: Expected SameValue(«false», «true») to be true (Testing with Float64Array.)
-test262/test/built-ins/WeakMap/iterable-with-symbol-keys.js:32: TypeError: not an object
-test262/test/built-ins/WeakMap/iterable-with-symbol-keys.js:32: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/delete/delete-entry-with-symbol-key-initial-iterable.js:26: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/delete/delete-entry-with-symbol-key-initial-iterable.js:26: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/delete/delete-entry-with-symbol-key.js:24: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/delete/delete-entry-with-symbol-key.js:24: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/delete/returns-false-when-symbol-key-not-present.js:18: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/delete/returns-false-when-symbol-key-not-present.js:18: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/get/returns-undefined-with-symbol-key.js:28: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/get/returns-undefined-with-symbol-key.js:28: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/get/returns-value-with-symbol-key.js:22: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/get/returns-value-with-symbol-key.js:22: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/has/returns-false-when-symbol-key-not-present.js:20: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/has/returns-false-when-symbol-key-not-present.js:20: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/has/returns-true-when-symbol-key-present.js:19: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/has/returns-true-when-symbol-key-present.js:19: strict mode: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/set/adds-symbol-element.js:17: TypeError: not an object
-test262/test/built-ins/WeakMap/prototype/set/adds-symbol-element.js:17: strict mode: TypeError: not an object
-test262/test/built-ins/WeakSet/iterable-with-symbol-values.js:24: TypeError: not an object
-test262/test/built-ins/WeakSet/iterable-with-symbol-values.js:24: strict mode: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/add/adds-symbol-element.js:17: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/add/adds-symbol-element.js:17: strict mode: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/add/returns-this-symbol.js:18: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/add/returns-this-symbol.js:18: strict mode: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/add/returns-this-when-ignoring-duplicate-symbol.js:20: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/add/returns-this-when-ignoring-duplicate-symbol.js:20: strict mode: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/delete/delete-symbol-entry.js:22: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/delete/delete-symbol-entry.js:22: strict mode: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/has/returns-false-when-symbol-value-not-present.js:20: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/has/returns-false-when-symbol-value-not-present.js:20: strict mode: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/has/returns-true-when-symbol-value-present.js:17: TypeError: not an object
-test262/test/built-ins/WeakSet/prototype/has/returns-true-when-symbol-value-present.js:17: strict mode: TypeError: not an object
 test262/test/language/expressions/assignment/target-member-computed-reference-null.js:32: Test262Error: Expected a DummyError but got a TypeError
 test262/test/language/expressions/assignment/target-member-computed-reference-null.js:32: strict mode: Test262Error: Expected a DummyError but got a TypeError
 test262/test/language/expressions/assignment/target-member-computed-reference-undefined.js:32: Test262Error: Expected a DummyError but got a TypeError

tests/test_builtin.js

@@ -618,10 +618,20 @@ function test_map()
 
 function test_weak_map()
 {
-    var a, i, n, tab, o, v, n2;
+    var a, e, i, n, tab, o, v, n2;
     a = new WeakMap();
     n = 10;
     tab = [];
+    for (const k of [null, 42, "no", Symbol.for("forbidden")]) {
+        e = undefined;
+        try {
+            a.set(k, 42);
+        } catch (_e) {
+            e = _e;
+        }
+        assert(!!e);
+        assert(e.message, "invalid value used as WeakMap key");
+    }
     for(i = 0; i < n; i++) {
         v = { };
         o = { id: i };
@@ -640,6 +650,22 @@ function test_weak_map()
     /* the WeakMap should be empty here */
 }
 
+function test_weak_set()
+{
+    var a, e;
+    a = new WeakSet();
+    for (const k of [null, 42, "no", Symbol.for("forbidden")]) {
+        e = undefined;
+        try {
+            a.add(k);
+        } catch (_e) {
+            e = _e;
+        }
+        assert(!!e);
+        assert(e.message, "invalid value used as WeakSet key");
+    }
+}
+
 function test_generator()
 {
     function *f() {
@@ -696,4 +722,5 @@ test_regexp();
 test_symbol();
 test_map();
 test_weak_map();
+test_weak_set();
 test_generator();