-
Notifications
You must be signed in to change notification settings - Fork 45
feat(ecmascript): %TypedArray%.prototype.with #663
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
base: main
Are you sure you want to change the base?
Conversation
5a8ae43
to
719e273
Compare
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! Nice job with the garbage collector stuff
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.
Some minor fixes to be made, and then there's a big point of making the data copying more efficient. That'll need some work.
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
192e6a5
to
880408a
Compare
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.
Some issues remain. The code overall looks really good with the helper functions <3
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Outdated
Show resolved
Hide resolved
880408a
to
9a86cf0
Compare
9a86cf0
to
de3b5bf
Compare
de3b5bf
to
5c4d33e
Compare
It seems like the results differ between
By the way, the test in question is timing out:
Is it okay to skip this…? |
.../ecmascript/builtins/indexed_collections/typed_array_objects/typed_array_intrinsic_object.rs
Show resolved
Hide resolved
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! Real nice work, this really cleaned up a lot of the TA prototype methods related code which is great to see <3
doc: https://tc39.es/ecma262/multipage/indexed-collections.html#sec-%typedarray%.prototype.with
ref: #145