-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
added Atomics and SharedArrayBuffer #1463
Conversation
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.
Looking good! I wonder if this could take a leaf from types like JSON to and have a mod Atomics { ... }
instead of a type Atomics
?
Looking great @ibaryshnikov! Thinking some more on this, what do you think of changing |
Oh, I didn't notice it in the beginning. I thought that |
Both u32 and i32 work but i32 doesn't need a job shim to switch the number to unsigned, so I think I'd lean towards i32 for this |
Fine. I also need to change |
That sounds right yeah, I sort of forget the specifics but if it works it works! |
…d_of to js_namespace, set typed_array type to Int32Array in notify and wait methods
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.
Did you still have changes you wanted to make or is this good to go?
I'd like to write tests, but I can do it in a separate pr, so we can merge this I think. |
@alexcrichton thank you for very useful comments! |
Added
Atomics
andSharedArrayBuffer
tojs-sys
.Ready for review.
Closes #1443