-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Minimal implementation of fixed-length lists to enable wit-bindgen runtime tests #10619
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
Conversation
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
DetailsTo modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
1cf1e10 to
c61a825
Compare
c63dd95 to
1cd3904
Compare
|
The best test for this functionality are the tests added to wit-bindgen in bytecodealliance/wit-bindgen#1277 . I can create a test for this one as well (I didn't find a good template in the existing tests), but perhaps it makes most sense to introduce tests together with the host support in #12315 . |
alexcrichton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it be possible to add a sample *.wast test which exercises this?
I guess it would be possible to make a two sub-component example which externally only offers a single boolean returning function, anything beyond this will depend on #12315 (e.g. passing an array to a function and checking the round-trip result). Judging from the amount of errors uncovered by a single wit-bindgen test I think |
|
That sounds reasonable to me yeah, and I think it's fine to expand the test coverage over time but I also think it'd be good to test what we can in this PR itself (which is, as you say, the composed-in-the-middle use) |
|
I added a moderately readable test (created in C++, then stripped down to 400 lines of wat). Though, I was unable to get Could it be that it usually sets properties according to the path of the test? Or is it supposed to work and I just missed something? |
|
Oh, that is a new to me error in CI: I guess I need to take a deeper look tomorrow. PS: The error on my computer is
|
|
Oh for that you'll want to add |
Subscribe to Label Actioncc @fitzgen DetailsThis issue or pull request has been labeled: "fuzzing", "wasmtime:api", "wasmtime:config"Thus the following users have been cc'd because of the following labels:
To subscribe or unsubscribe from this label, edit the |
My assumption was that this is caused by the component to component copying code https://github.com/bytecodealliance/wasmtime/pull/10619/files#diff-32f96d31ce61f8177ede00faf17bc565a8d15a00d42eb20b28a948a7a257dd87R2885-R2953 - I would assume that all adapters need multi-memory to work. |
I was looking for |
…ngth lists" changing wasm-tools (parser etc. is a different task)
|
😊 Oh, I took a look whether "fixed-size-list" or "fixed-size-lists" is the name in the component spec, it is "fixed-length lists". So, I made sure that wasmtime adheres to the standard name. Whether/how to change wasm-tools will be a separate discussion. |
Implement the necessary parts of fixed size lists (see WebAssembly/component-model#384 ) to enable runtime testing in wit-bindgen.