Skip to content

Allow symbols as WeakMap and WeakSet keys #58

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Nov 16, 2023
Merged

Allow symbols as WeakMap and WeakSet keys #58

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 55 additions & 16 deletions quickjs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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++;

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand All @@ -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;
p->first_weak_ref = mr;
mr->next_weak_ref = *pmr;
*pmr = mr;
} else {
JS_DupValue(ctx, key);
}
Expand All @@ -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 = &p->first_weak_ref;
pmr = get_first_weak_ref(mr->key);
for(;;) {
mr1 = *pmr;
assert(mr1 != NULL);
Expand Down Expand Up @@ -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 */
Expand All @@ -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,
Expand All @@ -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))
return JS_ThrowTypeErrorNotAnObject(ctx);
if (magic & MAGIC_SET)
if (s->is_weak) {
switch (JS_VALUE_GET_TAG(key)) {
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];
Expand Down
32 changes: 0 additions & 32 deletions test262_errors.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 28 additions & 1 deletion tests/test_builtin.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 };
Expand All @@ -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() {
Expand Down Expand Up @@ -696,4 +722,5 @@ test_regexp();
test_symbol();
test_map();
test_weak_map();
test_weak_set();
test_generator();