Support wasip3 WIT changes in WAC parser#210
Conversation
|
Sorry about closing #180 (that was my mistake)! This is great, thanks for tackling this! |
no problem at all, I was worried I was missing something haha |
|
@ryan-surname-p should i review this in its current state or are you planning to bring wac-parser up-to-date first? |
|
Hey @fibonacci1729, I was actually under the impression that by updating the This bridges the gap between The backend already understood these keywords. What was missing was purely the frontend surface to write them in a So I'm ready for a review as-is and if I missed something important please call it out and I'll take care of it as best I can. |
| ), | ||
| )) | ||
| } else if lookahead.peek(Token::StreamKeyword) { | ||
| let span = lexer.next().unwrap().1; |
There was a problem hiding this comment.
nit: The stream and future parse arms here are nearly identical. I'll leave it up to you but it might be nice to define a helper func.
| func_results: &ast::ResultList<'a>, | ||
| kind: FuncKind, | ||
| resource: Option<ResourceId>, | ||
| is_async: bool, |
There was a problem hiding this comment.
nit: Positional booleans are mildly error-prone at call sites. I would consider adding something like:
#[derive(Clone, Copy, PartialEq, Eq)]
enum Async { No, Yes }
impl From<bool> for Async {
fn from(b: bool) -> Self { if b { Async::Yes } else { Async::No } }
}
But again, ill leave this up to you!
There was a problem hiding this comment.
This looks great!
Just two nits and few things to consider/note:
-
It would be nice to add a negative test asserting that
error-context<u8>(error-context takes no payload) or an emptystream<>is rejected. -
This update is technically a breaking change given the new reserved keywords so we just need to remember to call this out in the release notes and cut the next release as
v0.11.0.
2 participants
Closes #180 (I thought, it appears to be closed so maybe this enhances 180?)
This PR brings the WAC source language up to date with the WIT additions needed to express wasip3 components.
Since updating
wasm-toolsbothwac-typesandwac-graphhave added support for WIT keywordsasync,stream,futureanderror-context.What's left is to bring
wac-parserup to date withwac-typesandwax-graph.