From e04ec1a2b0934ca710d63a11206c5fd10b1b1bf5 Mon Sep 17 00:00:00 2001 From: dev-willbird1936 Date: Wed, 22 Jul 2026 16:23:48 +0100 Subject: [PATCH 1/3] fix(engine): preserve binary operand values --- .../src/bytecompiler/expression/binary.rs | 6 ++-- core/engine/src/bytecompiler/mod.rs | 36 ++++++++++++++++++- core/engine/src/vm/tests.rs | 13 +++++++ 3 files changed, 51 insertions(+), 4 deletions(-) diff --git a/core/engine/src/bytecompiler/expression/binary.rs b/core/engine/src/bytecompiler/expression/binary.rs index 66a80a2d415..d592591ea4e 100644 --- a/core/engine/src/bytecompiler/expression/binary.rs +++ b/core/engine/src/bytecompiler/expression/binary.rs @@ -14,17 +14,17 @@ impl ByteCompiler<'_> { pub(crate) fn compile_binary(&mut self, binary: &Binary, dst: &Register) { match binary.op() { BinaryOp::Arithmetic(op) => { - self.compile_expr_operand(binary.lhs(), |self_, lhs| { + self.compile_expr_operand_snapshot(binary.lhs(), |self_, lhs| { self_.compile_binary_arithmetic(op, binary.rhs(), dst, lhs); }); } BinaryOp::Bitwise(op) => { - self.compile_expr_operand(binary.lhs(), |self_, lhs| { + self.compile_expr_operand_snapshot(binary.lhs(), |self_, lhs| { self_.compile_binary_bitwise(op, binary.rhs(), dst, lhs); }); } BinaryOp::Relational(op) => { - self.compile_expr_operand(binary.lhs(), |self_, lhs| { + self.compile_expr_operand_snapshot(binary.lhs(), |self_, lhs| { self_.compile_binary_relational(op, binary.rhs(), dst, lhs); }); } diff --git a/core/engine/src/bytecompiler/mod.rs b/core/engine/src/bytecompiler/mod.rs index fc5d3b1458e..f65e26e95c1 100644 --- a/core/engine/src/bytecompiler/mod.rs +++ b/core/engine/src/bytecompiler/mod.rs @@ -1351,7 +1351,7 @@ impl<'ctx> ByteCompiler<'ctx> { }); } None => { - self.compile_expr_operand(binary.lhs(), |compiler, lhs| { + self.compile_expr_operand_snapshot(binary.lhs(), |compiler, lhs| { compiler.compile_expr_operand(binary.rhs(), |compiler, rhs| { label_index = compiler.next_opcode_location(); emit_fn(&mut compiler.bytecode, Self::DUMMY_ADDRESS, lhs, rhs); @@ -1825,6 +1825,40 @@ impl<'ctx> ByteCompiler<'ctx> { self.register_allocator.dealloc(reg); } + /// Compile an expression operand while preserving the value of mutable local identifiers. + /// + /// Mutable locals live in persistent registers. If compiling a later operand can mutate the + /// local, using that persistent register directly would observe the later value instead of the + /// value produced by the earlier expression. Snapshot mutable locals into a temporary register + /// before invoking the callback; immutable locals and cached constants remain on the fast path. + pub(crate) fn compile_expr_operand_snapshot( + &mut self, + expr: &Expression, + inner_fn: impl FnOnce(&mut Self, RegisterOperand), + ) { + if let Expression::Identifier(name) = expr { + let name = self.resolve_identifier_expect(*name); + let binding = self.lexical_scope.get_identifier_reference(name.clone()); + let index = self.get_binding(&binding); + if let BindingKind::Local(Some(local_reg)) = &index { + if !matches!(self.lexical_scope.is_binding_mutable(&name), Some(false)) { + let snapshot = self.register_allocator.alloc(); + self.bytecode + .emit_move(snapshot.variable(), (*local_reg).into()); + let op = snapshot.variable(); + inner_fn(self, op); + self.register_allocator.dealloc(snapshot); + return; + } + + inner_fn(self, (*local_reg).into()); + return; + } + } + + self.compile_expr_operand(expr, inner_fn); + } + /// Compile a property access expression, prepending `this` to the property value in the stack. /// /// This compiles the access in a way that the state of the stack after executing the property diff --git a/core/engine/src/vm/tests.rs b/core/engine/src/vm/tests.rs index 0b25366ac41..24a4501badf 100644 --- a/core/engine/src/vm/tests.rs +++ b/core/engine/src/vm/tests.rs @@ -453,6 +453,19 @@ fn super_construction_with_parameter_expression() { ]); } +#[test] +fn binary_operands_preserve_left_to_right_evaluation() { + run_test_actions([ + TestAction::assert_eq("(function() { let x = 1; return x + (x = 2); })()", 3), + TestAction::assert_eq("(function() { let x = 1; return x | (x = 2); })()", 3), + TestAction::assert_eq("(function() { let x = 1; return x === (x = 2); })()", false), + TestAction::assert_eq( + "(function() { let x = 1; return x < (x = 2) ? 1 : 0; })()", + 1, + ), + ]); +} + #[test] fn cross_context_function_call() { let context1 = &mut Context::default(); From aecd0d7d7cea2743909100639d82be158a13f22f Mon Sep 17 00:00:00 2001 From: Codex Date: Sun, 26 Jul 2026 20:07:09 +0900 Subject: [PATCH 2/3] Update bytecode snapshots for operand preservation --- ...de__compile_bytecode@loop-hoisting.js.snap | 47 +++++----- ...code__compile_bytecode@try-finally.js.snap | 94 ++++++++++--------- 2 files changed, 72 insertions(+), 69 deletions(-) diff --git a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap index c4f44884d67..09e5c1945d9 100644 --- a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap +++ b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@loop-hoisting.js.snap @@ -19,26 +19,27 @@ Location Handler Opcode Operands 000049 Jump address:000058 00004e IncrementLoopIteration 00004f Inc src:r05, dst:r05 - 000058 JumpIfNotLessThan lhs:r05, rhs:r04, address:00006a - 000065 Jump address:00004e - 00006a StoreInt8 value:100, dst:r06 - 000070 PutLexicalValue src:r06, binding_index:1 - 000079 Move src:r06, dst:r07 - 000082 PushFromRegister src:r00 - 000087 GetNameGlobal dst:r06, binding_index:2, ic_index:0 - 000094 PushFromRegister src:r06 - 000099 Call argument_count:0 - 00009e Pop - 00009f StoreInt8 value:100, dst:r06 - 0000a5 PutLexicalValue src:r06, binding_index:3 - 0000ae PushFromRegister src:r00 - 0000b3 GetNameGlobal dst:r08, binding_index:4, ic_index:1 - 0000c0 PushFromRegister src:r08 - 0000c5 Call argument_count:0 - 0000ca PopIntoRegister dst:r06 - 0000cf SetAccumulator src:r06 - 0000d4 CheckReturn - 0000d5 Return + 000058 Move src:r05, dst:r06 + 000061 JumpIfNotLessThan lhs:r06, rhs:r04, address:000073 + 00006e Jump address:00004e + 000073 StoreInt8 value:100, dst:r06 + 000079 PutLexicalValue src:r06, binding_index:1 + 000082 Move src:r06, dst:r07 + 00008b PushFromRegister src:r00 + 000090 GetNameGlobal dst:r06, binding_index:2, ic_index:0 + 00009d PushFromRegister src:r06 + 0000a2 Call argument_count:0 + 0000a7 Pop + 0000a8 StoreInt8 value:100, dst:r06 + 0000ae PutLexicalValue src:r06, binding_index:3 + 0000b7 PushFromRegister src:r00 + 0000bc GetNameGlobal dst:r08, binding_index:4, ic_index:1 + 0000c9 PushFromRegister src:r08 + 0000ce Call argument_count:0 + 0000d3 PopIntoRegister dst:r06 + 0000d8 SetAccumulator src:r06 + 0000dd CheckReturn + 0000de Return Register Count: 9, Flags: CodeBlockFlags(HAS_PROTOTYPE_PROPERTY) Constants: @@ -58,6 +59,6 @@ Bindings: Handlers: Source Map: 0000: 17..79: (2, 26) - 0001: 79..153: (5, 24) - 0002: 153..197: (14, 4) - 0003: 197..207: (23, 4) + 0001: 79..162: (5, 24) + 0002: 162..206: (14, 4) + 0003: 206..216: (23, 4) diff --git a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap index 2972605ee3b..a70bb315565 100644 --- a/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap +++ b/tests/insta-bytecode/src/snapshots/insta_bytecode__compile_bytecode@try-finally.js.snap @@ -12,49 +12,51 @@ Location Handler Opcode Operands 000019 Jump address:000028 00001e IncrementLoopIteration 00001f Inc src:r02, dst:r02 - 000028 JumpIfNotLessThan lhs:r02, rhs:r03, address:000147 + 000028 JumpIfNotLessThan lhs:r02, rhs:r03, address:000159 000035 StoreTrue dst:r04 00003a StoreZero dst:r05 - 00003f > 0: 0000aa StoreInt8 value:2, dst:r07 - 000045 0: 0000aa StrictEq lhs:r02, rhs:r07, dst:r06 - 000052 0: 0000aa JumpIfFalse value:r06, address:00006f - 00005b 0: 0000aa StoreFalse dst:r04 - 000060 0: 0000aa StoreOne dst:r05 - 000065 0: 0000aa Jump address:0000b9 - 00006a 0: 0000aa Jump address:00006f - 00006f 0: 0000aa StoreInt8 value:4, dst:r07 - 000075 0: 0000aa StrictEq lhs:r02, rhs:r07, dst:r06 - 000082 0: 0000aa JumpIfFalse value:r06, address:0000a0 - 00008b 0: 0000aa StoreFalse dst:r04 - 000090 0: 0000aa StoreInt8 value:2, dst:r05 - 000096 0: 0000aa Jump address:0000b9 - 00009b 0: 0000aa Jump address:0000a0 - 0000a0 0: 0000aa StoreFalse dst:r04 - 0000a5 < 0: 0000aa Jump address:0000b9 - 0000aa > 1: 0000b9 Exception dst:r06 - 0000af 1: 0000b9 StoreTrue dst:r04 - 0000b4 < 1: 0000b9 Jump address:0000b9 - 0000b9 SetRegisterFromAccumulator dst:r07 - 0000be GetName dst:r08, binding_index:0 - 0000c7 Move src:r02, dst:r09 - 0000d0 Add lhs:r08, rhs:r09, dst:r08 - 0000dd SetName src:r08, binding_index:0 - 0000e6 SetAccumulator src:r07 - 0000eb JumpIfFalse value:r04, address:0000f9 - 0000f4 Throw src:r06 - 0000f9 JumpTable index:5, jump_table:(00011d, 000113, 000118) - 00010e Jump address:00011d - 000113 Jump address:00001e - 000118 Jump address:000147 - 00011d GetName dst:r04, binding_index:0 - 000126 StoreInt8 value:100, dst:r05 - 00012c Add lhs:r04, rhs:r05, dst:r04 - 000139 SetName src:r04, binding_index:0 - 000142 Jump address:00001e - 000147 GetName dst:r03, binding_index:0 - 000150 SetAccumulator src:r03 - 000155 CheckReturn - 000156 Return + 00003f > 0: 0000bc Move src:r02, dst:r07 + 000048 0: 0000bc StoreInt8 value:2, dst:r08 + 00004e 0: 0000bc StrictEq lhs:r07, rhs:r08, dst:r06 + 00005b 0: 0000bc JumpIfFalse value:r06, address:000078 + 000064 0: 0000bc StoreFalse dst:r04 + 000069 0: 0000bc StoreOne dst:r05 + 00006e 0: 0000bc Jump address:0000cb + 000073 0: 0000bc Jump address:000078 + 000078 0: 0000bc Move src:r02, dst:r07 + 000081 0: 0000bc StoreInt8 value:4, dst:r08 + 000087 0: 0000bc StrictEq lhs:r07, rhs:r08, dst:r06 + 000094 0: 0000bc JumpIfFalse value:r06, address:0000b2 + 00009d 0: 0000bc StoreFalse dst:r04 + 0000a2 0: 0000bc StoreInt8 value:2, dst:r05 + 0000a8 0: 0000bc Jump address:0000cb + 0000ad 0: 0000bc Jump address:0000b2 + 0000b2 0: 0000bc StoreFalse dst:r04 + 0000b7 < 0: 0000bc Jump address:0000cb + 0000bc > 1: 0000cb Exception dst:r06 + 0000c1 1: 0000cb StoreTrue dst:r04 + 0000c6 < 1: 0000cb Jump address:0000cb + 0000cb SetRegisterFromAccumulator dst:r07 + 0000d0 GetName dst:r08, binding_index:0 + 0000d9 Move src:r02, dst:r09 + 0000e2 Add lhs:r08, rhs:r09, dst:r08 + 0000ef SetName src:r08, binding_index:0 + 0000f8 SetAccumulator src:r07 + 0000fd JumpIfFalse value:r04, address:00010b + 000106 Throw src:r06 + 00010b JumpTable index:5, jump_table:(00012f, 000125, 00012a) + 000120 Jump address:00012f + 000125 Jump address:00001e + 00012a Jump address:000159 + 00012f GetName dst:r04, binding_index:0 + 000138 StoreInt8 value:100, dst:r05 + 00013e Add lhs:r04, rhs:r05, dst:r04 + 00014b SetName src:r04, binding_index:0 + 000154 Jump address:00001e + 000159 GetName dst:r03, binding_index:0 + 000162 SetAccumulator src:r03 + 000167 CheckReturn + 000168 Return Register Count: 10, Flags: CodeBlockFlags(HAS_PROTOTYPE_PROPERTY) Constants: @@ -62,9 +64,9 @@ Constants: Bindings: 0000: total, scope: GlobalDeclarative Handlers: - 0000: Range: [00003f, 0000aa): Handler: 0000aa, Environment: 00 - 0001: Range: [0000aa, 0000b9): Handler: 0000b9, Environment: 00 + 0000: Range: [00003f, 0000bc): Handler: 0000bc, Environment: 00 + 0001: Range: [0000bc, 0000cb): Handler: 0000cb, Environment: 00 Source Map: - 0000: 31..190: (7, 24) - 0001: 190..285: (12, 5) - 0002: 285..322: (14, 3) + 0000: 31..208: (7, 24) + 0001: 208..303: (12, 5) + 0002: 303..340: (14, 3) From 5a469a6b4583d8a0479ee95514b2fd364157b292 Mon Sep 17 00:00:00 2001 From: Codex Date: Mon, 27 Jul 2026 08:29:04 +0900 Subject: [PATCH 3/3] test(engine): cover immutable binary operand path --- core/engine/src/vm/tests.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/core/engine/src/vm/tests.rs b/core/engine/src/vm/tests.rs index 24a4501badf..0d18c2caa37 100644 --- a/core/engine/src/vm/tests.rs +++ b/core/engine/src/vm/tests.rs @@ -463,6 +463,10 @@ fn binary_operands_preserve_left_to_right_evaluation() { "(function() { let x = 1; return x < (x = 2) ? 1 : 0; })()", 1, ), + TestAction::assert_eq( + "(function() { const x = 1; let y = 0; return x + (y = 2); })()", + 3, + ), ]); }