dicej opened PR #37 from dicej:component-async
to bytecodealliance:main
:
Add support for the Component Model Async ABI to
wasm-tools
,wit-bindgen
, andwasmtime
.
tschneidereit submitted PR review:
Hooray! :partying_face:
From a brief skim, the parts of this that I'm qualified to have an opinion on all look good to me. I left a couple of inline comments on specific aspects, but they're not particularly important.
tschneidereit created PR review comment:
I think to some degree we don't really need this kind of discussion: the "what else could we do" question has been answered at the spec level, and we'd have had a ton of opportunity to voice disagreement there. At this point, I think we should handle this the same as other features, such as the recent support for Tail Calls.
As such, I wonder if there are possible alternative approaches to implementing support of the spec feature that would be worth discussing here? And if not, is there a way to explain why that is the case as part of a "there really isn't much of an alternative" paragraph?
tschneidereit created PR review comment:
How (un)likely are scenarios in which all imports, but only some exports should be async, or vice-versa? If they might be reasonably likely, it might make sense to support
All
orNone
for one, andSome
for the other. I assume you have a much better intuition on this than I do, though
dicej updated PR #37.
dicej submitted PR review.
dicej created PR review comment:
Good point. If anything, I'd be tempted to remove the
All
option entirely, given that I expect most worlds will contain mostly non-blocking functions (e.g.wasi:http
which contains just two functions which actually do I/O:wasi:http/types#[static]body.finish
andwasi:http/handler#handle
). And I supposeNone
is even more redundant; we could reduce it all to:pub struct AsyncConfig { imports: HashSet<String>, exports: HashSet<String>, }
Note that the proposed
nonblocking
function attribute will allow us to do the right thing automatically without any configuration required.
dicej updated PR #37.
dicej edited PR #37:
Add support for the Component Model Async ABI to
wasm-tools
,wit-bindgen
, andwasmtime
.
dicej updated PR #37.
dicej merged PR #37.
Ugh, I definitely did _not_ mean to merge this. I must have pushed to origin/main by mistake. Sorry; will revert.
I've reverted all the earlier commits, rebased, and opened https://github.com/bytecodealliance/rfcs/pull/38 -- please direct further discussion there. Sorry again for the confusion!
BTW, would it make sense to tweak the branch protection rules in this repo to block pushing straight to main? Asking for a friend.
you can let your friend know that the main
branch is now protected, as it should be
Last updated: Nov 22 2024 at 17:03 UTC