-
Notifications
You must be signed in to change notification settings - Fork 687
Add %TypedArray%.prototype.subarray([ begin [, end ] ]) support. #2410
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
Add %TypedArray%.prototype.subarray([ begin [, end ] ]) support. #2410
Conversation
|
|
||
| if (ecma_is_value_empty (ret_value)) | ||
| { | ||
| ret_value = ecma_typedarray_helper_dispatch_construct (arguments_p, 3, src_builtin_id); |
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.
On this line I get the following assertion failure:
ICE: Assertion 'ecma_typedarray_helper_is_typedarray (builtin_id)' failed at /Users/acalandra/jerryscript/jerry-core/ecma/builtin-objects/typedarray/ecma-builtin-typedarray-helpers.c(ecma_typedarray_helper_dispatch_construct):180.
Error: ERR_FAILED_INTERNAL_ASSERTION
Some help solving this issue would be greatly appreciated as I can't seem to figure out why this isn't a typedarray.
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.
@AnthonyCalandra The main reason is that src_builtin_id is incorrrent and that causes the assertion error.
uint8_t src_builtin_id = ecma_get_object_builtin_id (src_typedarray_p); returns with ECMA_BUILTIN_ID__COUNT which means the object is not builtin.
Currently I cannot find any helper function in the code base to get the builtin_id from a typedarray object, so my suggestion is to create a function like:
static uint8_t
ecma_typedarray_helper_get_builtin_id (ecma_object_t *obj_p) /**< typedarray object **/
{
#define TYPEDARRAY_ID_CASE(magic_id, builtin_id) \
case magic_id: \
{ \
return builtin_id; \
}
switch (ecma_object_get_class_name (obj_p))
{
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_INT8_ARRAY_UL, ECMA_BUILTIN_ID_INT8ARRAY)
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_UINT8_ARRAY_UL, ECMA_BUILTIN_ID_UINT8ARRAY)
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_UINT8_CLAMPED_ARRAY_UL, ECMA_BUILTIN_ID_UINT8CLAMPEDARRAY)
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_INT16_ARRAY_UL, ECMA_BUILTIN_ID_INT16ARRAY)
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_UINT16_ARRAY_UL, ECMA_BUILTIN_ID_UINT16ARRAY)
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_INT32_ARRAY_UL, ECMA_BUILTIN_ID_INT32ARRAY)
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_UINT32_ARRAY_UL, ECMA_BUILTIN_ID_UINT32ARRAY)
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_FLOAT32_ARRAY_UL, ECMA_BUILTIN_ID_FLOAT32ARRAY)
#if CONFIG_ECMA_NUMBER_TYPE == CONFIG_ECMA_NUMBER_FLOAT64
TYPEDARRAY_ID_CASE (LIT_MAGIC_STRING_FLOAT64_ARRAY_UL, ECMA_BUILTIN_ID_FLOAT64ARRAY)
#endif /* CONFIG_ECMA_NUMBER_TYPE == CONFIG_ECMA_NUMBER_FLOAT64 */
default:
{
JERRY_UNREACHABLE ();
}
}
#undef TYPEDARRAY_ID_CASE
} /* ecma_typedarray_helper_get_builtin_id */
(Note: this function is the opposite of ecma_typedarray_helper_get_magic_string (uint8_t builtin_id))
I hope it solves your problem!
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.
Thanks Robert, I found your comment very helpful.
874722f to
11b3bb8
Compare
e77e12d to
ec5c08b
Compare
| if ((int64_t) end_index_uint32 - begin_index_uint32 > 0) | ||
| { | ||
| subarray_length = end_index_uint32 - begin_index_uint32; | ||
| } |
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 think the int64_t cast can be eliminated like this:
ecma_length_t subarray_length = 0;
if (end_index_uint32 > begin_index_uint32)
{
// Cannot underflow
subarray_length = end_index_uint32 - begin_index_uint32;
}
// Otherwise the subarray_length must be 0
ec5c08b to
445df4f
Compare
| } | ||
|
|
||
| ECMA_OP_TO_NUMBER_FINALIZE (relative_begin); | ||
|
|
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.
We should return here if ret_value contains an error. The ECMA_OP_TO_NUMBER_TRY_CATCH might fail, so it can fill the ret_value with an error.
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.
Done
445df4f to
185674f
Compare
LaszloLango
left a comment
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.
LGTM
| * limitations under the License. | ||
| */ | ||
|
|
||
| var a = new Int32Array([1, 2, 3, 4, 5]); |
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.
What happens if the original Int32Array is mapped only to a subarray of the arraybuffer? I would like a test case for that as well.
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.
Done. If the new tests aren't what you requested please put your request into pseudocode.
JerryScript-DCO-1.0-Signed-off-by: Anthony Calandra anthony@anthony-calandra.com
185674f to
b06bd84
Compare
zherczeg
left a comment
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.
LGTM
This patch attempts to add support for the
%TypedArray%.prototype.subarrayfunction according to Section 22.2.3.26 of the ES2015 spec.JerryScript-DCO-1.0-Signed-off-by: AnthonyCalandra anthony@anthony-calandra.com