-
Notifications
You must be signed in to change notification settings - Fork 204
Support (de)serializing Map and Set objects #483
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33114,9 +33114,11 @@ typedef enum BCTagEnum { | |
BC_TAG_DATE, | ||
BC_TAG_OBJECT_VALUE, | ||
BC_TAG_OBJECT_REFERENCE, | ||
BC_TAG_MAP, | ||
BC_TAG_SET, | ||
} BCTagEnum; | ||
|
||
#define BC_VERSION 12 | ||
#define BC_VERSION 13 | ||
|
||
typedef struct BCWriterState { | ||
JSContext *ctx; | ||
|
@@ -33757,6 +33759,9 @@ static int JS_WriteRegExp(BCWriterState *s, JSRegExp regexp) | |
return 0; | ||
} | ||
|
||
static int JS_WriteMap(BCWriterState *s, struct JSMapState *map_state); | ||
static int JS_WriteSet(BCWriterState *s, struct JSMapState *map_state); | ||
|
||
static int JS_WriteObjectRec(BCWriterState *s, JSValue obj) | ||
{ | ||
uint32_t tag; | ||
|
@@ -33860,6 +33865,14 @@ static int JS_WriteObjectRec(BCWriterState *s, JSValue obj) | |
bc_put_u8(s, BC_TAG_OBJECT_VALUE); | ||
ret = JS_WriteObjectRec(s, p->u.object_data); | ||
break; | ||
case JS_CLASS_MAP: | ||
bc_put_u8(s, BC_TAG_MAP); | ||
ret = JS_WriteMap(s, p->u.map_state); | ||
break; | ||
case JS_CLASS_SET: | ||
bc_put_u8(s, BC_TAG_SET); | ||
ret = JS_WriteSet(s, p->u.map_state); | ||
break; | ||
default: | ||
if (p->class_id >= JS_CLASS_UINT8C_ARRAY && | ||
p->class_id <= JS_CLASS_FLOAT64_ARRAY) { | ||
|
@@ -34996,6 +35009,9 @@ static JSValue JS_ReadObjectValue(BCReaderState *s) | |
return JS_EXCEPTION; | ||
} | ||
|
||
static JSValue JS_ReadMap(BCReaderState *s); | ||
static JSValue JS_ReadSet(BCReaderState *s); | ||
|
||
static JSValue JS_ReadObjectRec(BCReaderState *s) | ||
{ | ||
JSContext *ctx = s->ctx; | ||
|
@@ -35103,6 +35119,12 @@ static JSValue JS_ReadObjectRec(BCReaderState *s) | |
obj = js_dup(JS_MKPTR(JS_TAG_OBJECT, s->objects[val])); | ||
} | ||
break; | ||
case BC_TAG_MAP: | ||
obj = JS_ReadMap(s); | ||
break; | ||
case BC_TAG_SET: | ||
obj = JS_ReadSet(s); | ||
break; | ||
default: | ||
invalid_tag: | ||
return JS_ThrowSyntaxError(ctx, "invalid tag (tag=%d pos=%u)", | ||
|
@@ -46026,6 +46048,99 @@ static JSValue js_map_iterator_next(JSContext *ctx, JSValue this_val, | |
} | ||
} | ||
|
||
static JSValue js_map_read(BCReaderState *s, int magic) | ||
{ | ||
JSContext *ctx = s->ctx; | ||
JSValue obj, rv, argv[2]; | ||
uint32_t i, prop_count; | ||
|
||
argv[0] = JS_UNDEFINED; | ||
argv[1] = JS_UNDEFINED; | ||
obj = js_map_constructor(ctx, JS_UNDEFINED, 0, NULL, magic); | ||
if (JS_IsException(obj)) | ||
return JS_EXCEPTION; | ||
if (BC_add_object_ref(s, obj)) | ||
goto fail; | ||
if (bc_get_leb128(s, &prop_count)) | ||
goto fail; | ||
for(i = 0; i < prop_count; i++) { | ||
argv[0] = JS_ReadObjectRec(s); | ||
if (JS_IsException(argv[0])) | ||
goto fail; | ||
if (!(magic & MAGIC_SET)) { | ||
argv[1] = JS_ReadObjectRec(s); | ||
if (JS_IsException(argv[1])) | ||
goto fail; | ||
} | ||
rv = js_map_set(ctx, obj, countof(argv), argv, magic); | ||
if (JS_IsException(rv)) | ||
goto fail; | ||
JS_FreeValue(ctx, rv); | ||
JS_FreeValue(ctx, argv[0]); | ||
JS_FreeValue(ctx, argv[1]); | ||
argv[0] = JS_UNDEFINED; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this necessary since yoy are about to return? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, because it's inside a loop, otherwise we can end up double-freeing argv[1] on the next iteration. (And I'm clearing argv[0] for clarity's sake, otherwise a casual reader will stop and think: "Why one and not the other?") There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great! |
||
argv[1] = JS_UNDEFINED; | ||
} | ||
return obj; | ||
fail: | ||
JS_FreeValue(ctx, obj); | ||
JS_FreeValue(ctx, argv[0]); | ||
JS_FreeValue(ctx, argv[1]); | ||
return JS_EXCEPTION; | ||
} | ||
|
||
static int js_map_write(BCWriterState *s, struct JSMapState *map_state, | ||
int magic) | ||
{ | ||
struct list_head *el; | ||
JSMapRecord *mr; | ||
uint32_t count; | ||
|
||
count = 0; | ||
|
||
if (map_state) { | ||
list_for_each(el, &map_state->records) { | ||
count++; | ||
} | ||
} | ||
|
||
bc_put_leb128(s, count); | ||
|
||
if (map_state) { | ||
list_for_each(el, &map_state->records) { | ||
mr = list_entry(el, JSMapRecord, link); | ||
if (JS_WriteObjectRec(s, mr->key)) | ||
return -1; | ||
// mr->value is always JS_UNDEFINED for sets | ||
if (!(magic & MAGIC_SET)) | ||
if (JS_WriteObjectRec(s, mr->value)) | ||
return -1; | ||
} | ||
} | ||
|
||
return 0; | ||
} | ||
|
||
static JSValue JS_ReadMap(BCReaderState *s) | ||
{ | ||
return js_map_read(s, 0); | ||
} | ||
|
||
static JSValue JS_ReadSet(BCReaderState *s) | ||
{ | ||
return js_map_read(s, MAGIC_SET); | ||
} | ||
|
||
static int JS_WriteMap(BCWriterState *s, struct JSMapState *map_state) | ||
{ | ||
return js_map_write(s, map_state, 0); | ||
} | ||
|
||
static int JS_WriteSet(BCWriterState *s, struct JSMapState *map_state) | ||
{ | ||
return js_map_write(s, map_state, MAGIC_SET); | ||
} | ||
|
||
static const JSCFunctionListEntry js_map_funcs[] = { | ||
JS_CFUNC_DEF("groupBy", 2, js_map_groupBy ), | ||
JS_CGETSET_DEF("[Symbol.species]", js_get_this, NULL ), | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm merely following the predominant style for
for
loops here (of the 533 for statements in quickjs.c, 66.8% are written this way.)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fabrice and i differ on this, I always put a space after
if
,for
,while
,switch
,return
with a value etc. Fabrice is less adamant about it :)