-
Notifications
You must be signed in to change notification settings - Fork 116
feat: introduce await?
to short-circuit fulfilled async
futures
#5215
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
Conversation
Thinking about this some more, maybe we should not overload Inspired by your pretty printing of IE and the fact that we are actually tweaking Then your current await_sort also makes more sense I think. |
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.
Code looks good - I guess the final thing is to add documentation to the language manual. I.e. just add a line for await?
in the the syntax of terms, and copy and adapt the section on await
. Or treat both in the same section.
I wouldn't worry about the tutorial style doc pages for now (maybe just add a <!--TODO --->), though I guess a faster parallel await would be a nice example...
* adjust text * reword * reword * Update doc/md/15-language-manual.md * tweaks * Apply suggestions from code review Co-authored-by: Jessie Mongeon <[email protected]> --------- Co-authored-by: Gabor Greif <[email protected]> Co-authored-by: Jessie Mongeon <[email protected]>
src/ir_interpreter/interpret_ir.ml
Outdated
assert (not env.flavor.has_await && env.flavor.has_async_typ); | ||
begin match V.as_tup v2 with | ||
| [vf; vr] -> | ||
let (_, f) = V.as_func vf in | ||
let (_, r) = V.as_func vr in | ||
await env exp.at (V.as_async v1) | ||
await env exp.at (sort <> T.AwaitFut false)(*FIXME?*) (V.as_async v1) |
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.
FIXME?
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 point. It was a FIXME, but now it should be fine. I was unsure whether AwaitCmp
is possible here. It probably isn't, but we don't behave differently for that as before, even then.
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.
So maybe an assert just in case?
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.
No need, this code is working just fine for ages.
} catch _ {} | ||
}; | ||
|
||
public func state(shortCircuit : Bool) : async () { |
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.
It might be worth adding a version of this test (or entire file) where let a = async { throw Prim.error(..) }
(exiting via throw
, not return
) and we check the results accordingly.
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.
See 087c2c7!
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.
LGTM, but maybe add one more test for await? on async that exits via throw
…5215) Fixes #5212. Previously one had to use `await` (without the `*`) to force an `async` future to a value. The extended semantics implemented for `await?` will opt out of the the mandatory state commit coming with `await` and return the value from the future instantly if already fulfilled. The intuition for the syntax is that this causes 0 or 1 commit points (the former when the future is already fulfilled in an earlier `await` or `await?`). FYI: @mraszyk TODO: - [x] IR-interpreter - [x] docs
Fixes #5212.
Previously one had to use
await
(without the*
) to force anasync
future to a value. The extended semantics implemented forawait?
will opt out of the the mandatory state commit coming withawait
and return the value from the future instantly if already fulfilled.The intuition for the syntax is that this causes 0 or 1 commit points (the former when the future is already fulfilled in an earlier
await
orawait?
).FYI: @mraszyk
TODO: