clarify canonopt requirements for async lifts #575
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I had _thought_ that the spec allowed an async lift to omit the memory canonopt if only the result type (and not the paarameter types) required it, leaving the core function free to call any `task.return` at runtime as long as the type and string encoding matched. Indeed, that's what we implemented in `wasm-tools` and Wasmtime. However, it looks like I misremembered, since the spec uses an exact comparison in `LiftOptions.equal`. That's good news, because allowing the core function to call any of several `task.return` functions, each with a different memory, makes it hard for the runtime to optimize compositions. So no actual change needed here -- just a clarification. We'll update `wasm-tools` and `Wasmtime` to match. Signed-off-by: Joel Dice <[email protected]>
dicej
added a commit
to dicej/wasm-tools
that referenced
this pull request
Nov 17, 2025
Per WebAssembly/component-model#575, both `wasm-tools` and Wasmtime have been too permissive, allowing async lifts to optionally omit a memory canonopt if only the return type (and not the parameter types) would need one. This fixes validation to require the option in such cases. Signed-off-by: Joel Dice <[email protected]>
alexcrichton
approved these changes
Nov 17, 2025
github-merge-queue bot
pushed a commit
to bytecodealliance/wasm-tools
that referenced
this pull request
Nov 17, 2025
* require memory for async lifts as needed Per WebAssembly/component-model#575, both `wasm-tools` and Wasmtime have been too permissive, allowing async lifts to optionally omit a memory canonopt if only the return type (and not the parameter types) would need one. This fixes validation to require the option in such cases. Signed-off-by: Joel Dice <[email protected]> * Fix streams-and-futures test Fixing a mistake from #2376 * add memory option to lifts when needed in wit-component Signed-off-by: Joel Dice <[email protected]> --------- Signed-off-by: Joel Dice <[email protected]> Co-authored-by: Alex Crichton <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I had thought that the spec allowed an async lift to omit the memory canonopt if only the result type (and not the paarameter types) required it, leaving the core function free to call any
task.returnat runtime as long as the type and string encoding matched. Indeed, that's what we implemented inwasm-toolsand Wasmtime.However, it looks like I misremembered, since the spec uses an exact comparison in
LiftOptions.equal. That's good news, because allowing the core function to call any of severaltask.returnfunctions, each with a different memory, makes it hard for the runtime to optimize compositions.So no actual change needed here -- just a clarification. We'll update
wasm-toolsandWasmtimeto match.