wac icon indicating copy to clipboard operation
wac copied to clipboard

support composing with fixed size lists

Open cpetig opened this issue 8 months ago • 7 comments

This implements another part of https://github.com/WebAssembly/component-model/pull/384 - needed for testing the code generation in wit-bindgen

cpetig avatar Apr 20 '25 19:04 cpetig

This also updates to wasm-tools 0.230.0

cpetig avatar May 05 '25 21:05 cpetig

@alexcrichton if you look for updating wasm-tools in wac this pull request includes updating to 0.230.0

cpetig avatar May 05 '25 21:05 cpetig

@cpetig this LGTM; are we good to merge?

fibonacci1729 avatar May 12 '25 19:05 fibonacci1729

Closer inspection this evening showed that encoding.rs would need special handling for fixed size lists, so I guess it is incomplete as is, sorry!

cpetig avatar May 12 '25 21:05 cpetig

But honestly I don't trust my understanding of all the code base involved until the tests in wit-bindgen come out ok, which requires more work on wit-bindgen and wasmtime.

As long as it looks good to you I assume that it could work as is.

And the tests in wit-bindgen require support by wac.

cpetig avatar May 12 '25 22:05 cpetig

Sounds good. I'm happy to leave this open for awhile while the various codebases this touches solidifies a bit. Let's merge when we feel confident.

fibonacci1729 avatar May 12 '25 22:05 fibonacci1729

If you ever need to update wasm-tools (e.g. because of the tuple bomb prevention) this patch should work well enough, I just can't tell whether it is correct enough for full fixed size list support.

cpetig avatar May 12 '25 22:05 cpetig

With a lot more tests passing in https://github.com/cpetig/wit-bindgen/tree/work-in-progress and https://github.com/cpetig/wasmtime-adapter/tree/fixed-size-list , and support for fixed size lists in wasm-component-ld main, I am now confident enough that this patch is correct.

The matching wit-bindgen and wasmtime changes need more polishing before getting merged, though.

Update: I see some changes not originally intended by me (changing from wac 0.7.0-dev to wac 0.7.0 - I guess I merged another branch and should take a look at the history.

cpetig avatar Jul 12 '25 15:07 cpetig

I now removed the changes from the release-0.7.0 branch, which I assume is branching off to a stable branch and not intended to ever get merged into main.

This update will also give you the latest async dependencies (async is currently in the process of upstream into wasmtime), my guess is that another release of wasm-tools is imminent but should only bring you small fixes and performance improvements but no new functionality.

cpetig avatar Jul 12 '25 16:07 cpetig

In short: Good to merge :wink:

cpetig avatar Jul 12 '25 16:07 cpetig