-
-
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] - Create new lazy Error type #2283
Conversation
Test262 conformance changesVM implementation
Fixed tests (4):
Broken tests (4):
|
Codecov Report
@@ Coverage Diff @@
## main #2283 +/- ##
==========================================
- Coverage 40.14% 39.84% -0.31%
==========================================
Files 241 242 +1
Lines 22966 23242 +276
==========================================
+ Hits 9219 9260 +41
- Misses 13747 13982 +235
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Benchmark for 1b283bdClick to view benchmark
|
4d84f7d
to
6a84f78
Compare
Benchmark for 75cf117Click to view benchmark
|
Benchmark for be6f36dClick to view benchmark
|
Benchmark for f7ecaa4Click to view benchmark
|
Benchmark for 761a735Click to view benchmark
|
8afcc6b
to
faac2f4
Compare
Finished documenting and adding examples. Let me know if I overlooked something :) |
Benchmark for 60b54c3Click to view benchmark
|
Benchmark for 36078ecClick to view benchmark
|
f558bac
to
5522db4
Compare
Benchmark for 27963bfClick to view benchmark
|
5522db4
to
9b53af3
Compare
Benchmark for 607203eClick to view benchmark
|
d99df0d
to
0ebf10d
Compare
Benchmark for 61aec7eClick to view benchmark
|
Benchmark for 83de1d4Click to view benchmark
|
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.
This is ready for me :) thank you!
It only needs a re-base.
Benchmark for 95b6f47Click to view benchmark
|
Benchmark for cbbdae0Click to view benchmark
|
Fix detection of Test262Error in boa_tester cargo update
Add doc examples Remove unnecessary whitespace in example
Benchmark for 8339f70Click to view benchmark
|
Benchmark for 6beb20cClick to view benchmark
|
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've gone through the 3 main files and a few of the other ones to get a feel for what the changes look like and I think it looks great, only had that nitpick with the documentation. Great job, seems like a really cool change.
bors r+ |
This is an experiment that tries to migrate the codebase from eager `Error` objects to lazy ones. In short words, this redefines `JsResult = Result<JsValue, JsError>`, where `JsError` is a brand new type that stores only the essential part of an error type, and only transforms those errors to `JsObject`s on demand (when having to pass them as arguments to functions or store them inside async/generators). This change is pretty big, because it unblocks a LOT of code from having to take a `&mut Context` on each call. It also paves the road for possibly making `JsError` a proper variant of `JsValue`, which can be a pretty big optimization for try/catch. A downside of this is that it exposes some brand new error types to our public API. However, we can now implement `Error` on `JsError`, making our `JsResult` type a bit more inline with Rust's best practices. ~Will mark this as draft, since it's missing some documentation and a lot of examples, but~ it's pretty much feature complete. As always, any comments about the design are very much appreciated! Note: Since there are a lot of changes which are essentially just rewriting `context.throw` to `JsNativeError::%type%`, I'll leave an "index" of the most important changes here: - [boa_engine/src/error.rs](https://github.com/boa-dev/boa/pull/2283/files#diff-f15f2715655440626eefda5c46193d29856f4949ad37380c129a8debc6b82f26) - [boa_engine/src/builtins/error/mod.rs](https://github.com/boa-dev/boa/pull/2283/files#diff-3eb1e4b4b5c7210eb98192a5277f5a239148423c6b970c4ae05d1b267f8f1084) - [boa_tester/src/exec/mod.rs](https://github.com/boa-dev/boa/pull/2283/files#diff-fc3d7ad7b5e64574258c9febbe56171f3309b74e0c8da35238a76002f3ee34d9)
Benchmark for d9361d9Click to view benchmark
|
Pull request successfully merged into main. Build succeeded: |
This is an experiment that tries to migrate the codebase from eager
Error
objects to lazy ones.In short words, this redefines
JsResult = Result<JsValue, JsError>
, whereJsError
is a brand new type that stores only the essential part of an error type, and only transforms those errors toJsObject
s on demand (when having to pass them as arguments to functions or store them inside async/generators).This change is pretty big, because it unblocks a LOT of code from having to take a
&mut Context
on each call. It also paves the road for possibly makingJsError
a proper variant ofJsValue
, which can be a pretty big optimization for try/catch.A downside of this is that it exposes some brand new error types to our public API. However, we can now implement
Error
onJsError
, making ourJsResult
type a bit more inline with Rust's best practices.Will mark this as draft, since it's missing some documentation and a lot of examples, butit's pretty much feature complete. As always, any comments about the design are very much appreciated!Note: Since there are a lot of changes which are essentially just rewriting
context.throw
toJsNativeError::%type%
, I'll leave an "index" of the most important changes here: