Summary: Computed Properties with increment sometimes produces incorrect results Test: let c = 0; let o1 = { [++c]: ++c }; // Should be 1 => 2 and is. { let x = 0; let o2 = { [++x]: ++x }; // Should be 1 => 2, but is not. Instead it is 2 => 2?! } Notes: > <global>#EZzSqs:[0x10f3740a0->0x10f37c520, NoneGlobal, 122]: 122 m_instructions; 976 bytes; 1 parameter(s); 12 callee register(s); 5 variable(s); scope at loc3 > [ 0] enter > [ 1] get_scope loc3 > [ 3] mov loc4, loc3 > [ 6] check_traps > [ 7] mov loc5, Undefined(const0) > [ 10] resolve_scope loc6, loc3, c(@id0), <GlobalLexicalVar>, 0, 0x10f3c40a0 > [ 17] put_to_scope loc6, c(@id0), Int32: 0(const1), 1048578<DoNotThrowIfNotFound|GlobalLexicalVar|Initialization>, <structure>, 242782464 > [ 24] resolve_scope loc6, loc3, o1(@id1), <GlobalLexicalVar>, 0, 0x10f3c40a0 > [ 31] new_object loc7, 0 > [ 35] resolve_scope loc8, loc3, c(@id0), <GlobalLexicalVar>, 0, 0x10f3c40a0 > [ 42] get_from_scope loc9, loc8, c(@id0), 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, 242782464 predicting None > [ 50] inc loc9 > [ 52] put_to_scope loc8, c(@id0), loc9, 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, <structure>, 242782464 > [ 59] resolve_scope loc10, loc3, c(@id0), <GlobalLexicalVar>, 0, 0x10f3c40a0 > [ 66] get_from_scope loc11, loc10, c(@id0), 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, 242782464 predicting None > [ 74] inc loc11 > [ 76] put_to_scope loc10, c(@id0), loc11, 2050<ThrowIfNotFound|GlobalLexicalVar|NotInitialization>, <structure>, 242782464 > [ 83] put_by_val_direct loc7, loc9, loc11 Original > [ 88] put_to_scope loc6, o1(@id1), loc7, 1048578<DoNotThrowIfNotFound|GlobalLexicalVar|Initialization>, <structure>, 242782472 > [ 95] mov loc6, <JSValue()>(const2) > [ 98] mov loc7, <JSValue()>(const2) > [ 101] mov loc6, Int32: 0(const1) > [ 104] new_object loc8, 0 > [ 108] inc loc6 > [ 110] inc loc6 > [ 112] put_by_val_direct loc8, loc6, loc6 Original > [ 117] mov loc7, loc8 > [ 120] end loc5 See Object 1's construction [31-88] compared to Object 2's [104-112].
Created attachment 307408 [details] patch-ish We weren't assigning the ++x to a new register, that was the bug.
Excellent, I'll include this when I also fix the order (computed property should happen first before the value).
Created attachment 391209 [details] Patch
Comment on attachment 391209 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=391209&action=review r=me with one comment. > Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp:742 > if (const auto* identifier = node.name()) { Let's insert `ASSERT(!propertyName);`.
Created attachment 391240 [details] Patch for landing
Comment on attachment 391240 [details] Patch for landing Clearing flags on attachment: 391240 Committed r257034: <https://trac.webkit.org/changeset/257034>
All reviewed patches have been landed. Closing bug.
<rdar://problem/59615634>
It's nice to see some other uses of the new opcode unrelated to class fields :) 👍
(In reply to Caitlin Potter (:caitp) from comment #9) > It's nice to see some other uses of the new opcode unrelated to class fields > :) > 👍 Yeah! :D I realized in the previous bug (bug 207297) that it was exactly the thing we needed.