search

WebAssembly / memory64

Memory with 64-bit indexes
Other
194 stars 21 forks source link

BigInt parameters for the JS API? #68

Open bvisness opened 2 days ago

bvisness commented 2 days ago

As I finalize our implementation of table64, I'd like some clarity on the JS API. Some previous comments (e.g. https://github.com/WebAssembly/memory64/issues/46#issuecomment-2072933689, https://github.com/WebAssembly/memory64/issues/46#issuecomment-2072979502) have suggested that .get, .set should accept both number and BigInt for their indices.

However, I'd like to clarify a couple things:

  1. i64 params must be BigInt (per ToWebAssemblyValue), so should BigInt be required for Table methods as well?
  2. What should Table.grow take? Memory.grow currently takes a number, even for memory64, despite memory.grow taking an i64. Perhaps this is just because the param is in units of pages? (But, if custom page sizes are coming down the pipe, this reasoning won't hold up for long.)

My suggestion would be for all of Table.get, Table.set, Table.grow, and Memory.grow to require BigInt when the index (address :slightly_smiling_face:) type is i64. This is consistent with ToWebAssemblyValue(v, i64), and therefore consistent with the core wasm versions of these instructions.

Altering Memory.grow should also be backwards-compatible at this stage since memory32 is unaffected - although it may cause slight difficulty for toolchains in practice. Worth it imo (but I am not a toolchain author).

rossberg commented 2 days ago

The interaction of this proposal with custom page sizes is a good point! I think we have to make memory.grow/size use the index type to count pages in this proposal, otherwise it will not be forward-compatible with custom page sizes, and impossible to fix later. I opened #69 for that.