enable lambda-lifting for the JavaScript target #586
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
saem
added
enhancement
saem
approved these changes
Mar 15, 2023
There was a problem hiding this comment.
This is awesome, good to merge! 🚀
I thought of two follow-ups. I'll start on the first one ~tomorrow and see about the second one based on the outcome:
- have as much of
tests/lang_callable/iterrun for js/all platforms see where things land - quick update of the manual to remove the "big" mentions of can't use closure iterators for JS (rest can be long tail)
Summary ======= Previously, inner closure procedures were not lifted into free-standing procedures during `transf`, but rather kept as is and mapped to nested JavaScript functions by `jsgen`. Besides requiring the JavaScript target to be special-cased in `transf` and `lambdalifting`, this also caused observable semantic issues: the destructors for captured variables were called when leaving the scope in which they were defined, not when the closure got destroyed. In order to bring the closure semantics in line with the other targets and as a prerequisite for full ORC support, lambda-lifting is now also enabled when using the JS target -- the code generator is adjusted to support the lifted procedures. The code generator changes are also a preparation for full closure iterator support. As a side-effect, closures now also work as expected when using the JS target and executing non-compile-time-only procedures that have inner closure procedures at compile-time. Details ======= - remove the dispatching based on the selected backend from `transf` and `lambdalifting` - remove all code related to the previous closure support from `jsgen` - implement support for lifted procedures in `jsgen` The hidden environment parameter of routines using the `.closure` calling convention is mapped to the this parameter at the JavaScript level. When a NimSkull closure object is created, a new function instance is created via `bind` and the provided environment bound to the `this` parameter. In addition, the both the function and environment are also assigned to the respective prc and env properties on the function object -- this allows for later retrieval. Compared to using an object, this approach has the benefit that closures can still be passed to an imported procedure that expects a JavaScript function object, without requiring any changes to the user code.
zerbina
force-pushed
the
lambda-lifting-for-js
branch
from
62a750d
to
3de5e7f
Compare
Yep, that's a good idea. Having the |
|
bors r+ |
|
Build succeeded: |
Merged
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.
Summary
Previously, inner closure procedures were not lifted into free-standing procedures during
transf, but rather kept as-is and mapped to nested JavaScript functions byjsgen. Besides requiring the JavaScript target to be special-cased intransfandlambdalifting, this also caused observable semantic issues: the destructors for captured variables were called when leaving the scope in which they were defined, not when the closure got destroyed.In order to bring the closure semantics in line with the other targets and as a prerequisite for full ORC support, lambda-lifting is now also enabled when using the JS target -- the code generator is adjusted to support the lifted procedures. The code generator changes are also a
preparation for full closure iterator support.
As a side-effect, closures now also work as expected when using the JS target and executing non-compile-time-only procedures that have inner closure procedures at compile-time.
Details
transfandlambdaliftingjsgenjsgenThe hidden environment parameter of routines using the
.closurecalling convention is mapped to thethisparameter at the JavaScript level.When a NimSkull closure object is created, a new function instance is created via
bindand the provided environment bound to thethisparameter. In addition, the both the function and environment are also assigned to the respectiveprcandenvproperties on the function object -- this allows for later retrieval.Compared to using an object, this approach has the benefit that closures can still be passed to an imported procedure that expects a JavaScript function object, without requiring any changes to the user code.
Notes for Reviewers