Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMove code from TypedArray::create and Add TypedArray::update #333
Conversation
|
We will definitely want a testcase added to |
| /// Update an existed JS typed array | ||
| pub unsafe fn update(length: u32, | ||
| data: Option<&[T::Element]>, | ||
| result: MutableHandleObject) { |
This comment has been minimized.
This comment has been minimized.
jdm
Feb 4, 2017
Member
Rather than making a static method like this, I recommend pub fn update(&mut self, data: &[T::Element]).
This comment has been minimized.
This comment has been minimized.
yysung1123
Feb 4, 2017
•
Author
Contributor
But if it isn't a static method, I can't call it from TypedArray::create.
This comment has been minimized.
This comment has been minimized.
jdm
Feb 4, 2017
Member
I argue that we can create a update_raw that encapsulates the relevant bit, and call that from both create and update (passing in a mutable destination buffer and an immutable source buffer).
|
Should I replace all |
|
Yes, that would lead to a cleaner API. |
yysung1123
force-pushed the
yysung1123:typedarray-update
branch
2 times, most recently
from
f712f19
to
648ff71
| @@ -124,22 +124,31 @@ impl<'a, T: TypedArrayElementCreator + TypedArrayElement> TypedArray<'a, T> { | |||
| /// be copied into the newly-allocated buffer. Returns the new JS reflector. | |||
| pub unsafe fn create(cx: *mut JSContext, | |||
| length: u32, | |||
| data: Option<&[T::Element]>, | |||
This comment has been minimized.
This comment has been minimized.
| TypedArray::<T>::update_raw(data, self.object.handle_mut()); | ||
| } | ||
|
|
||
| pub unsafe fn update_raw(data: &[T::Element], |
This comment has been minimized.
This comment has been minimized.
| } | ||
|
|
||
| pub unsafe fn update_raw(data: &[T::Element], | ||
| result: MutableHandleObject) { |
This comment has been minimized.
This comment has been minimized.
|
|
||
| typedarray!(in(cx) let mut array: Uint32Array = rval.get()); | ||
| array.as_mut().unwrap().update(Some(&[0, 2, 4, 6])); | ||
| assert_eq!(array.unwrap().as_slice(), &[0, 2, 4, 6, 0][..]); | ||
| } |
This comment has been minimized.
This comment has been minimized.
jdm
Feb 5, 2017
Member
Let's add another test in this file that uses #[should_panic] that calls update with an argument that is longer than the typed array that it is replacing, too.
|
Not sure why my last comment is marked outdated, but please look at it as well. |
yysung1123
force-pushed the
yysung1123:typedarray-update
branch
2 times, most recently
from
9017800
to
eed397f
|
About panic test? I have added it already! |
|
The test looks good. A couple small stylistic changes and this should be ready to go! |
| let buf = T::get_data(result.get()); | ||
| ptr::copy_nonoverlapping(data.as_ptr(), buf, data.len()); | ||
| if data.is_some() { | ||
| TypedArray::<T>::update_raw(data.unwrap(), result.handle()); |
This comment has been minimized.
This comment has been minimized.
yysung1123
force-pushed the
yysung1123:typedarray-update
branch
2 times, most recently
from
1f2e497
to
7388337
|
Conflicts resolved |
|
@bors-servo: r+ |
|
|
Move code from TypedArray::create and Add TypedArray::update For servo/servo#15350 ```TypedArray::update``` is used to be replace ```update_array_buffer_view``` <!-- Reviewable:start --> --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-mozjs/333) <!-- Reviewable:end -->
|
|
yysung1123 commentedFeb 4, 2017
•
edited by larsbergstrom
For servo/servo#15350
TypedArray::updateis used to be replaceupdate_array_buffer_viewThis change is