-
Notifications
You must be signed in to change notification settings - Fork 470
Add support for ArrayBuffer and typed arrays to @unboxed #7788
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
Conversation
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
| Some instance_type -> Some (InstanceType instance_type)) | ||
| {desc = Ttuple _} -> Some (InstanceType Array) | ||
| _ -> None | ||
(* First check the original (unexpanded) type for typed arrays and other instance types *) |
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.
The stdlib uses t = Stdlib_TypedArray.t<int>
and t = Stdlib_TypedArray.t<float>
for typed arrays, so we can't differentiate them by their expanded types.
@@ -1,5 +1,3 @@ | |||
module Array = Ocaml_Array |
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 removed this line and replaced arr[0]
with arr->Array.getUnsafe(0)
in the test to make compiling with ./cli/bsc.js tests/tests/src/UntaggedVariants.res
easier (bsc would complain about unknown dependency Ocaml_Array
otherwise)
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.
Can we have a summary of how these new types compare to other types in terms of runtime representation?
Are they all objects and are all instance of cases mutually exclusive (with each other and with existing types).
Just double checking where this extension sits wrt what's already there.
const buffer = new ArrayBuffer(8)
const view = new Int32Array(buffer)
console.log(typeof buffer) // 'object'
console.log(typeof view) // 'object' They can be differentiated with const buffer = new ArrayBuffer(8)
const view = new Int32Array(buffer)
console.log(buffer instanceof ArrayBuffer) // true
console.log(buffer instanceof Array) // false
console.log(Array.isArray(buffer)) // false
console.log(buffer instanceof DataView) // false
console.log(view instanceof Int32Array) // true
console.log(view instanceof Array) // false
console.log(Array.isArray(view)) // false
console.log(view instanceof ArrayBuffer) // false
console.log(view instanceof DataView) // false |
@mediremi Could you add a CHANGELOG entry for this PR? |
Closes #6757
While adding support for
ArrayBuffer.t
was simple, supporting typed arrays (e.g.Int8Array
) required changingAst_untagged_variants.get_block_type_from_typ
so it first looks at the unexpanded types (e.g.Int8Array
) rather than just the fully expanded version (e.g.Stdlib_TypedArray.t
).