about summary refs log tree commit diff
diff options
context:
space:
mode:
authorVincent Ambo <mail@tazj.in>2022-08-26T17·46+0300
committertazjin <tazjin@tvl.su>2022-09-03T13·22+0000
commit4f00f75e31afc6bb1f163aa67e4b1cb0cadb2e12 (patch)
treea1d3d6c69841191901290a03d1e3c91a315dabf3
parent3b64b7eb2ec178c11056d1e361def5b5b965ba43 (diff)
refactor(tvix/eval): add opcode::JumpOffset type for less ambiguity r/4619
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 <sternenseemann@systemli.org>
-rw-r--r--tvix/eval/src/compiler/mod.rs21
-rw-r--r--tvix/eval/src/opcode.rs14
-rw-r--r--tvix/eval/src/vm.rs10
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;