Skip to content

Commit

Permalink
Use JsVal instead of JsRef in more places (nr. 3) (#4236)
Browse files Browse the repository at this point in the history 
This also fixes `JsArray_pop` which leaked memory and returned a singleton array
with the element removed rather than the element itself.
  • Loading branch information
@hoodmane
hoodmane committed Oct 22, 2023
1 parent 98cc8a2 commit a41e267
Show file tree
Hide file tree
Showing 17 changed files with 488 additions and 609 deletions.
4 changes: 4 additions & 0 deletions docs/project/changelog.md
View file
Expand Up @@ -19,6 +19,10 @@ myst:
- {{ Enhancement }} Added experimental support for stack switching.
{pr}`3957`, {pr}`3964`, {pr}`3987`, {pr}`3990`, {pr}`3210`

- {{ Fix }} `jsarray.pop` now works correctly. It previously returned the wrong
value and leaked memory.
{pr}`4236`

### Packages

- Added `river` version 0.19.0 {pr}`4197`
Expand Down
2 changes: 1 addition & 1 deletion src/core/error_handling.h
View file
Expand Up @@ -199,7 +199,7 @@ console_error_obj(JsRef obj);

#define FAIL_IF_JS_NULL(ref) \
do { \
if (unlikely(Jsv_is_null(ref))) { \
if (unlikely(JsvNull_Check(ref))) { \
FAIL(); \
} \
} while (0)
Expand Down
207 changes: 0 additions & 207 deletions src/core/hiwire.c
View file
Expand Up @@ -235,38 +235,12 @@ EM_JS(void _Py_NO_RETURN, hiwire_throw_error, (JsRef iderr), {
throw Hiwire.pop_value(iderr);
});

static JsRef
convert_va_args(va_list args)
{
JsRef idargs = JsArray_New();
while (true) {
JsRef idarg = va_arg(args, JsRef);
if (idarg == NULL) {
break;
}
JsArray_Push_unchecked(idargs, idarg);
}
va_end(args);
return idargs;
}

EM_JS_REF(JsRef, hiwire_call, (JsRef idfunc, JsRef idargs), {
let jsfunc = Hiwire.get_value(idfunc);
let jsargs = Hiwire.get_value(idargs);
return Hiwire.new_value(jsfunc(... jsargs));
});

JsRef
hiwire_call_va(JsRef idobj, ...)
{
va_list args;
va_start(args, idobj);
JsRef idargs = convert_va_args(args);
JsRef idresult = hiwire_call(idobj, idargs);
hiwire_decref(idargs);
return idresult;
}

EM_JS_REF(JsRef, hiwire_call_OneArg, (JsRef idfunc, JsRef idarg), {
let jsfunc = Hiwire.get_value(idfunc);
let jsarg = Hiwire.get_value(idarg);
Expand Down Expand Up @@ -377,28 +351,6 @@ hiwire_CallMethodId(JsRef idobj, Js_Identifier* name_id, JsRef idargs)
return hiwire_CallMethod(idobj, name_ref, idargs);
}

JsRef
hiwire_CallMethodString_va(JsRef idobj, const char* ptrname, ...)
{
va_list args;
va_start(args, ptrname);
JsRef idargs = convert_va_args(args);
JsRef idresult = hiwire_CallMethodString(idobj, ptrname, idargs);
hiwire_decref(idargs);
return idresult;
}

JsRef
hiwire_CallMethodId_va(JsRef idobj, Js_Identifier* name, ...)
{
va_list args;
va_start(args, name);
JsRef idargs = convert_va_args(args);
JsRef idresult = hiwire_CallMethodId(idobj, name, idargs);
hiwire_decref(idargs);
return idresult;
}

JsRef
hiwire_CallMethodId_NoArgs(JsRef obj, Js_Identifier* name)
{
Expand Down Expand Up @@ -511,32 +463,6 @@ hiwire_get_length(JsRef idobj)
return -1;
}

EM_JS_BOOL(bool, hiwire_get_bool, (JsRef idobj), {
let val = Hiwire.get_value(idobj);
// clang-format off
if (!val) {
return false;
}
// We want to return false on container types with size 0.
if (val.size === 0) {
if(/HTML[A-Za-z]*Element/.test(getTypeTag(val))){
// HTMLSelectElement and HTMLInputElement can have size 0 but we still
// want to return true.
return true;
}
// I think other things with a size are container types.
return false;
}
if (val.length === 0 && JsArray_Check(idobj)) {
return false;
}
if (val.byteLength === 0) {
return false;
}
return true;
// clang-format on
});

EM_JS_BOOL(bool, hiwire_is_generator, (JsRef idobj), {
// clang-format off
return getTypeTag(Hiwire.get_value(idobj)) === "[object Generator]";
Expand Down Expand Up @@ -694,139 +620,6 @@ EM_JS_REF(JsRef, hiwire_subarray, (JsRef idarr, int start, int end), {
return Hiwire.new_value(jssub);
});

// ==================== JsArray API ====================

EM_JS_BOOL(bool, JsArray_Check, (JsRef idobj), {
let obj = Hiwire.get_value(idobj);
if (Array.isArray(obj)) {
return true;
}
let typeTag = getTypeTag(obj);
// We want to treat some standard array-like objects as Array.
// clang-format off
if(typeTag === "[object HTMLCollection]" || typeTag === "[object NodeList]"){
// clang-format on
return true;
}
// What if it's a TypedArray?
// clang-format off
if (ArrayBuffer.isView(obj) && obj.constructor.name !== "DataView") {
// clang-format on
return true;
}
return false;
});

// clang-format off
EM_JS_REF(JsRef, JsArray_New, (), {
return Hiwire.new_value([]);
});
// clang-format on

EM_JS_NUM(errcode, JsArray_Push, (JsRef idarr, JsRef idval), {
Hiwire.get_value(idarr).push(Hiwire.get_value(idval));
});

EM_JS(int, JsArray_Push_unchecked, (JsRef idarr, JsRef idval), {
const arr = Hiwire.get_value(idarr);
arr.push(Hiwire.get_value(idval));
return arr.length - 1;
});

EM_JS_NUM(errcode, JsArray_Extend, (JsRef idarr, JsRef idvals), {
Hiwire.get_value(idarr).push(... Hiwire.get_value(idvals));
});

EM_JS_REF(JsRef, JsArray_Get, (JsRef idobj, int idx), {
let obj = Hiwire.get_value(idobj);
let result = obj[idx];
// clang-format off
if (result === undefined && !(idx in obj)) {
// clang-format on
return ERROR_REF;
}
return Hiwire.new_value(result);
});

EM_JS_NUM(errcode, JsArray_Set, (JsRef idobj, int idx, JsRef idval), {
Hiwire.get_value(idobj)[idx] = Hiwire.get_value(idval);
});

EM_JS_NUM(errcode, JsArray_Delete, (JsRef idobj, int idx), {
let obj = Hiwire.get_value(idobj);
// Weird edge case: allow deleting an empty entry, but we raise a key error if
// access is attempted.
if (idx < 0 || idx >= obj.length) {
return ERROR_NUM;
}
obj.splice(idx, 1);
});

EM_JS_REF(JsRef, JsArray_Splice, (JsRef idobj, int idx), {
let obj = Hiwire.get_value(idobj);
// Weird edge case: allow deleting an empty entry, but we raise a key error if
// access is attempted.
if (idx < 0 || idx >= obj.length) {
return 0;
}
return Hiwire.new_value(obj.splice(idx, 1));
});

// clang-format off
EM_JS_REF(JsRef,
JsArray_slice,
(JsRef idobj, int length, int start, int stop, int step),
{
let obj = Hiwire.get_value(idobj);
let result;
if (step === 1) {
result = obj.slice(start, stop);
} else {
result = Array.from({ length }, (_, i) => obj[start + i * step]);
}
return Hiwire.new_value(result);
});

EM_JS_NUM(errcode,
JsArray_slice_assign,
(JsRef idobj, int slicelength, int start, int stop, int step, int values_length, PyObject **values),
{
let obj = Hiwire.get_value(idobj);
let jsvalues = [];
for(let i = 0; i < values_length; i++){
let ref = _python2js(DEREF_U32(values, i));
if(ref === 0){
return -1;
}
jsvalues.push(Hiwire.pop_value(ref));
}
if (step === 1) {
obj.splice(start, slicelength, ...jsvalues);
} else {
if(values !== 0) {
for(let i = 0; i < slicelength; i ++){
obj.splice(start + i * step, 1, jsvalues[i]);
}
} else {
for(let i = slicelength - 1; i >= 0; i --){
obj.splice(start + i * step, 1);
}
}
}
});
// clang-format on

EM_JS_NUM(errcode, JsArray_Clear, (JsRef idobj), {
let obj = Hiwire.get_value(idobj);
obj.splice(0, obj.length);
})

EM_JS_NUM(JsRef, JsArray_ShallowCopy, (JsRef idobj), {
const obj = Hiwire.get_value(idobj);
const res = ("slice" in obj) ? obj.slice() : Array.from(obj);
return Hiwire.new_value(res);
})

// ==================== JsObject API ====================

// clang-format off
Expand Down
95 changes: 0 additions & 95 deletions src/core/hiwire.h
View file
Expand Up @@ -147,16 +147,6 @@ hiwire_call(JsRef idobj, JsRef idargs);
JsRef
hiwire_call_OneArg(JsRef idobj, JsRef idarg);

/**
* Call a function
*
* Arguments are specified as a NULL-terminated variable arguments list of
* JsRefs.
*
*/
JsRef
hiwire_call_va(JsRef idobj, ...);

JsRef
hiwire_call_bound(JsRef idfunc, JsRef idthis, JsRef idargs);

Expand Down Expand Up @@ -187,38 +177,19 @@ hiwire_CallMethodString(JsRef obj, const char* name, JsRef args);
JsRef
hiwire_CallMethodString_OneArg(JsRef obj, const char* name, JsRef arg);

/**
* name is the method name, as null-terminated UTF8.
* Arguments are specified as a NULL-terminated variable arguments list of
* JsRefs.
*/
JsRef
hiwire_CallMethodString_va(JsRef obj, const char* name, ...);

JsRef
hiwire_CallMethod(JsRef obj, JsRef name, JsRef args);

JsRef
hiwire_CallMethod_OneArg(JsRef obj, JsRef name, JsRef arg);

JsRef
hiwire_CallMethod_va(JsRef obj, JsRef name, ...);

/**
* name is the method name, as a Js_Identifier
* args is a hiwire Array containing the arguments.
*/
JsRef
hiwire_CallMethodId(JsRef obj, Js_Identifier* name, JsRef args);

/**
* name is the method name, as a Js_Identifier
* Arguments are specified as a NULL-terminated variable arguments list of
* JsRefs.
*/
JsRef
hiwire_CallMethodId_va(JsRef obj, Js_Identifier* name, ...);

JsRef
hiwire_CallMethodId_NoArgs(JsRef obj, Js_Identifier* name);

Expand Down Expand Up @@ -403,72 +374,6 @@ hiwire_get_buffer_info(JsRef idobj,
JsRef
hiwire_subarray(JsRef idarr, int start, int end);

// ==================== JsArray API ====================

bool
JsArray_Check(JsRef idobj);

/**
* Create a new JavaScript Array.
*
* Returns: New reference
*/
JsRef
JsArray_New();

/**
* Push a value to the end of a JavaScript array.
*/
errcode WARN_UNUSED
JsArray_Push(JsRef idobj, JsRef idval);

/**
* Same as JsArray_Push but panics on failure
*/
int
JsArray_Push_unchecked(JsRef idobj, JsRef idval);

errcode
JsArray_Extend(JsRef idobj, JsRef idvals);

JsRef
JsArray_ShallowCopy(JsRef idobj);

/**
* Get an object member by integer.
*
* Returns: New reference
*/
JsRef
JsArray_Get(JsRef idobj, int idx);

/**
* Set an object member by integer.
*/
errcode WARN_UNUSED
JsArray_Set(JsRef idobj, int idx, JsRef idval);

errcode WARN_UNUSED
JsArray_Delete(JsRef idobj, int idx);

JsRef
JsArray_Splice(JsRef idobj, int idx);

JsRef
JsArray_slice(JsRef idobj, int slicelength, int start, int stop, int step);

errcode
JsArray_slice_assign(JsRef idobj,
int slicelength,
int start,
int stop,
int step,
int values_length,
PyObject** values);

errcode
JsArray_Clear(JsRef idobj);

// ==================== JsObject API ====================

/**
Expand Down

0 comments on commit a41e267

Please sign in to comment.