Skip to content
This repository has been archived by the owner on Nov 12, 2022. It is now read-only.

Usable DataView typed array bindings #453

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aditj
Copy link

@aditj aditj commented Feb 21, 2019

This PR tries to solve issue #406
The changes use the traits of a typed array but the APIs for Dataview object from mozjs are used.
The changes have not been tested , any suggested tests/changes would be appreciated. :)


This change is Reviewable

@jdm
Copy link
Member

jdm commented Feb 21, 2019

@aditj
Copy link
Author

aditj commented Feb 22, 2019

We can't use the TypedArrayElement and TypedArrayElementCreator traits since the functions of length_and_data and create_new would be different in case of a DataView.

@aditj
Copy link
Author

aditj commented Mar 2, 2019

We can't use the TypedArrayElement and TypedArrayElementCreator traits since the functions of length_and_data and create_new would be different in case of a DataView.

@jdm Help 🤕
Is there a way I could overload functions of a trait (The same way I would overload functions in an OOP Language), or is there any other alternative?

@jdm
Copy link
Member

jdm commented Mar 2, 2019

Could you explain the problem that you are having? What is the code you are trying to write that is not working?

$get_data: ident) => (
data_view_element!($t, $element, $unwrap, $length);
impl TypedArrayElementCreator for $t {
unsafe fn create_new(cx: *mut JSContext,array_buffer: *mut JSObject,byte_offset: u32,byte_length: u32) -> *mut JSObject {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jdm
This code creates a create_new function for data view object by extending the TypedArrayElement , and due to the mismatch in definition and prototype we are having errors like (Link to travis CI errors):

  1. for the length and data method
  2. for the create new method
    etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, you're right. I don't know if we can reuse the typed array support code for data views after all :(

@aditj
Copy link
Author

aditj commented Mar 2, 2019

Could you explain the problem that you are having? What is the code you are trying to write that is not working?

See this comment

@jdm
Copy link
Member

jdm commented Mar 2, 2019

I think we will have to create a new DataView type similar to TypedArray which directly wraps these APIs and then add a macro like typedarray! which creates a rooted instance.

@aditj
Copy link
Author

aditj commented Mar 3, 2019

I think we will have to create a new DataView type similar to TypedArray which directly wraps these APIs and then add a macro like typedarray! which creates a rooted instance.

I'll try doing that 👍

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #541) made this pull request unmergeable. Please resolve the merge conflicts.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants