-
-
Notifications
You must be signed in to change notification settings - Fork 411
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
[Merged by Bors] - Make Realm
shareable between functions
#2801
Conversation
Test262 conformance changes
Fixed tests (40):
|
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.
I looked into everything but the code block, and it looks great to me :) good job!!
Codecov Report
@@ Coverage Diff @@
## main #2801 +/- ##
==========================================
+ Coverage 50.57% 51.41% +0.83%
==========================================
Files 415 416 +1
Lines 41207 41081 -126
==========================================
+ Hits 20841 21122 +281
+ Misses 20366 19959 -407
... and 7 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
Greak work! Really nice refactor of the call/construct code.
bors r+ |
This Pull Request fixes #2317 and #1835, finally giving our engine proper realms 🥳. It changes the following: - Extracts the compile environment stack from `Realm` and into `Vm`. - Adjusts the bytecompiler to accommodate this change. - Adjusts `call/construct_internal` to accommodate this change. This also coincidentally fixed #2317, which I'm pretty happy about. - Adjusts several APIs (`NativeJob`, `Realm`) and builtins (`eval`, initializers) to accommodate this change. - Adjusts `JsNativeError`s to hold a reference to the Realm from which they were created. This only affects errors created within calls to function objects. Native calls don't need to set the realm because it's inherited by the next outer active function object. TLDR: `JsError` API stays the same, we just set the origin Realm of errors in `JsObject::call/construct_internal`.
Pull request successfully merged into main. Build succeeded: |
Realm
shareable between functionsRealm
shareable between functions
This Pull Request fixes #2317 and #1835, finally giving our engine proper realms 🥳.
It changes the following:
Realm
and intoVm
.call/construct_internal
to accommodate this change. This also coincidentally fixed Panic while executingeval
inside block inside function #2317, which I'm pretty happy about.NativeJob
,Realm
) and builtins (eval
, initializers) to accommodate this change.JsNativeError
s to hold a reference to the Realm from which they were created. This only affects errors created within calls to function objects. Native calls don't need to set the realm because it's inherited by the next outer active function object. TLDR:JsError
API stays the same, we just set the origin Realm of errors inJsObject::call/construct_internal
.