From 4f00f75e31afc6bb1f163aa67e4b1cb0cadb2e12 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Fri, 26 Aug 2022 20:46:43 +0300 Subject: refactor(tvix/eval): add opcode::JumpOffset type for less ambiguity This adds a transparent wrapper around `usize` used for jump offsets in the opcodes. This is a step towards getting rid of ambiguous plain `usize` usage in the opcode. Change-Id: I21e35e67d94b32d68251908b96c7f62b6f56a8bb Reviewed-on: https://cl.tvl.fyi/c/depot/+/6282 Tested-by: BuildkiteCI Reviewed-by: sterni --- tvix/eval/src/compiler/mod.rs | 21 ++++++++++++--------- tvix/eval/src/opcode.rs | 14 ++++++++++---- tvix/eval/src/vm.rs | 10 +++++----- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/tvix/eval/src/compiler/mod.rs b/tvix/eval/src/compiler/mod.rs index 69e661d46fcc..ac22fc1a6794 100644 --- a/tvix/eval/src/compiler/mod.rs +++ b/tvix/eval/src/compiler/mod.rs @@ -22,7 +22,7 @@ use std::rc::Rc; use crate::chunk::Chunk; use crate::errors::{Error, ErrorKind, EvalResult}; -use crate::opcode::{CodeIdx, OpCode}; +use crate::opcode::{CodeIdx, JumpOffset, OpCode}; use crate::value::{Closure, Lambda, Value}; use crate::warnings::{EvalWarning, WarningKind}; @@ -371,7 +371,7 @@ impl Compiler { // If this value is false, jump over the right-hand side - the // whole expression is false. - let end_idx = self.chunk().push_op(OpCode::OpJumpIfFalse(0)); + let end_idx = self.chunk().push_op(OpCode::OpJumpIfFalse(JumpOffset(0))); // Otherwise, remove the previous value and leave the // right-hand side on the stack. Its result is now the value @@ -395,7 +395,7 @@ impl Compiler { // Opposite of above: If this value is **true**, we can // short-circuit the right-hand side. - let end_idx = self.chunk().push_op(OpCode::OpJumpIfTrue(0)); + let end_idx = self.chunk().push_op(OpCode::OpJumpIfTrue(JumpOffset(0))); self.chunk().push_op(OpCode::OpPop); self.compile(node.rhs().unwrap()); self.patch_jump(end_idx); @@ -414,7 +414,7 @@ impl Compiler { self.chunk().push_op(OpCode::OpInvert); // Exactly as `||` (because `a -> b` = `!a || b`). - let end_idx = self.chunk().push_op(OpCode::OpJumpIfTrue(0)); + let end_idx = self.chunk().push_op(OpCode::OpJumpIfTrue(JumpOffset(0))); self.chunk().push_op(OpCode::OpPop); self.compile(node.rhs().unwrap()); self.patch_jump(end_idx); @@ -617,10 +617,13 @@ impl Compiler { for fragment in path.attrs() { self.compile_attr(fragment); self.chunk().push_op(OpCode::OpAttrsTrySelect); - jumps.push(self.chunk().push_op(OpCode::OpJumpIfNotFound(0))); + jumps.push( + self.chunk() + .push_op(OpCode::OpJumpIfNotFound(JumpOffset(0))), + ); } - let final_jump = self.chunk().push_op(OpCode::OpJump(0)); + let final_jump = self.chunk().push_op(OpCode::OpJump(JumpOffset(0))); for jump in jumps { self.patch_jump(jump); @@ -656,12 +659,12 @@ impl Compiler { fn compile_if_else(&mut self, node: ast::IfElse) { self.compile(node.condition().unwrap()); - let then_idx = self.chunk().push_op(OpCode::OpJumpIfFalse(0)); + let then_idx = self.chunk().push_op(OpCode::OpJumpIfFalse(JumpOffset(0))); self.chunk().push_op(OpCode::OpPop); // discard condition value self.compile(node.body().unwrap()); - let else_idx = self.chunk().push_op(OpCode::OpJump(0)); + let else_idx = self.chunk().push_op(OpCode::OpJump(JumpOffset(0))); self.patch_jump(then_idx); // patch jump *to* else_body self.chunk().push_op(OpCode::OpPop); // discard condition value @@ -866,7 +869,7 @@ impl Compiler { /// not known at the time when the jump operation itself is /// emitted. fn patch_jump(&mut self, idx: CodeIdx) { - let offset = self.chunk().code.len() - 1 - idx.0; + let offset = JumpOffset(self.chunk().code.len() - 1 - idx.0); match &mut self.chunk().code[idx.0] { OpCode::OpJump(n) diff --git a/tvix/eval/src/opcode.rs b/tvix/eval/src/opcode.rs index dadb2ecbf86f..220933f41ea9 100644 --- a/tvix/eval/src/opcode.rs +++ b/tvix/eval/src/opcode.rs @@ -1,12 +1,18 @@ //! This module implements the instruction set running on the abstract //! machine implemented by tvix. +#[repr(transparent)] #[derive(Clone, Copy, Debug)] pub struct ConstantIdx(pub usize); +#[repr(transparent)] #[derive(Clone, Copy, Debug)] pub struct CodeIdx(pub usize); +#[repr(transparent)] +#[derive(Clone, Copy, Debug)] +pub struct JumpOffset(pub usize); + #[allow(clippy::enum_variant_names)] #[warn(variant_size_differences)] #[derive(Clone, Copy, Debug)] @@ -40,10 +46,10 @@ pub enum OpCode { OpMoreOrEq, // Logical operators & generic jumps - OpJump(usize), - OpJumpIfTrue(usize), - OpJumpIfFalse(usize), - OpJumpIfNotFound(usize), + OpJump(JumpOffset), + OpJumpIfTrue(JumpOffset), + OpJumpIfFalse(JumpOffset), + OpJumpIfNotFound(JumpOffset), // Attribute sets OpAttrs(usize), diff --git a/tvix/eval/src/vm.rs b/tvix/eval/src/vm.rs index 6de9cd03c49c..a74051a64975 100644 --- a/tvix/eval/src/vm.rs +++ b/tvix/eval/src/vm.rs @@ -6,7 +6,7 @@ use std::rc::Rc; use crate::{ chunk::Chunk, errors::{ErrorKind, EvalResult}, - opcode::OpCode, + opcode::{JumpOffset, OpCode}, value::{Closure, Lambda, NixAttrs, NixList, Value}, }; @@ -266,23 +266,23 @@ impl VM { OpCode::OpInterpolate(count) => self.run_interpolate(count)?, - OpCode::OpJump(offset) => { + OpCode::OpJump(JumpOffset(offset)) => { self.frame_mut().ip += offset; } - OpCode::OpJumpIfTrue(offset) => { + OpCode::OpJumpIfTrue(JumpOffset(offset)) => { if self.peek(0).as_bool()? { self.frame_mut().ip += offset; } } - OpCode::OpJumpIfFalse(offset) => { + OpCode::OpJumpIfFalse(JumpOffset(offset)) => { if !self.peek(0).as_bool()? { self.frame_mut().ip += offset; } } - OpCode::OpJumpIfNotFound(offset) => { + OpCode::OpJumpIfNotFound(JumpOffset(offset)) => { if matches!(self.peek(0), Value::AttrNotFound) { self.pop(); self.frame_mut().ip += offset; -- cgit 1.4.1