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.

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

Remove incorrect cursor.set_goal() call in AssignmentExpression parsing #2177

Closed
wants to merge 3 commits into from

Conversation

jbarthelmess
Copy link

This Pull Request fixes/closes #2148 .

It changes the following:

  • Adds a test to the lexer showing that the lexar produces correct tokens
  • Removes cursor.set_goal() call from the AssignmentExpression parse expression

@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #2177 (bbe1f50) into main (556f3ce) will decrease coverage by 0.00%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #2177      +/-   ##
==========================================
- Coverage   41.91%   41.90%   -0.01%     
==========================================
  Files         231      231              
  Lines       21502    21501       -1     
==========================================
- Hits         9012     9010       -2     
- Misses      12490    12491       +1     
Impacted Files Coverage Δ
...ine/src/syntax/parser/expression/assignment/mod.rs 53.06% <ø> (-0.48%) ⬇️
...engine/src/syntax/parser/expression/primary/mod.rs 36.29% <0.00%> (-0.39%) ⬇️

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 556f3ce...bbe1f50. Read the comment docs.

@Razican Razican changed the title remove cursor.set_goal Remove incorrect cursor.set_goal() call in AssignmentExpression parsing Jul 14, 2022
@Razican
Copy link
Member

Razican commented Jul 14, 2022

It seems that this makes a ton of existing tests to fail (check Test262 output). I guess the goal needs to be set at some point, but inside nested expression parsing

@Razican
Copy link
Member

Razican commented Jul 15, 2022

Test262 conformance changes

VM implementation

Test result main count PR count difference
Total 90,847 90,847 0
Passed 59,784 59,680 -104
Ignored 14,012 14,012 0
Failed 17,051 17,155 +104
Panics 0 0 0
Conformance 65.81% 65.69% -0.11%
Broken tests (104):
test/built-ins/TypedArray/prototype/sort/stability.js [strict mode] (previously Passed)
test/built-ins/TypedArray/prototype/sort/stability.js (previously Passed)
test/built-ins/Array/prototype/map/15.4.4.19-8-c-iii-1.js [strict mode] (previously Passed)
test/built-ins/Array/prototype/map/15.4.4.19-8-c-iii-1.js (previously Passed)
test/language/expressions/class/elements/after-same-line-static-async-method-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/after-same-line-static-async-method-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/multiple-definitions-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/multiple-definitions-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/new-sc-line-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/new-sc-line-gen-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/after-same-line-static-method-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/after-same-line-static-method-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/same-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/same-line-method-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/same-line-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/same-line-gen-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/after-same-line-static-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/after-same-line-static-gen-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/new-sc-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/new-sc-line-method-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/new-no-sc-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/new-no-sc-line-method-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/wrapped-in-sc-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/wrapped-in-sc-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/after-same-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/after-same-line-method-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/after-same-line-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/after-same-line-gen-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/multiple-stacked-definitions-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/multiple-stacked-definitions-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/regular-definitions-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/regular-definitions-static-private-methods.js (previously Passed)
test/language/expressions/class/elements/same-line-async-method-static-private-methods.js [strict mode] (previously Passed)
test/language/expressions/class/elements/same-line-async-method-static-private-methods.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A2.1_T2.2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A2.1_T2.2.js (previously Passed)
test/language/expressions/compound-assignment/div-whitespace.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/div-whitespace.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A2.1_T3.2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A2.1_T3.2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A2.1_T1.2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A2.1_T1.2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A3.2_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A3.2_T2.js (previously Passed)
test/language/expressions/modulus/S11.5.3_A4_T7.js [strict mode] (previously Passed)
test/language/expressions/modulus/S11.5.3_A4_T7.js (previously Passed)
test/language/expressions/division/S11.5.2_A4_T4.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A4_T4.js (previously Passed)
test/language/expressions/division/S11.5.2_A4_T5.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A4_T5.js (previously Passed)
test/language/expressions/division/S11.5.2_A2.3_T1.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A2.3_T1.js (previously Passed)
test/language/expressions/division/S11.5.2_A3_T2.4.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A3_T2.4.js (previously Passed)
test/language/expressions/division/S11.5.2_A4_T1.1.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A4_T1.1.js (previously Passed)
test/language/expressions/division/S11.5.2_A3_T1.4.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A3_T1.4.js (previously Passed)
test/language/expressions/division/S11.5.2_A2.4_T1.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A2.4_T1.js (previously Passed)
test/language/expressions/division/S11.5.2_A2.1_T1.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A2.1_T1.js (previously Passed)
test/language/expressions/division/S11.5.2_A2.4_T3.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A2.4_T3.js (previously Passed)
test/language/expressions/division/S11.5.2_A3_T2.6.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A3_T2.6.js (previously Passed)
test/language/expressions/division/S11.5.2_A3_T2.8.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A3_T2.8.js (previously Passed)
test/language/expressions/division/S11.5.2_A2.1_T2.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A2.1_T2.js (previously Passed)
test/language/expressions/division/S11.5.2_A4_T1.2.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A4_T1.2.js (previously Passed)
test/language/expressions/division/S11.5.2_A4_T3.js [strict mode] (previously Passed)
test/language/expressions/division/S11.5.2_A4_T3.js (previously Passed)
test/language/statements/class/elements/after-same-line-static-async-method-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/after-same-line-static-async-method-static-private-methods.js (previously Passed)
test/language/statements/class/elements/multiple-definitions-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/multiple-definitions-static-private-methods.js (previously Passed)
test/language/statements/class/elements/new-sc-line-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/new-sc-line-gen-static-private-methods.js (previously Passed)
test/language/statements/class/elements/after-same-line-static-method-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/after-same-line-static-method-static-private-methods.js (previously Passed)
test/language/statements/class/elements/same-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/same-line-method-static-private-methods.js (previously Passed)
test/language/statements/class/elements/same-line-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/same-line-gen-static-private-methods.js (previously Passed)
test/language/statements/class/elements/after-same-line-static-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/after-same-line-static-gen-static-private-methods.js (previously Passed)
test/language/statements/class/elements/new-sc-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/new-sc-line-method-static-private-methods.js (previously Passed)
test/language/statements/class/elements/new-no-sc-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/new-no-sc-line-method-static-private-methods.js (previously Passed)
test/language/statements/class/elements/wrapped-in-sc-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/wrapped-in-sc-static-private-methods.js (previously Passed)
test/language/statements/class/elements/after-same-line-method-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/after-same-line-method-static-private-methods.js (previously Passed)
test/language/statements/class/elements/after-same-line-gen-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/after-same-line-gen-static-private-methods.js (previously Passed)
test/language/statements/class/elements/multiple-stacked-definitions-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/multiple-stacked-definitions-static-private-methods.js (previously Passed)
test/language/statements/class/elements/regular-definitions-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/regular-definitions-static-private-methods.js (previously Passed)
test/language/statements/class/elements/same-line-async-method-static-private-methods.js [strict mode] (previously Passed)
test/language/statements/class/elements/same-line-async-method-static-private-methods.js (previously Passed)

@Razican
Copy link
Member

Razican commented Sep 7, 2022

Closing in favour of #2268

@Razican Razican closed this Sep 7, 2022
@Razican Razican added bug Something isn't working parser Issues surrounding the parser labels Sep 7, 2022
bors bot pushed a commit that referenced this pull request Sep 16, 2022
This Pull Request fixes/closes #2148.

It changes the following:

- When we start an assignment expression, the `/` token must be a regular expression literal. The division can only occur between expressions after the assignment operator.
- Added tests for the new behaviour, taken from #2177

This overrides #2177
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working parser Issues surrounding the parser
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The expression /=/ does not get parsed as a regexp
2 participants