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 <mail@tazj.in>
This commit is contained in:
Vincent Ambo 2021-10-19 22:59:48 +02:00 committed by tazjin
parent e92a951f6c
commit 050a2b473c
4 changed files with 31 additions and 25 deletions

View file

@ -1,6 +1,6 @@
use std::ops::Index; use std::ops::Index;
use super::opcode::OpCode; use super::opcode::{ConstantIdx, OpCode};
use super::value; use super::value;
// In the book, this type is a hand-rolled dynamic array // In the book, this type is a hand-rolled dynamic array
@ -38,8 +38,8 @@ impl Chunk {
idx idx
} }
pub fn constant(&self, idx: usize) -> &value::Value { pub fn constant(&self, idx: ConstantIdx) -> &value::Value {
self.constants.index(idx) self.constants.index(idx.0)
} }
fn add_line(&mut self, line: usize) { fn add_line(&mut self, line: usize) {
@ -79,7 +79,7 @@ pub fn disassemble_instruction(chunk: &Chunk, offset: usize) {
match chunk.code.index(offset) { match chunk.code.index(offset) {
OpCode::OpConstant(idx) => { OpCode::OpConstant(idx) => {
println!("OpConstant({}) '{:?}'", *idx, chunk.constant(*idx)) println!("OpConstant({:?}) '{:?}'", idx, chunk.constant(*idx))
} }
op => println!("{:?}", op), op => println!("{:?}", op),
} }

View file

@ -1,7 +1,7 @@
use super::chunk::Chunk; use super::chunk::Chunk;
use super::errors::{Error, ErrorKind, LoxResult}; use super::errors::{Error, ErrorKind, LoxResult};
use super::interner::{InternedStr, Interner}; use super::interner::{InternedStr, Interner};
use super::opcode::OpCode; use super::opcode::{ConstantIdx, OpCode, StackIdx};
use super::value::Value; use super::value::Value;
use crate::scanner::{self, Token, TokenKind}; use crate::scanner::{self, Token, TokenKind};
@ -234,7 +234,7 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
self.define_variable(global) 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 { if self.locals.scope_depth == 0 {
self.emit_op(OpCode::OpDefineGlobal(var)); self.emit_op(OpCode::OpDefineGlobal(var));
} else { } else {
@ -493,12 +493,12 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
Ok(self.strings.intern(ident)) Ok(self.strings.intern(ident))
} }
fn identifier_constant(&mut self) -> LoxResult<usize> { fn identifier_constant(&mut self) -> LoxResult<ConstantIdx> {
let ident = self.identifier_str(Self::previous)?; let ident = self.identifier_str(Self::previous)?;
Ok(self.emit_constant(Value::String(ident.into()), false)) Ok(self.emit_constant(Value::String(ident.into()), false))
} }
fn resolve_local(&mut self) -> Option<usize> { fn resolve_local(&mut self) -> Option<StackIdx> {
dbg!(&self.locals); dbg!(&self.locals);
for (idx, local) in self.locals.locals.iter().enumerate().rev() { for (idx, local) in self.locals.locals.iter().enumerate().rev() {
if self.previous().lexeme == local.name.lexeme { if self.previous().lexeme == local.name.lexeme {
@ -506,21 +506,20 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
// TODO(tazjin): *return* err // TODO(tazjin): *return* err
panic!("can't read variable in its own initialiser"); panic!("can't read variable in its own initialiser");
} }
return Some(idx); return Some(StackIdx(idx));
} }
} }
None None
} }
fn add_local(&mut self, name: Token) -> LoxResult<()> { fn add_local(&mut self, name: Token) {
let local = Local { let local = Local {
name, name,
depth: Depth::Unitialised, depth: Depth::Unitialised,
}; };
self.locals.locals.push(local); self.locals.locals.push(local);
Ok(()) // TODO(tazjin): needed?
} }
fn declare_variable(&mut self) -> LoxResult<()> { fn declare_variable(&mut self) -> LoxResult<()> {
@ -543,10 +542,11 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
} }
} }
self.add_local(name) self.add_local(name);
Ok(())
} }
fn parse_variable(&mut self) -> LoxResult<usize> { fn parse_variable(&mut self) -> LoxResult<ConstantIdx> {
consume!( consume!(
self, self,
TokenKind::Identifier(_), TokenKind::Identifier(_),
@ -555,7 +555,7 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
self.declare_variable()?; self.declare_variable()?;
if self.locals.scope_depth > 0 { 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)?; let id = self.identifier_str(Self::previous)?;
@ -583,8 +583,8 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
self.current_chunk().add_op(op, line); self.current_chunk().add_op(op, line);
} }
fn emit_constant(&mut self, val: Value, with_op: bool) -> usize { fn emit_constant(&mut self, val: Value, with_op: bool) -> ConstantIdx {
let idx = self.chunk.add_constant(val); let idx = ConstantIdx(self.chunk.add_constant(val));
if with_op { if with_op {
self.emit_op(OpCode::OpConstant(idx)); self.emit_op(OpCode::OpConstant(idx));

View file

@ -1,7 +1,13 @@
#[derive(Clone, Copy, Debug)]
pub struct ConstantIdx(pub usize);
#[derive(Clone, Copy, Debug)]
pub struct StackIdx(pub usize);
#[derive(Debug)] #[derive(Debug)]
pub enum OpCode { pub enum OpCode {
/// Push a constant onto the stack. /// Push a constant onto the stack.
OpConstant(usize), OpConstant(ConstantIdx),
// Literal pushes // Literal pushes
OpNil, OpNil,
@ -31,9 +37,9 @@ pub enum OpCode {
OpPop, OpPop,
// Variable management // Variable management
OpDefineGlobal(usize), OpDefineGlobal(ConstantIdx),
OpGetGlobal(usize), OpGetGlobal(ConstantIdx),
OpSetGlobal(usize), OpSetGlobal(ConstantIdx),
OpGetLocal(usize), OpGetLocal(StackIdx),
OpSetLocal(usize), OpSetLocal(StackIdx),
} }

View file

@ -196,13 +196,13 @@ impl VM {
} }
OpCode::OpGetLocal(local_idx) => { OpCode::OpGetLocal(local_idx) => {
let value = self.stack[*local_idx].clone(); let value = self.stack[local_idx.0].clone();
self.push(value); self.push(value);
} }
OpCode::OpSetLocal(local_idx) => { OpCode::OpSetLocal(local_idx) => {
debug_assert!(self.stack.len() > *local_idx, "stack is not currently large enough for local"); debug_assert!(self.stack.len() > local_idx.0, "stack is not currently large enough for local");
self.stack[*local_idx] = self.stack.last().unwrap().clone(); self.stack[local_idx.0] = self.stack.last().unwrap().clone();
} }
} }