-
Notifications
You must be signed in to change notification settings - Fork 82
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
Add 'non-blocking' function attribute #442
base: main
Are you sure you want to change the base?
Conversation
9bc797b
to
c8be8a4
Compare
c8be8a4
to
d1db043
Compare
@@ -1295,7 +1296,7 @@ typedef-item ::= resource-item | |||
|
|||
func-item ::= id ':' func-type ';' | |||
|
|||
func-type ::= 'func' param-list result-list |
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.
Could you add some text to Wit.md briefly describing what non-blocking
does, from a Wit user perspective? And maybe link back to the Explainer.md for the full details?
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.
Good idea, fixed, PTAL
``` | ||
If a resource-type has a potentially-blocking constructor, it can simply use | ||
`static new: func(...) -> my-resource` instead; `constructor` has no advantages | ||
beyond more-idiomatic bindings generation in some languages. |
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.
From a new Wit user perspective. the name "non-blocking" could sound like it means "no stopping the caller" rather than "no waiting for I/O", which would be confusing since it does the exact opposite of that :-}. At least we should clearly document the Wit-user-facing side of this keyword.
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.
Yeah, I suppose "blocking" does have multiple interpretations. I added this addition to the previous note, but it could probably be improved.
design/mvp/Explainer.md
Outdated
a `valtype`. | ||
a `valtype`. Function types can optionally be annotated with a `non-blocking` | ||
attribute which has no semantic effect and is ignored at validation- and | ||
run-time, serving primarily as a hint that tells bindings generators to lift |
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.
Is the "primarily" here redundant?
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.
Yeah, I suppose so. It felt a bit off without any word, so I tried "only", and also tightened up the rest of the sentence in this commit, PTAL
3bdea27
to
c81fbb2
Compare
Since this is just a hint for the binding generators and not actually enforced at runtime: as a WIT author I'd rather prefer the defaults to be swapped. I.e. functions should be generated as As an example I annotated the wasi-sockets proposal with both defaults:
Aside from the amount of work it is for me (which I can live with :P ), annotating only the "blocking" functions is more familiar to readers who have experience with |
I said the same thing when @lukewagner and I discussed this a while ago. I believe his response was that constructors are non-blocking by convention and that, once WIT has special syntax for resource getters and setters, those will also be non-blocking by convention, which should reduce the number of explicit annotations required. That said, I still tend to agree with you about swapping, since experience has show that functions in the average WIT interface that involve I/O are in the minority, even after you ignore the constructors, getters, and setters. |
Scanning that wasi-sockets PR, 28 of the 34 For the case of For the case of But for the last case, |
Regarding As for
I see your point (and the "but not hard" caveat), but I don't know how to extrapolate that guidance:
Having native syntax for getters & setters could indeed help alleviate many of the annotations in wasi-sockets. As you already counted out, that would bring the amount of needed annotations down from 34 to just 6. Versus 5. So: potáto, potàto. 🙃 My other point still stands though; this is a syntax is directed at binding generators / language implementors. IMO it still makes sense to speak "their" language, not "ours". From a JavaScript/Python/Rust/C#/.. developer's POV, seeing an |
This PR adds
non-blocking
tofunctype
(both in WAT and WIT). It's a pretty small addition and doesn't touch validation/runtime.While it initially seemed attractive to enforce
non-blocking
with trap-on-block semantics, there are valid scenarios where a callee might actually want/need to block and where the loss of concurrency in the caller is fine. Thus the PR proposes makingnon-blocking
just a hint (ignored by validation/runtime) to inform bindings generation (e.g., allowing bindings generators that make all functionsasync
by default emit non-async
functions fornon-blocking
).constructor
impliesnon-blocking
(sincenew
expressions in most languages can't be async). However, to avoid breaking wasip2, we don't (yet) require validation of[constructor]
-named functions to containnon-blocking
(adding this to the list of warts to remove in the next breaking change). In any case, bindings generators can always just take the non-blocking hint directly from seeing[constructor]
.