Skip to content
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.

Sign up for GitHub

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

Jump to bottom

BigUint64Array/BigInt64Array should be constructed from u64 arrays, not BigInt arrays #3009

Closed
ishitatsuyuki opened this issue Jul 28, 2022 · 2 comments · Fixed by #3011
Closed

BigUint64Array/BigInt64Array should be constructed from u64 arrays, not BigInt arrays #3009

ishitatsuyuki opened this issue Jul 28, 2022 · 2 comments · Fixed by #3011
Labels
breaking-change Tracking breaking changes for the next major version bump (if ever) bug

Comments

@ishitatsuyuki
Copy link
Contributor

In https://docs.rs/js-sys/latest/js_sys/struct.BigUint64Array.html, various methods like pub fn copy_from(&self, src: &[BigInt]) takes a slice of BigInts. But BigInt is an object, and this just doesn't look right. When indexed from JS code they should indeed return a BigInt object, but from the view of wasm their representation should be a u64.
@Liamolucko Liamolucko added the breaking-change Tracking breaking changes for the next major version bump (if ever) label Jul 29, 2022
@Liamolucko
Copy link
Collaborator

You're right, but unfortunately it's a breaking change to fix it.

@ishitatsuyuki
Copy link
Contributor Author

Well, does the old signature even work? It sounds like it will read garbage since the type signature doesn't make much sense?

ishitatsuyuki added a commit to ishitatsuyuki/wasm-bindgen that referenced this issue Jul 29, 2022
@ishitatsuyuki
Convert Big[Ui,I]nt64Array bindings to use [u/i]64
32f6081 
Using BigInt as the macro parameters leads to absurd signatures like
slices of BigInt objects (can't exist). Instead, just use the numeric
types. bindgen already converts them to bigint when appropriate.

Closes: rustwasm#3009
Fixes: d6d056c ("Add math-related intrinsics/functions for `JsValue`s (rustwasm#2629)")
@ishitatsuyuki ishitatsuyuki mentioned this issue Jul 29, 2022
Merged
ishitatsuyuki added a commit to ishitatsuyuki/wasm-bindgen that referenced this issue Jul 29, 2022
@ishitatsuyuki
Convert Big[Ui,I]nt64Array bindings to use [u/i]64
6f3323e 
Using BigInt as the macro parameters leads to absurd signatures like
slices of BigInt objects (can't exist). Instead, just use the numeric
types. bindgen already converts them to bigint when appropriate.

Closes: rustwasm#3009
Fixes: d6d056c ("Add math-related intrinsics/functions for `JsValue`s (rustwasm#2629)")
alexcrichton pushed a commit that referenced this issue Jul 29, 2022
@ishitatsuyuki
Convert Big[Ui,I]nt64Array bindings to use [u/i]64 (#3011)
6daa3d1 
Using BigInt as the macro parameters leads to absurd signatures like
slices of BigInt objects (can't exist). Instead, just use the numeric
types. bindgen already converts them to bigint when appropriate.

Closes: #3009
Fixes: d6d056c ("Add math-related intrinsics/functions for `JsValue`s (#2629)")
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
breaking-change Tracking breaking changes for the next major version bump (if ever) bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Convert Big[Ui,I]nt64Array bindings to use [u/i]64 ishitatsuyuki/wasm-bindgen
2 participants
@ishitatsuyuki @Liamolucko