From 050a2b473c48b87994e56ade381afbfc2bca4de3 Mon Sep 17 00:00:00 2001 From: Vincent Ambo Date: Tue, 19 Oct 2021 22:59:48 +0200 Subject: refactor(tazjin/rlox): Introduce newtypes for various indices Since this code is essentially a fairly plain translation from C, it is a bit confusing to deal with the original untyped code. This is an attempt to try and clean some of it up. Change-Id: Icd21f531932e1a811c0d6dbf2e9acba61ca9c45d Reviewed-on: https://cl.tvl.fyi/c/depot/+/3734 Tested-by: BuildkiteCI Reviewed-by: tazjin --- users/tazjin/rlox/src/bytecode/chunk.rs | 8 ++++---- users/tazjin/rlox/src/bytecode/compiler.rs | 24 ++++++++++++------------ users/tazjin/rlox/src/bytecode/opcode.rs | 18 ++++++++++++------ users/tazjin/rlox/src/bytecode/vm.rs | 6 +++--- 4 files changed, 31 insertions(+), 25 deletions(-) diff --git a/users/tazjin/rlox/src/bytecode/chunk.rs b/users/tazjin/rlox/src/bytecode/chunk.rs index 7132be430a..b9a3994fe1 100644 --- a/users/tazjin/rlox/src/bytecode/chunk.rs +++ b/users/tazjin/rlox/src/bytecode/chunk.rs @@ -1,6 +1,6 @@ use std::ops::Index; -use super::opcode::OpCode; +use super::opcode::{ConstantIdx, OpCode}; use super::value; // In the book, this type is a hand-rolled dynamic array @@ -38,8 +38,8 @@ impl Chunk { idx } - pub fn constant(&self, idx: usize) -> &value::Value { - self.constants.index(idx) + pub fn constant(&self, idx: ConstantIdx) -> &value::Value { + self.constants.index(idx.0) } fn add_line(&mut self, line: usize) { @@ -79,7 +79,7 @@ pub fn disassemble_instruction(chunk: &Chunk, offset: usize) { match chunk.code.index(offset) { OpCode::OpConstant(idx) => { - println!("OpConstant({}) '{:?}'", *idx, chunk.constant(*idx)) + println!("OpConstant({:?}) '{:?}'", idx, chunk.constant(*idx)) } op => println!("{:?}", op), } diff --git a/users/tazjin/rlox/src/bytecode/compiler.rs b/users/tazjin/rlox/src/bytecode/compiler.rs index 28b00cbb6e..02757a7a14 100644 --- a/users/tazjin/rlox/src/bytecode/compiler.rs +++ b/users/tazjin/rlox/src/bytecode/compiler.rs @@ -1,7 +1,7 @@ use super::chunk::Chunk; use super::errors::{Error, ErrorKind, LoxResult}; use super::interner::{InternedStr, Interner}; -use super::opcode::OpCode; +use super::opcode::{ConstantIdx, OpCode, StackIdx}; use super::value::Value; use crate::scanner::{self, Token, TokenKind}; @@ -234,7 +234,7 @@ impl> Compiler { self.define_variable(global) } - fn define_variable(&mut self, var: usize) -> LoxResult<()> { + fn define_variable(&mut self, var: ConstantIdx) -> LoxResult<()> { if self.locals.scope_depth == 0 { self.emit_op(OpCode::OpDefineGlobal(var)); } else { @@ -493,12 +493,12 @@ impl> Compiler { Ok(self.strings.intern(ident)) } - fn identifier_constant(&mut self) -> LoxResult { + fn identifier_constant(&mut self) -> LoxResult { let ident = self.identifier_str(Self::previous)?; Ok(self.emit_constant(Value::String(ident.into()), false)) } - fn resolve_local(&mut self) -> Option { + fn resolve_local(&mut self) -> Option { dbg!(&self.locals); for (idx, local) in self.locals.locals.iter().enumerate().rev() { if self.previous().lexeme == local.name.lexeme { @@ -506,21 +506,20 @@ impl> Compiler { // TODO(tazjin): *return* err panic!("can't read variable in its own initialiser"); } - return Some(idx); + return Some(StackIdx(idx)); } } None } - fn add_local(&mut self, name: Token) -> LoxResult<()> { + fn add_local(&mut self, name: Token) { let local = Local { name, depth: Depth::Unitialised, }; self.locals.locals.push(local); - Ok(()) // TODO(tazjin): needed? } fn declare_variable(&mut self) -> LoxResult<()> { @@ -543,10 +542,11 @@ impl> Compiler { } } - self.add_local(name) + self.add_local(name); + Ok(()) } - fn parse_variable(&mut self) -> LoxResult { + fn parse_variable(&mut self) -> LoxResult { consume!( self, TokenKind::Identifier(_), @@ -555,7 +555,7 @@ impl> Compiler { self.declare_variable()?; if self.locals.scope_depth > 0 { - return Ok(0); // TODO(tazjin): grr sentinel + return Ok(ConstantIdx(0)); // TODO(tazjin): grr sentinel } let id = self.identifier_str(Self::previous)?; @@ -583,8 +583,8 @@ impl> Compiler { self.current_chunk().add_op(op, line); } - fn emit_constant(&mut self, val: Value, with_op: bool) -> usize { - let idx = self.chunk.add_constant(val); + fn emit_constant(&mut self, val: Value, with_op: bool) -> ConstantIdx { + let idx = ConstantIdx(self.chunk.add_constant(val)); if with_op { self.emit_op(OpCode::OpConstant(idx)); diff --git a/users/tazjin/rlox/src/bytecode/opcode.rs b/users/tazjin/rlox/src/bytecode/opcode.rs index 143a79f419..d2a2642424 100644 --- a/users/tazjin/rlox/src/bytecode/opcode.rs +++ b/users/tazjin/rlox/src/bytecode/opcode.rs @@ -1,7 +1,13 @@ +#[derive(Clone, Copy, Debug)] +pub struct ConstantIdx(pub usize); + +#[derive(Clone, Copy, Debug)] +pub struct StackIdx(pub usize); + #[derive(Debug)] pub enum OpCode { /// Push a constant onto the stack. - OpConstant(usize), + OpConstant(ConstantIdx), // Literal pushes OpNil, @@ -31,9 +37,9 @@ pub enum OpCode { OpPop, // Variable management - OpDefineGlobal(usize), - OpGetGlobal(usize), - OpSetGlobal(usize), - OpGetLocal(usize), - OpSetLocal(usize), + OpDefineGlobal(ConstantIdx), + OpGetGlobal(ConstantIdx), + OpSetGlobal(ConstantIdx), + OpGetLocal(StackIdx), + OpSetLocal(StackIdx), } diff --git a/users/tazjin/rlox/src/bytecode/vm.rs b/users/tazjin/rlox/src/bytecode/vm.rs index 10d0f595d6..bbdf70d8dc 100644 --- a/users/tazjin/rlox/src/bytecode/vm.rs +++ b/users/tazjin/rlox/src/bytecode/vm.rs @@ -196,13 +196,13 @@ impl VM { } OpCode::OpGetLocal(local_idx) => { - let value = self.stack[*local_idx].clone(); + let value = self.stack[local_idx.0].clone(); self.push(value); } OpCode::OpSetLocal(local_idx) => { - debug_assert!(self.stack.len() > *local_idx, "stack is not currently large enough for local"); - self.stack[*local_idx] = self.stack.last().unwrap().clone(); + debug_assert!(self.stack.len() > local_idx.0, "stack is not currently large enough for local"); + self.stack[local_idx.0] = self.stack.last().unwrap().clone(); } } -- cgit 1.4.1