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

Make update operations reuse the last found binding locator #2876

Merged
merged 9 commits into from
May 3, 2023
Merged

Make update operations reuse the last found binding locator #2876

merged 9 commits into from
May 3, 2023

Conversation

jedel1043
Copy link
Member

@jedel1043 jedel1043 commented Apr 27, 2023 •
edited
Loading

This Pull Request fixes update operations by reusing the lvalue accessed by the update.

It changes the following:

  • Creates ops GetNameAndLocator and SetNameByLocator.
  • Use new ops on update operations (++, +=, &&=, etc.) only when the binding is not lexical.

@jedel1043 jedel1043 added execution Issues or PRs related to code execution vm Issues and PRs related to the Boa Virtual Machine. labels Apr 27, 2023
@jedel1043 jedel1043 added this to the v0.17.0 milestone Apr 27, 2023
@github-actions
Copy link

github-actions bot commented Apr 27, 2023 •
edited
Loading

Test262 conformance changes

Test result main count PR count difference
Total 94,601 94,601 0
Passed 73,228 73,293 +65
Ignored 17,540 17,540 0
Failed 3,833 3,768 -65
Panics 0 0 0
Conformance 77.41% 77.48% +0.07%
Fixed tests (65): 
test/language/statements/switch/scope-lex-open-case.js [strict mode] (previously Failed)
test/language/statements/switch/scope-lex-open-case.js (previously Failed)
test/language/statements/with/unscopables-inc-dec.js (previously Failed)
test/language/expressions/assignment/S11.13.1_A5_T1.js (previously Failed)
test/language/expressions/assignment/S11.13.1_A6_T1.js (previously Failed)
test/language/expressions/assignment/S11.13.1_A6_T2.js (previously Failed)
test/language/expressions/assignment/S11.13.1_A6_T3.js (previously Failed)
test/language/expressions/assignment/S11.13.1_A5_T2.js (previously Failed)
test/language/expressions/assignment/S11.13.1_A5_T3.js (previously Failed)
test/language/expressions/prefix-increment/S11.4.4_A5_T1.js (previously Failed)
test/language/expressions/prefix-increment/S11.4.4_A5_T3.js (previously Failed)
test/language/expressions/prefix-increment/S11.4.4_A5_T2.js (previously Failed)
test/language/expressions/postfix-decrement/S11.3.2_A5_T1.js (previously Failed)
test/language/expressions/postfix-decrement/S11.3.2_A5_T2.js (previously Failed)
test/language/expressions/postfix-decrement/S11.3.2_A5_T3.js (previously Failed)
test/language/expressions/postfix-increment/S11.3.1_A5_T1.js (previously Failed)
test/language/expressions/postfix-increment/S11.3.1_A5_T2.js (previously Failed)
test/language/expressions/postfix-increment/S11.3.1_A5_T3.js (previously Failed)
test/language/expressions/prefix-decrement/S11.4.5_A5_T3.js (previously Failed)
test/language/expressions/prefix-decrement/S11.4.5_A5_T2.js (previously Failed)
test/language/expressions/prefix-decrement/S11.4.5_A5_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.9_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.8_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.7_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.1_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.4_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.10_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.11_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.9_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.6_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.9_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.3_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.4_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.7_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.1_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.11_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.7_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.2_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.5_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.9_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.11_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.3_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.1_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.3_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.10_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.8_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.1_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.2_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.11_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.4_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.4_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.5_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.2_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.3_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.5_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.8_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.10_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.5_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A6.6_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.6_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.10_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.6_T2.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.8_T1.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.2_T3.js (previously Failed)
test/language/expressions/compound-assignment/S11.13.2_A5.7_T1.js (previously Failed)

@codecov
Copy link

codecov bot commented Apr 27, 2023 •
edited
Loading

Codecov Report

Merging #2876 (e671ada) into main (8ef440a) will increase coverage by 0.04%.
The diff coverage is 87.87%.

@@            Coverage Diff             @@
##             main    #2876      +/-   ##
==========================================
+ Coverage   51.25%   51.30%   +0.04%     
==========================================
  Files         427      427              
  Lines       42546    42612      +66     
==========================================
+ Hits        21805    21860      +55     
- Misses      20741    20752      +11
Impacted Files Coverage Δ
boa_engine/src/builtins/json/mod.rs 85.22% <ø> (-0.05%) ⬇️
boa_engine/src/bytecompiler/class.rs 15.48% <0.00%> (-0.22%) ⬇️
boa_engine/src/bytecompiler/function.rs 81.44% <ø> (-0.19%) ⬇️
boa_engine/src/bytecompiler/statement/block.rs 100.00% <ø> (ø)
boa_engine/src/bytecompiler/statement/try.rs 100.00% <ø> (ø)
boa_engine/src/context/mod.rs 47.88% <ø> (-0.25%) ⬇️
boa_engine/src/vm/code_block.rs 55.63% <ø> (ø)
boa_engine/src/vm/flowgraph/mod.rs 0.00% <ø> (ø)
boa_engine/src/vm/mod.rs 53.89% <ø> (ø)
boa_engine/src/vm/opcode/mod.rs 45.45% <ø> (ø)
... and 9 more

... and 3 files with indirect coverage changes

@jedel1043 jedel1043 force-pushed the assignment-rebinding branch from de5bde3 to 6c14317 Compare May 2, 2023 03:01
@jedel1043 jedel1043 requested a review from Razican May 2, 2023 21:10
raskad
raskad approved these changes May 3, 2023
View reviewed changes
Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

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

Really nice implementation.

@jedel1043 jedel1043 force-pushed the assignment-rebinding branch from 736e3a0 to e671ada Compare May 3, 2023 06:52
Razican
Razican approved these changes May 3, 2023
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.

Looks good to me! thanks!

@Razican Razican added this pull request to the merge queue May 3, 2023
Merged via the queue into main with commit c341772 May 3, 2023
@bors bors bot deleted the assignment-rebinding branch May 3, 2023 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@raskad raskad raskad approved these changes

@Razican Razican Razican approved these changes

@jasonwilliams jasonwilliams Awaiting requested review from jasonwilliams

@HalidOdat HalidOdat Awaiting requested review from HalidOdat

@RageKnify RageKnify Awaiting requested review from RageKnify

@nekevss nekevss Awaiting requested review from nekevss

Assignees
No one assigned
Labels
execution Issues or PRs related to code execution vm Issues and PRs related to the Boa Virtual Machine.
Projects
None yet
Milestone
v0.17.0
Development

Successfully merging this pull request may close these issues.
3 participants
@jedel1043 @Razican @raskad