Skip to content

Implement portable variants of TypedArray.wrap #1309

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

Merged
merged 2 commits into from
May 29, 2020

Conversation

jletellier
Copy link
Contributor

@jletellier jletellier commented May 29, 2020

Implements #1304

  • I've read the contributing guidelines

@MaxGraey
Copy link
Member

MaxGraey commented May 29, 2020

@dcodeIO wdyt should it also optionally add for BigInt64Array and BigUint64Array?

@dcodeIO
Copy link
Member

dcodeIO commented May 29, 2020

Hmm, good question. There is no good i64 story yet, and portable doesn't yet try to polyfill any of that, including the basic type. I suggest leaving the PR as-is for now, and think about better portable i64 support in general later, for example we'd also have to alias BigInt64Array to Int64Array first.

Copy link
Member

@MaxGraey MaxGraey left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@dcodeIO dcodeIO merged commit 7d133cc into AssemblyScript:master May 29, 2020
@dcodeIO
Copy link
Member

dcodeIO commented May 29, 2020

Perhaps on a second note, one way to polyfill i64 in general might be to hijack the i64.xy set of instructions, so one can write i64.add(x, y) etc. in both JS and Wasm to make it work. That would also fit well into the compiler I guess, with an i64 being a Long.js object in JS. However, there are no inline-assembler variants for very basics ops yet, like the mentioned i64.add, but that shouldn't be too hard to add (just a lot of work to add all of them for i32, f32, f64 etc.).

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

Successfully merging this pull request may close these issues.

3 participants