Lift a simple function into an operation safely. #879
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
neurosnap
reviewed
Jan 6, 2024
lib/lift.ts
Outdated
| *[Symbol.iterator]() { | ||
| return yield () => { | ||
| return shift(function* (k) { | ||
| k({ ok: true, value: fn(...args) }); |
There was a problem hiding this comment.
should this be k.tail since you don't care about the return value of k?
There is the potential for a function, if it is a continuation, to cause the scope in which it is called to be destroyed. In this case, nothing after that function should ever be called. However, because of the way `lift()` is currently implemented, this will not be the case. This unpacks the `lift()` function to be a single instruction that immediately returns value of invoking the function. However, because there is a `yield` point in the computation, it is an opportunity to not continue which will happen if corresponding frame is closed. At the same time, it removes the named types since they don't really add much clarity when you look at them from your language server.
neurosnap
approved these changes
Jan 6, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation
There is the potential for a function, if it is a continuation, to cause the scope in which it is called to be destroyed. In this case, nothing after that function should ever be called. However, because of the way
lift()is currently implemented, this will not be the case.Approach
This unpacks the
lift()function just a bit to return an operation with a single instruction that immediately returns the result of invoking the function. However, because there is ayieldpoint in the computation, it is an opportunity to not continue which will happen if corresponding frame is closed as a result of calling the function.You can see in the test case how when the spawned producer closes the signal, it cause its parent to finish up and return, meaning that the producer should shut down immediately and not continue executing.
At the same time, it removes the named types since they don't really add much clarity when you look at them from your language server.