Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Merged by Bors] - Refactor IdentifierReference parsing #2055

Closed
wants to merge 1 commit into from
Closed

[Merged by Bors] - Refactor IdentifierReference parsing #2055

wants to merge 1 commit into from

Conversation

raskad
Copy link
Member

@raskad raskad commented May 2, 2022

This Pull Request changes the following:

  • Crate dedicated IdentifierReference parser and refactor PrimaryExpression and PropertyDefinition parsers to use it.
  • Move BindingIdentifier and LabelIdentifier parsers from statement parser module to expression parser module to conform with the spec.
  • Add and early error case while converting an ObjectLiteral to an ObjectAssignmentPattern

@raskad raskad added enhancement New feature or request parser Issues surrounding the parser labels May 2, 2022
@raskad raskad added this to the v0.15.0 milestone May 2, 2022
@github-actions
Copy link

github-actions bot commented May 2, 2022

Test262 conformance changes

VM implementation

Test result main count PR count difference
Total 89,590 89,590 0
Passed 53,460 53,493 +33
Ignored 22,725 22,725 0
Failed 13,405 13,372 -33
Panics 0 0 0
Conformance 59.67% 59.71% +0.04%
Fixed tests (35): 
test/language/statements/class/class-name-ident-await.js [strict mode] (previously Failed)
test/language/statements/class/class-name-ident-await.js (previously Failed)
test/language/statements/class/class-name-ident-await-escaped.js [strict mode] (previously Failed)
test/language/statements/class/class-name-ident-await-escaped.js (previously Failed)
test/language/expressions/function/static-init-await-binding.js [strict mode] (previously Failed)
test/language/expressions/function/static-init-await-binding.js (previously Failed)
test/language/expressions/function/static-init-await-reference.js [strict mode] (previously Failed)
test/language/expressions/function/static-init-await-reference.js (previously Failed)
test/language/expressions/class/class-name-ident-await.js [strict mode] (previously Failed)
test/language/expressions/class/class-name-ident-await.js (previously Failed)
test/language/expressions/class/class-name-ident-await-escaped.js [strict mode] (previously Failed)
test/language/expressions/class/class-name-ident-await-escaped.js (previously Failed)
test/language/expressions/generators/static-init-await-binding.js [strict mode] (previously Failed)
test/language/expressions/generators/static-init-await-binding.js (previously Failed)
test/language/expressions/generators/static-init-await-reference.js [strict mode] (previously Failed)
test/language/expressions/generators/static-init-await-reference.js (previously Failed)
test/language/expressions/object/identifier-shorthand-await-strict-mode.js (previously Failed)
test/language/expressions/object/let-non-strict-syntax.js (previously Failed)
test/language/expressions/object/let-non-strict-access.js (previously Failed)
test/language/expressions/object/identifier-shorthand-static-init-await-valid.js [strict mode] (previously Failed)
test/language/expressions/object/identifier-shorthand-static-init-await-valid.js (previously Failed)
test/language/expressions/object/method-definition/static-init-await-reference-accessor.js [strict mode] (previously Failed)
test/language/expressions/object/method-definition/static-init-await-reference-accessor.js (previously Failed)
test/language/expressions/object/method-definition/static-init-await-binding-accessor.js [strict mode] (previously Failed)
test/language/expressions/object/method-definition/static-init-await-binding-accessor.js (previously Failed)
test/language/expressions/object/method-definition/static-init-await-binding-generator.js [strict mode] (previously Failed)
test/language/expressions/object/method-definition/static-init-await-binding-generator.js (previously Failed)
test/language/expressions/object/method-definition/static-init-await-reference-generator.js [strict mode] (previously Failed)
test/language/expressions/object/method-definition/static-init-await-reference-generator.js (previously Failed)
test/language/expressions/object/method-definition/static-init-await-binding-normal.js [strict mode] (previously Failed)
test/language/expressions/object/method-definition/static-init-await-binding-normal.js (previously Failed)
test/language/expressions/object/method-definition/static-init-await-reference-normal.js [strict mode] (previously Failed)
test/language/expressions/object/method-definition/static-init-await-reference-normal.js (previously Failed)
test/language/module-code/top-level-await/new-await-script-code.js [strict mode] (previously Failed)
test/language/module-code/top-level-await/new-await-script-code.js (previously Failed)
Broken tests (2): 
test/language/expressions/class/static-init-await-binding.js [strict mode] (previously Passed)
test/language/expressions/class/static-init-await-binding.js (previously Passed)

@codecov
Copy link

codecov bot commented May 2, 2022

Codecov Report

Merging #2055 (a24b428) into main (20f72d1) will increase coverage by 0.01%.
The diff coverage is 35.97%.

@@            Coverage Diff             @@
##             main    #2055      +/-   ##
==========================================
+ Coverage   44.16%   44.18%   +0.01%     
==========================================
  Files         212      214       +2     
  Lines       18812    18874      +62     
==========================================
+ Hits         8309     8339      +30     
- Misses      10503    10535      +32
Impacted Files Coverage Δ
...tax/parser/expression/assignment/arrow_function.rs 38.57% <ø> (ø)
boa_engine/src/syntax/parser/expression/mod.rs 33.33% <ø> (ø)
...xpression/primary/async_function_expression/mod.rs 40.00% <ø> (ø)
...pression/primary/async_generator_expression/mod.rs 40.00% <ø> (ø)
.../parser/expression/primary/class_expression/mod.rs 0.00% <ø> (ø)
...rser/expression/primary/function_expression/mod.rs 43.24% <ø> (ø)
...ser/expression/primary/generator_expression/mod.rs 48.71% <ø> (ø)
boa_engine/src/syntax/parser/function/mod.rs 32.01% <ø> (ø)
boa_engine/src/syntax/parser/mod.rs 39.68% <ø> (ø)
...ngine/src/syntax/parser/statement/break_stm/mod.rs 58.82% <ø> (ø)
... and 31 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 20f72d1...a24b428. Read the comment docs.

@github-actions
Copy link

github-actions bot commented May 2, 2022

Benchmark for 487c4a8

Click to view benchmark
Test Base PR %
Arithmetic operations (Compiler) 525.7±4.60ns 523.5±0.69ns -0.42%
Arithmetic operations (Execution) 644.2±4.03ns 632.7±0.32ns -1.79%
Arithmetic operations (Parser) 5.9±0.01µs 6.0±0.01µs +1.69%
Array access (Compiler) 1276.1±3.53ns 1280.6±2.99ns +0.35%
Array access (Execution) 7.7±0.02µs 7.7±0.02µs 0.00%
Array access (Parser) 12.6±0.10µs 13.0±0.02µs +3.17%
Array creation (Compiler) 1763.4±7.80ns 1778.0±3.70ns +0.83%
Array creation (Execution) 2.5±0.00ms 2.5±0.00ms 0.00%
Array creation (Parser) 14.3±0.02µs 15.0±0.02µs +4.90%
Array pop (Compiler) 3.8±0.01µs 3.7±0.01µs -2.63%
Array pop (Execution) 1109.5±6.18µs 1113.8±6.81µs +0.39%
Array pop (Parser) 144.5±0.08µs 146.6±0.13µs +1.45%
Boolean Object Access (Compiler) 1068.6±1.93ns 1068.8±2.55ns +0.02%
Boolean Object Access (Execution) 4.2±0.01µs 4.2±0.01µs 0.00%
Boolean Object Access (Parser) 15.0±0.01µs 15.9±0.11µs +6.00%
Clean js (Compiler) 3.3±0.01µs 3.4±0.01µs +3.03%
Clean js (Execution) 661.1±4.87µs 657.5±4.47µs -0.54%
Clean js (Parser) 30.9±0.03µs 32.3±0.03µs +4.53%
Create Realm 276.1±0.21ns 284.2±1.03ns +2.93%
Dynamic Object Property Access (Compiler) 1627.5±3.63ns 1644.2±23.15ns +1.03%
Dynamic Object Property Access (Execution) 5.4±0.02µs 5.3±0.02µs -1.85%
Dynamic Object Property Access (Parser) 11.1±0.02µs 11.5±0.02µs +3.60%
Fibonacci (Compiler) 2.3±0.01µs 2.3±0.01µs 0.00%
Fibonacci (Execution) 1330.1±2.90µs 1318.6±4.06µs -0.86%
Fibonacci (Parser) 17.0±0.02µs 17.8±0.02µs +4.71%
For loop (Compiler) 1938.0±17.29ns 1927.3±4.98ns -0.55%
For loop (Execution) 15.3±0.07µs 15.2±0.05µs -0.65%
For loop (Parser) 14.6±0.01µs 15.3±0.01µs +4.79%
Mini js (Compiler) 3.2±0.01µs 3.2±0.01µs 0.00%
Mini js (Execution) 627.5±3.13µs 625.3±4.85µs -0.35%
Mini js (Parser) 26.9±0.04µs 28.5±0.06µs +5.95%
Number Object Access (Compiler) 1023.3±2.05ns 1027.7±1.73ns +0.43%
Number Object Access (Execution) 3.3±0.01µs 3.2±0.01µs -3.03%
Number Object Access (Parser) 11.7±0.04µs 12.3±0.03µs +5.13%
Object Creation (Compiler) 1357.7±7.35ns 1401.8±15.79ns +3.25%
Object Creation (Execution) 5.1±0.02µs 5.0±0.01µs -1.96%
Object Creation (Parser) 9.6±0.03µs 10.0±0.02µs +4.17%
RegExp (Compiler) 1607.2±4.30ns 1621.7±4.26ns +0.90%
RegExp (Execution) 11.2±0.05µs 11.8±0.07µs +5.36%
RegExp (Parser) 10.7±0.02µs 11.1±0.03µs +3.74%
RegExp Creation (Compiler) 1388.7±3.81ns 1419.0±2.84ns +2.18%
RegExp Creation (Execution) 8.4±0.03µs 9.0±0.04µs +7.14%
RegExp Creation (Parser) 8.8±0.03µs 9.3±0.06µs +5.68%
RegExp Literal (Compiler) 1613.4±3.85ns 1611.5±4.86ns -0.12%
RegExp Literal (Execution) 11.2±0.06µs 11.9±0.11µs +6.25%
RegExp Literal (Parser) 8.6±0.02µs 9.0±0.01µs +4.65%
RegExp Literal Creation (Compiler) 1382.0±8.12ns 1389.7±4.07ns +0.56%
RegExp Literal Creation (Execution) 8.3±0.03µs 9.0±0.03µs +8.43%
RegExp Literal Creation (Parser) 6.7±0.02µs 7.1±0.01µs +5.97%
Static Object Property Access (Compiler) 1397.3±3.63ns 1439.2±3.94ns +3.00%
Static Object Property Access (Execution) 5.2±0.01µs 5.2±0.02µs 0.00%
Static Object Property Access (Parser) 10.4±0.02µs 10.8±0.11µs +3.85%
String Object Access (Compiler) 1470.1±19.53ns 1443.6±4.88ns -1.80%
String Object Access (Execution) 5.9±0.02µs 6.0±0.03µs +1.69%
String Object Access (Parser) 14.6±0.01µs 15.5±0.01µs +6.16%
String comparison (Compiler) 1981.9±5.44ns 1993.8±4.43ns +0.60%
String comparison (Execution) 4.5±0.01µs 4.6±0.01µs +2.22%
String comparison (Parser) 11.2±0.01µs 11.8±0.05µs +5.36%
String concatenation (Compiler) 1562.8±3.23ns 1576.1±2.78ns +0.85%
String concatenation (Execution) 4.3±0.01µs 4.4±0.01µs +2.33%
String concatenation (Parser) 7.9±0.01µs 8.3±0.01µs +5.06%
String copy (Compiler) 1254.9±3.43ns 1261.6±2.89ns +0.53%
String copy (Execution) 4.1±0.01µs 4.1±0.02µs 0.00%
String copy (Parser) 5.9±0.01µs 6.2±0.01µs +5.08%
Symbols (Compiler) 926.9±1.25ns 934.4±6.62ns +0.81%
Symbols (Execution) 4.1±0.02µs 4.1±0.02µs 0.00%
Symbols (Parser) 4.7±0.01µs 4.9±0.02µs +4.26%

Razican
Razican approved these changes May 4, 2022
View reviewed changes
Copy link
Member

@Razican Razican left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added some comments, but it looks great!

boa_engine/src/syntax/parser/mod.rs Show resolved Hide resolved
boa_engine/src/syntax/parser/expression/mod.rs Show resolved Hide resolved
boa_engine/src/syntax/parser/expression/identifiers.rs Show resolved Hide resolved
HalidOdat
HalidOdat approved these changes May 9, 2022
View reviewed changes
Copy link
Member

@HalidOdat HalidOdat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me! :)

@HalidOdat
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request May 9, 2022
@raskad
Refactor IdentifierReference parsing (#2055)
e42e2f6 
This Pull Request changes the following:

- Crate dedicated `IdentifierReference` parser and refactor `PrimaryExpression` and `PropertyDefinition` parsers to use it.
- Move `BindingIdentifier` and `LabelIdentifier` parsers from statement parser module to expression parser module to conform with the spec.
- Add and early error case while converting an `ObjectLiteral` to an `ObjectAssignmentPattern`
@bors
Copy link

bors bot commented May 9, 2022

Pull request successfully merged into main.

Build succeeded:

@bors bors bot changed the title Refactor IdentifierReference parsing [Merged by Bors] - Refactor IdentifierReference parsing May 9, 2022
@bors bors bot closed this May 9, 2022
@bors bors bot deleted the refactor-identifier-reference branch May 9, 2022 15:36
Razican pushed a commit that referenced this pull request Jun 8, 2022
@raskad @Razican
Refactor IdentifierReference parsing (#2055)
7abd41b 
This Pull Request changes the following:

- Crate dedicated `IdentifierReference` parser and refactor `PrimaryExpression` and `PropertyDefinition` parsers to use it.
- Move `BindingIdentifier` and `LabelIdentifier` parsers from statement parser module to expression parser module to conform with the spec.
- Add and early error case while converting an `ObjectLiteral` to an `ObjectAssignmentPattern`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Razican Razican Razican approved these changes

@HalidOdat HalidOdat HalidOdat approved these changes

@jasonwilliams jasonwilliams Awaiting requested review from jasonwilliams

@RageKnify RageKnify Awaiting requested review from RageKnify

@jedel1043 jedel1043 Awaiting requested review from jedel1043

Assignees
No one assigned
Labels
enhancement New feature or request parser Issues surrounding the parser
Projects
None yet
Milestone
v0.15.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@raskad @HalidOdat @Razican