refactor(tazjin/rlox): Introduce newtypes for various indices r/2980

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>

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 7132be430a0f..b9a3994fe1c1 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 28b00cbb6ed8..02757a7a1422 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<T: Iterator<Item = Token>> Compiler<T> {
         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<T: Iterator<Item = Token>> Compiler<T> {
         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)?;
         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);
         for (idx, local) in self.locals.locals.iter().enumerate().rev() {
             if self.previous().lexeme == local.name.lexeme {
@@ -506,21 +506,20 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
                     // 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<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!(
             self,
             TokenKind::Identifier(_),
@@ -555,7 +555,7 @@ impl<T: Iterator<Item = Token>> Compiler<T> {
 
         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<T: Iterator<Item = Token>> Compiler<T> {
         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 143a79f41916..d2a264242496 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 10d0f595d6d4..bbdf70d8dc2f 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();
                 }
             }