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 <sternenseemann@systemli.org>
This commit is contained in:
Vincent Ambo 2022-08-26 20:46:43 +03:00 committed by tazjin
parent 3b64b7eb2e
commit 4f00f75e31
3 changed files with 27 additions and 18 deletions

View file

@ -22,7 +22,7 @@ use std::rc::Rc;
use crate::chunk::Chunk; use crate::chunk::Chunk;
use crate::errors::{Error, ErrorKind, EvalResult}; 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::value::{Closure, Lambda, Value};
use crate::warnings::{EvalWarning, WarningKind}; use crate::warnings::{EvalWarning, WarningKind};
@ -371,7 +371,7 @@ impl Compiler {
// If this value is false, jump over the right-hand side - the // If this value is false, jump over the right-hand side - the
// whole expression is false. // 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 // Otherwise, remove the previous value and leave the
// right-hand side on the stack. Its result is now the value // 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 // Opposite of above: If this value is **true**, we can
// short-circuit the right-hand side. // 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.chunk().push_op(OpCode::OpPop);
self.compile(node.rhs().unwrap()); self.compile(node.rhs().unwrap());
self.patch_jump(end_idx); self.patch_jump(end_idx);
@ -414,7 +414,7 @@ impl Compiler {
self.chunk().push_op(OpCode::OpInvert); self.chunk().push_op(OpCode::OpInvert);
// Exactly as `||` (because `a -> b` = `!a || b`). // 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.chunk().push_op(OpCode::OpPop);
self.compile(node.rhs().unwrap()); self.compile(node.rhs().unwrap());
self.patch_jump(end_idx); self.patch_jump(end_idx);
@ -617,10 +617,13 @@ impl Compiler {
for fragment in path.attrs() { for fragment in path.attrs() {
self.compile_attr(fragment); self.compile_attr(fragment);
self.chunk().push_op(OpCode::OpAttrsTrySelect); 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 { for jump in jumps {
self.patch_jump(jump); self.patch_jump(jump);
@ -656,12 +659,12 @@ impl Compiler {
fn compile_if_else(&mut self, node: ast::IfElse) { fn compile_if_else(&mut self, node: ast::IfElse) {
self.compile(node.condition().unwrap()); 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.chunk().push_op(OpCode::OpPop); // discard condition value
self.compile(node.body().unwrap()); 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.patch_jump(then_idx); // patch jump *to* else_body
self.chunk().push_op(OpCode::OpPop); // discard condition value 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 /// not known at the time when the jump operation itself is
/// emitted. /// emitted.
fn patch_jump(&mut self, idx: CodeIdx) { 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] { match &mut self.chunk().code[idx.0] {
OpCode::OpJump(n) OpCode::OpJump(n)

View file

@ -1,12 +1,18 @@
//! This module implements the instruction set running on the abstract //! This module implements the instruction set running on the abstract
//! machine implemented by tvix. //! machine implemented by tvix.
#[repr(transparent)]
#[derive(Clone, Copy, Debug)] #[derive(Clone, Copy, Debug)]
pub struct ConstantIdx(pub usize); pub struct ConstantIdx(pub usize);
#[repr(transparent)]
#[derive(Clone, Copy, Debug)] #[derive(Clone, Copy, Debug)]
pub struct CodeIdx(pub usize); pub struct CodeIdx(pub usize);
#[repr(transparent)]
#[derive(Clone, Copy, Debug)]
pub struct JumpOffset(pub usize);
#[allow(clippy::enum_variant_names)] #[allow(clippy::enum_variant_names)]
#[warn(variant_size_differences)] #[warn(variant_size_differences)]
#[derive(Clone, Copy, Debug)] #[derive(Clone, Copy, Debug)]
@ -40,10 +46,10 @@ pub enum OpCode {
OpMoreOrEq, OpMoreOrEq,
// Logical operators & generic jumps // Logical operators & generic jumps
OpJump(usize), OpJump(JumpOffset),
OpJumpIfTrue(usize), OpJumpIfTrue(JumpOffset),
OpJumpIfFalse(usize), OpJumpIfFalse(JumpOffset),
OpJumpIfNotFound(usize), OpJumpIfNotFound(JumpOffset),
// Attribute sets // Attribute sets
OpAttrs(usize), OpAttrs(usize),

View file

@ -6,7 +6,7 @@ use std::rc::Rc;
use crate::{ use crate::{
chunk::Chunk, chunk::Chunk,
errors::{ErrorKind, EvalResult}, errors::{ErrorKind, EvalResult},
opcode::OpCode, opcode::{JumpOffset, OpCode},
value::{Closure, Lambda, NixAttrs, NixList, Value}, value::{Closure, Lambda, NixAttrs, NixList, Value},
}; };
@ -266,23 +266,23 @@ impl VM {
OpCode::OpInterpolate(count) => self.run_interpolate(count)?, OpCode::OpInterpolate(count) => self.run_interpolate(count)?,
OpCode::OpJump(offset) => { OpCode::OpJump(JumpOffset(offset)) => {
self.frame_mut().ip += offset; self.frame_mut().ip += offset;
} }
OpCode::OpJumpIfTrue(offset) => { OpCode::OpJumpIfTrue(JumpOffset(offset)) => {
if self.peek(0).as_bool()? { if self.peek(0).as_bool()? {
self.frame_mut().ip += offset; self.frame_mut().ip += offset;
} }
} }
OpCode::OpJumpIfFalse(offset) => { OpCode::OpJumpIfFalse(JumpOffset(offset)) => {
if !self.peek(0).as_bool()? { if !self.peek(0).as_bool()? {
self.frame_mut().ip += offset; self.frame_mut().ip += offset;
} }
} }
OpCode::OpJumpIfNotFound(offset) => { OpCode::OpJumpIfNotFound(JumpOffset(offset)) => {
if matches!(self.peek(0), Value::AttrNotFound) { if matches!(self.peek(0), Value::AttrNotFound) {
self.pop(); self.pop();
self.frame_mut().ip += offset; self.frame_mut().ip += offset;