Fixed a problem with super and nested lambdas
#1838
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
We discovered this problem in our fork, where we disable the "function peeling optimization", but we have managed to reproduce it with a unit test that mixes compiled and interpreted functions. The problem is that nested lambda does not have the `homeObject` set correctly. In the interpreter we pick it up from the containing `NativeCall` (the activation), but in some situations, like without function peeling or when going from a compiled function to an interpreted one, it might not be set. The fix we came up with is just to propagate the home object on the nested closure function, that gets wrapped in the `ArrowFunction`. It's not particularly elegant, but it seems to fix the problem, does not cause any regression, and it's pretty cheap at runtime. Co-authored-by: Satish Srinivasan <satish.srinivasan@servicenow.com>
|
Looks good to me and simple enough. Thanks! |
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.
We discovered this problem in our fork, where we disable the "function peeling optimization", but we have managed to reproduce it with a unit test that mixes compiled and interpreted functions.
The problem is that nested lambda does not have the
homeObjectset correctly. In the interpreter we pick it up from the containingNativeCall(the activation), but in some situations, like without function peeling or when going from a compiled function to an interpreted one, it might not be set.The fix we came up with is just to propagate the home object on the nested closure function, that gets wrapped in the
ArrowFunction. It's not particularly elegant, but it seems to fix the problem, does not cause any regression, and it's pretty cheap at runtime.