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

Fix value property assignment order #2903

Closed
wants to merge 1 commit into from

Conversation

jedel1043
Copy link
Member

@jedel1043 jedel1043 commented May 5, 2023

This PR reorders assignments evaluation to conform to the spec... however, it seems to be uncertain if the new assignment semantics are intended or a bug on the spec (see tc39/ecma262#2659 and tc39/test262#3407).

If it's indeed a bug on the spec, this should be closed in favour of the current semantics. Otherwise, this can be merged.

Per the previous explanation, this is blocked on tc39/ecma262#2659

@jedel1043 jedel1043 added blocked Waiting for another code change execution Issues or PRs related to code execution vm Issues and PRs related to the Boa Virtual Machine. labels May 5, 2023
@jedel1043 jedel1043 requested a review from a team May 5, 2023 16:49
@github-actions
Copy link

github-actions bot commented May 5, 2023

Test262 conformance changes

Test result main count PR count difference
Total 94,601 94,601 0
Passed 73,298 73,230 -68
Ignored 17,540 17,540 0
Failed 3,763 3,831 +68
Panics 0 0 0
Conformance 77.48% 77.41% -0.07%
Fixed tests (2):
test/language/expressions/assignment/S11.13.1_A7_T3.js [strict mode] (previously Failed)
test/language/expressions/assignment/S11.13.1_A7_T3.js (previously Failed)
Broken tests (70):
test/language/expressions/assignment/target-member-computed-reference-undefined.js [strict mode] (previously Passed)
test/language/expressions/assignment/target-member-computed-reference-undefined.js (previously Passed)
test/language/expressions/assignment/target-member-computed-reference-null.js [strict mode] (previously Passed)
test/language/expressions/assignment/target-member-computed-reference-null.js (previously Passed)
test/language/expressions/prefix-increment/S11.4.4_A6_T2.js [strict mode] (previously Passed)
test/language/expressions/prefix-increment/S11.4.4_A6_T2.js (previously Passed)
test/language/expressions/prefix-increment/S11.4.4_A6_T1.js [strict mode] (previously Passed)
test/language/expressions/prefix-increment/S11.4.4_A6_T1.js (previously Passed)
test/language/expressions/postfix-decrement/S11.3.2_A6_T2.js [strict mode] (previously Passed)
test/language/expressions/postfix-decrement/S11.3.2_A6_T2.js (previously Passed)
test/language/expressions/postfix-decrement/S11.3.2_A6_T1.js [strict mode] (previously Passed)
test/language/expressions/postfix-decrement/S11.3.2_A6_T1.js (previously Passed)
test/language/expressions/postfix-increment/S11.3.1_A6_T2.js [strict mode] (previously Passed)
test/language/expressions/postfix-increment/S11.3.1_A6_T2.js (previously Passed)
test/language/expressions/postfix-increment/S11.3.1_A6_T1.js [strict mode] (previously Passed)
test/language/expressions/postfix-increment/S11.3.1_A6_T1.js (previously Passed)
test/language/expressions/prefix-decrement/S11.4.5_A6_T1.js [strict mode] (previously Passed)
test/language/expressions/prefix-decrement/S11.4.5_A6_T1.js (previously Passed)
test/language/expressions/prefix-decrement/S11.4.5_A6_T2.js [strict mode] (previously Passed)
test/language/expressions/prefix-decrement/S11.4.5_A6_T2.js (previously Passed)
test/language/expressions/logical-assignment/lgcl-nullish-assignment-operator-lhs-before-rhs.js [strict mode] (previously Passed)
test/language/expressions/logical-assignment/lgcl-nullish-assignment-operator-lhs-before-rhs.js (previously Passed)
test/language/expressions/logical-assignment/lgcl-or-assignment-operator-lhs-before-rhs.js [strict mode] (previously Passed)
test/language/expressions/logical-assignment/lgcl-or-assignment-operator-lhs-before-rhs.js (previously Passed)
test/language/expressions/logical-assignment/lgcl-and-assignment-operator-lhs-before-rhs.js [strict mode] (previously Passed)
test/language/expressions/logical-assignment/lgcl-and-assignment-operator-lhs-before-rhs.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.9_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.9_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.5_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.5_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.1_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.1_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.4_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.4_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.5_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.5_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.7_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.7_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.10_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.10_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.2_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.2_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.8_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.8_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.10_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.10_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.9_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.9_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.2_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.2_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.3_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.3_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.8_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.8_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.6_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.6_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.6_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.6_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.11_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.11_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.7_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.7_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.4_T1.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.4_T1.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.1_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.1_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.3_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.3_T2.js (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.11_T2.js [strict mode] (previously Passed)
test/language/expressions/compound-assignment/S11.13.2_A7.11_T2.js (previously Passed)

@codecov
Copy link

codecov bot commented May 5, 2023

Codecov Report

Merging #2903 (c3e7b17) into main (0aaf462) will increase coverage by 0.53%.
The diff coverage is 34.24%.

@@            Coverage Diff             @@
##             main    #2903      +/-   ##
==========================================
+ Coverage   51.30%   51.83%   +0.53%     
==========================================
  Files         427      431       +4     
  Lines       42612    43226     +614     
==========================================
+ Hits        21860    22407     +547     
- Misses      20752    20819      +67     
Impacted Files Coverage Δ
boa_engine/src/bytecompiler/class.rs 15.05% <0.00%> (-0.43%) ⬇️
boa_engine/src/bytecompiler/expression/assign.rs 39.28% <0.00%> (-1.16%) ⬇️
boa_engine/src/bytecompiler/expression/mod.rs 60.74% <0.00%> (-0.29%) ⬇️
boa_engine/src/property/mod.rs 82.72% <ø> (-1.11%) ⬇️
boa_engine/src/vm/code_block.rs 56.84% <ø> (+1.21%) ⬆️
boa_engine/src/vm/flowgraph/mod.rs 0.00% <ø> (ø)
boa_engine/src/vm/opcode/copy/mod.rs 57.14% <0.00%> (-2.86%) ⬇️
boa_engine/src/vm/opcode/define/class/getter.rs 0.00% <0.00%> (ø)
boa_engine/src/vm/opcode/define/class/method.rs 0.00% <0.00%> (ø)
boa_engine/src/vm/opcode/define/class/setter.rs 0.00% <0.00%> (ø)
... and 12 more

... and 47 files with indirect coverage changes

@jedel1043
Copy link
Member Author

jedel1043 commented Apr 15, 2024

Closing since it was decided that the old behaviour was the correct one.
tc39/ecma262#3295

@jedel1043 jedel1043 closed this Apr 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked Waiting for another code change execution Issues or PRs related to code execution vm Issues and PRs related to the Boa Virtual Machine.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant