-
-
Notifications
You must be signed in to change notification settings - Fork 407
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] - Implement super
expressions
#2116
Conversation
Test262 conformance changesVM implementation
Fixed tests (496):
|
Codecov Report
@@ Coverage Diff @@
## main #2116 +/- ##
==========================================
- Coverage 43.52% 42.68% -0.84%
==========================================
Files 220 222 +2
Lines 20028 20763 +735
==========================================
+ Hits 8717 8863 +146
- Misses 11311 11900 +589
Continue to review full report at Codecov.
|
Benchmark for f4aa3ecClick to view benchmark
|
/// Parse the given source text in strict mode. | ||
pub(crate) fn parse_strict<S>(&mut self, src: S) -> Result<StatementList, ParseError> | ||
/// Parse the given source text with eval specific handling. | ||
pub(crate) fn parse_eval<S>( |
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.
We can probably use this in boa_tester
, in the $262
object that gets passed to the context. It has a special eval
function that currently does the parsing twice, but can probably benefit from this.
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.
You mean $262.evalScript()
right? I think that is different. This one is specific to the PerformEval
op: https://tc39.es/ecma262/#sec-performeval
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.
Ye, that's what I meant xD didn't remember it well. But yes, I get that. Maybe we can do the same somehow in evalScript()
.
Benchmark for bc073cbClick to view benchmark
|
This changes the trigger type for PR benchmarks back to the default (`opened`, `synchronize`, `reopened`). As part of #2114 I added the `labeled` trigger type. This causes the benchmarks to run when the `run-benchmark` label is present and another label is added. For example in #2116 I added the `run-benchmark` label while creating the PR. The benchmarks then where triggered six times; one for the PR creation (`opened`) and five times for each label that I initially added to the PR. The only drawback is that the benchmarks are not triggered, when we just add the label, but unfortunately I don't have a clever idea on how to achieve that right now. We will have to add the label and then trigger the run via a `synchronize` (push).
Ok(body) => body, | ||
Err(e) => return context.throw_syntax_error(e), | ||
Err(e) => return context.throw_syntax_error(e.to_string()), |
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.
That works too, yes, but my thoughts were more on using the following:
map_err(ToString::to_string)
instead of adding a closure in map_err
, but maybe it wasn't possible, due to by value/by reference function signatures.
Rebased. |
Benchmark for c10876aClick 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 reviewed it, but I'm not confident enough with the VM. Looks pretty good, but I expect a hit in Parser performance, due to the tree traversal.
I added some comments, mostly related to documentation, but now that I think of it, adding backticks everywhere might not make much sense xD just give it a look and let me know :)
boa_engine/src/syntax/parser/expression/primary/object_initializer/mod.rs
Show resolved
Hide resolved
boa_engine/src/syntax/parser/expression/primary/object_initializer/mod.rs
Show resolved
Hide resolved
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 Looks good from my side! :)
bors r+ |
🕐 Waiting for PR status (Github check) to be set, probably by CI. Bors will automatically try to run when all required PR statuses are set. |
Benchmark for 0353c96Click to view benchmark
|
bors r+ |
Pull request successfully merged into main. Build succeeded: |
super
expressionssuper
expressions
This Pull Request changes the following:
super
expression parsing / execution.super
expressions.