Skip to content

support composing with fixed size lists #156

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cpetig
Copy link

@cpetig cpetig commented Apr 20, 2025

This implements another part of WebAssembly/component-model#384 - needed for testing the code generation in wit-bindgen

@cpetig cpetig force-pushed the fixed-size-lists branch from 3d6c1a0 to 6934009 Compare May 5, 2025 21:46
@cpetig cpetig marked this pull request as ready for review May 5, 2025 21:48
@cpetig
Copy link
Author

cpetig commented May 5, 2025

This also updates to wasm-tools 0.230.0

@cpetig
Copy link
Author

cpetig commented May 5, 2025

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

@fibonacci1729
Copy link
Collaborator

@cpetig this LGTM; are we good to merge?

@cpetig
Copy link
Author

cpetig commented May 12, 2025

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
Copy link
Author

cpetig commented May 12, 2025

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.

@fibonacci1729
Copy link
Collaborator

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.

@cpetig
Copy link
Author

cpetig commented May 12, 2025

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.

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

Successfully merging this pull request may close these issues.

2 participants