-
Notifications
You must be signed in to change notification settings - Fork 313
require memory for async lifts as needed #2382
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
require memory for async lifts as needed #2382
Conversation
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]>
|
I'll work on the test failures, but can you add a test along the lines of bytecodealliance/wasmtime#11726 to ensure that wit-component correctly adds the canonical options necessary for the lift too? |
Yeah, I realized there was more to do right after I opened this 🤦 |
Fixing a mistake from bytecodealliance#2376
|
Ok pushed up a commit which should resolve the test failure |
|
Also, on second thought, shouldn't this also require |
When would lifting ever require |
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.
Oops yes carry on, got my polarities swapped
Signed-off-by: Joel Dice <[email protected]>
Per WebAssembly/component-model#575, both
wasm-toolsand 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.