diff --git a/numbat/src/bytecode_interpreter.rs b/numbat/src/bytecode_interpreter.rs index b2f7f0d73..15dce4ae2 100644 --- a/numbat/src/bytecode_interpreter.rs +++ b/numbat/src/bytecode_interpreter.rs @@ -109,6 +109,32 @@ impl BytecodeInterpreter { self.compile_expression(lhs)?; self.vm.add_op(Op::LogicalNeg); } + Expression::BinaryOperator( + _span, + op @ (BinaryOperator::LogicalOr | BinaryOperator::LogicalAnd), + lhs, + rhs, + _type, + ) => { + self.compile_expression(lhs)?; + + let jump_offset = self.vm.current_offset() + 1; // +1 for the opcode + self.vm.add_op1( + match op { + BinaryOperator::LogicalOr => Op::JumpIfTrue, + BinaryOperator::LogicalAnd => Op::JumpIfFalse, + _ => unreachable!(), + }, + 0xffff, + ); + self.vm.add_op(Op::Pop); + + self.compile_expression(rhs)?; + + let else_block_offset = self.vm.current_offset(); + self.vm + .patch_u16_value_at(jump_offset, else_block_offset - (jump_offset + 2)); + } Expression::BinaryOperator(_span, operator, lhs, rhs, _type) => { self.compile_expression(lhs)?; self.compile_expression(rhs)?; @@ -126,8 +152,7 @@ impl BytecodeInterpreter { BinaryOperator::GreaterOrEqual => Op::GreatorOrEqual, BinaryOperator::Equal => Op::Equal, BinaryOperator::NotEqual => Op::NotEqual, - BinaryOperator::LogicalAnd => Op::LogicalAnd, - BinaryOperator::LogicalOr => Op::LogicalOr, + BinaryOperator::LogicalAnd | BinaryOperator::LogicalOr => unreachable!(), }; self.vm.add_op(op); } @@ -243,7 +268,7 @@ impl BytecodeInterpreter { self.compile_expression(condition)?; let if_jump_offset = self.vm.current_offset() + 1; // +1 for the opcode - self.vm.add_op1(Op::JumpIfFalse, 0xffff); + self.vm.add_op1(Op::PopJumpIfFalse, 0xffff); self.compile_expression(then_expr)?; diff --git a/numbat/src/value.rs b/numbat/src/value.rs index 6d3b32d7e..04dfbccf1 100644 --- a/numbat/src/value.rs +++ b/numbat/src/value.rs @@ -52,9 +52,9 @@ impl Value { } #[track_caller] - pub fn unsafe_as_bool(self) -> bool { + pub fn unsafe_as_bool(&self) -> bool { if let Value::Boolean(b) = self { - b + *b } else { panic!("Expected value to be a bool"); } diff --git a/numbat/src/vm.rs b/numbat/src/vm.rs index dd3ec68cd..99b448e0a 100644 --- a/numbat/src/vm.rs +++ b/numbat/src/vm.rs @@ -75,10 +75,11 @@ pub enum Op { GreatorOrEqual, Equal, NotEqual, - LogicalAnd, - LogicalOr, LogicalNeg, + /// Pop the top value off the stack. + Pop, + /// Similar to Add, but has DateTime on the LHS and a quantity on the RHS AddToDateTime, /// Similar to Sub, but has DateTime on the LHS and a quantity on the RHS @@ -88,7 +89,13 @@ pub enum Op { /// Move IP forward by the given offset argument if the popped-of value on /// top of the stack is false. + PopJumpIfFalse, + /// Move IP forward by the given offset argument if the peeked value on + /// top of the stack is false. JumpIfFalse, + /// Move IP forward by the given offset argument if the peeked value on + /// top of the stack is true. + JumpIfTrue, /// Unconditionally move IP forward by the given offset argument Jump, @@ -132,7 +139,9 @@ impl Op { | Op::GetUpvalue | Op::PrintString | Op::JoinString + | Op::PopJumpIfFalse | Op::JumpIfFalse + | Op::JumpIfTrue | Op::Jump | Op::CallCallable | Op::AccessStructField @@ -154,11 +163,10 @@ impl Op { | Op::GreatorOrEqual | Op::Equal | Op::NotEqual - | Op::LogicalAnd - | Op::LogicalOr | Op::LogicalNeg | Op::Return - | Op::GetLastResult => 0, + | Op::GetLastResult + | Op::Pop => 0, } } @@ -187,10 +195,10 @@ impl Op { Op::GreatorOrEqual => "GreatorOrEqual", Op::Equal => "Equal", Op::NotEqual => "NotEqual", - Op::LogicalAnd => "LogicalAnd", - Op::LogicalOr => "LogicalOr", Op::LogicalNeg => "LogicalNeg", + Op::PopJumpIfFalse => "PopJumpIfFalse", Op::JumpIfFalse => "JumpIfFalse", + Op::JumpIfTrue => "JumpIfTrue", Op::Jump => "Jump", Op::Call => "Call", Op::FFICallFunction => "FFICallFunction", @@ -202,6 +210,7 @@ impl Op { Op::BuildStructInstance => "BuildStructInstance", Op::AccessStructField => "AccessStructField", Op::BuildList => "BuildList", + Op::Pop => "Pop", } } } @@ -584,6 +593,14 @@ impl Vm { self.pop().unsafe_as_bool() } + #[track_caller] + fn peek_bool(&mut self) -> bool { + self.stack + .last() + .expect("stack should not be empty") + .unsafe_as_bool() + } + #[track_caller] fn pop_datetime(&mut self) -> jiff::Zoned { match self.pop() { @@ -634,6 +651,9 @@ impl Vm { self.stack .push(self.constants[constant_idx as usize].to_value()); } + Op::Pop => { + self.pop(); + } Op::ApplyPrefix => { let quantity = self.pop_quantity(); let prefix_idx = self.read_u16(); @@ -788,17 +808,6 @@ impl Vm { }; self.push(Value::Boolean(result)); } - op @ (Op::LogicalAnd | Op::LogicalOr) => { - let rhs = self.pop_bool(); - let lhs = self.pop_bool(); - - let result = match op { - Op::LogicalAnd => lhs && rhs, - Op::LogicalOr => lhs || rhs, - _ => unreachable!(), - }; - self.push_bool(result); - } Op::LogicalNeg => { let rhs = self.pop_bool(); self.push_bool(!rhs); @@ -824,12 +833,24 @@ impl Vm { self.push_quantity(Quantity::from_scalar(math::factorial(lhs, order))); } - Op::JumpIfFalse => { + Op::PopJumpIfFalse => { let offset = self.read_u16() as usize; if !self.pop_bool() { self.current_frame_mut().ip += offset; } } + Op::JumpIfFalse => { + let offset = self.read_u16() as usize; + if !self.peek_bool() { + self.current_frame_mut().ip += offset; + } + } + Op::JumpIfTrue => { + let offset = self.read_u16() as usize; + if self.peek_bool() { + self.current_frame_mut().ip += offset; + } + } Op::Jump => { let offset = self.read_u16() as usize; self.current_frame_mut().ip += offset; diff --git a/numbat/tests/interpreter.rs b/numbat/tests/interpreter.rs index 7cb5c0273..00142a3b7 100644 --- a/numbat/tests/interpreter.rs +++ b/numbat/tests/interpreter.rs @@ -650,6 +650,12 @@ fn test_logical() { expect_output("true && false", "false"); expect_output("true && true", "true"); + // short-circuiting + expect_output("false && head([])", "false"); + expect_output("true || head([])", "true"); + insta::assert_snapshot!(fail("true && head([])"), @"Empty list"); + insta::assert_snapshot!(fail("false || head([])"), @"Empty list"); + // priority expect_output("false || true && false", "false"); expect_output("false || true && !false", "true");