From 86f8e47b0ce082422fb3a87e3dc017dc5ed11ee7 Mon Sep 17 00:00:00 2001 From: Jeff Date: Wed, 11 Sep 2024 04:22:54 -0400 Subject: [PATCH] Avoid cloning constant and move identifier stack to the chunk --- dust-lang/src/chunk.rs | 83 +++++++++++++++++++++++++----------- dust-lang/src/instruction.rs | 24 +++++------ dust-lang/src/parser.rs | 5 +-- dust-lang/src/vm.rs | 30 +++++++++---- dust-lang/tests/variables.rs | 16 +++++++ 5 files changed, 110 insertions(+), 48 deletions(-) diff --git a/dust-lang/src/chunk.rs b/dust-lang/src/chunk.rs index 4a1a8f9..e9b983f 100644 --- a/dust-lang/src/chunk.rs +++ b/dust-lang/src/chunk.rs @@ -4,10 +4,10 @@ use serde::{Deserialize, Serialize}; use crate::{AnnotatedError, Identifier, Instruction, Span, Value}; -#[derive(Clone, Eq, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Serialize, Deserialize)] pub struct Chunk { code: Vec<(u8, Span)>, - constants: Vec, + constants: Vec>, identifiers: Vec, scope_depth: usize, } @@ -29,7 +29,7 @@ impl Chunk { ) -> Self { Self { code, - constants, + constants: constants.into_iter().map(Some).collect(), identifiers, scope_depth: 0, } @@ -61,19 +61,27 @@ impl Chunk { self.constants .get(index as usize) .ok_or(ChunkError::ConstantIndexOutOfBounds { index, position }) + .and_then(|value| { + value + .as_ref() + .ok_or(ChunkError::ConstantAlreadyUsed { index, position }) + }) } - pub fn remove_constant(&mut self, index: u8, position: Span) -> Result { + pub fn use_constant(&mut self, index: u8, position: Span) -> Result { let index = index as usize; - if index >= self.constants.len() { - Err(ChunkError::ConstantIndexOutOfBounds { + self.constants + .get_mut(index) + .ok_or_else(|| ChunkError::ConstantIndexOutOfBounds { + index: index as u8, + position, + })? + .take() + .ok_or(ChunkError::ConstantAlreadyUsed { index: index as u8, position, }) - } else { - Ok(self.constants.remove(index)) - } } pub fn push_constant(&mut self, value: Value, position: Span) -> Result { @@ -82,7 +90,7 @@ impl Chunk { if starting_length + 1 > (u8::MAX as usize) { Err(ChunkError::ConstantOverflow { position }) } else { - self.constants.push(value); + self.constants.push(Some(value)); Ok(starting_length as u8) } @@ -100,11 +108,12 @@ impl Chunk { .ok_or(ChunkError::IdentifierIndexOutOfBounds { index, position }) } - pub fn get_identifier(&self, index: u8, position: Span) -> Result<&Identifier, ChunkError> { - self.identifiers - .get(index as usize) - .map(|local| &local.identifier) - .ok_or(ChunkError::IdentifierIndexOutOfBounds { index, position }) + pub fn get_identifier(&self, index: u8) -> Option<&Identifier> { + if let Some(local) = self.identifiers.get(index as usize) { + Some(&local.identifier) + } else { + None + } } pub fn get_identifier_index( @@ -114,8 +123,8 @@ impl Chunk { ) -> Result { self.identifiers .iter() - .enumerate() .rev() + .enumerate() .find_map(|(index, local)| { if &local.identifier == identifier { Some(index as u8) @@ -211,25 +220,30 @@ impl Chunk { previous = Some(instruction); } - output.push_str("\n Constants \n"); + output.push_str("\n Constants\n"); output.push_str("----- ---- -----\n"); output.push_str("INDEX KIND VALUE\n"); output.push_str("----- ---- -----\n"); - for (index, value) in self.constants.iter().enumerate() { - let value_kind_display = match value { - Value::Raw(_) => "RAW ", - Value::Reference(_) => "REF ", - Value::Mutable(_) => "MUT ", + for (index, value_option) in self.constants.iter().enumerate() { + let value_kind_display = match value_option { + Some(Value::Raw(_)) => "RAW ", + Some(Value::Reference(_)) => "REF ", + Some(Value::Mutable(_)) => "MUT ", + None => "EMPTY", }; - let display = format!("{index:3} {value_kind_display} {value}\n"); + let value_display = value_option + .as_ref() + .map(|value| value.to_string()) + .unwrap_or_else(|| "EMPTY".to_string()); + let display = format!("{index:3} {value_kind_display} {value_display}\n",); output.push_str(&display); } - output.push_str("\n Identifiers \n"); + output.push_str("\n Identifiers\n"); output.push_str("----- ---------- -----\n"); - output.push_str("INDEX IDENTIFIER DEPTH\n"); + output.push_str("INDEX NAME DEPTH\n"); output.push_str("----- ---------- -----\n"); for (index, Local { identifier, depth }) in self.identifiers.iter().enumerate() { @@ -259,6 +273,16 @@ impl Debug for Chunk { } } +impl Eq for Chunk {} + +impl PartialEq for Chunk { + fn eq(&self, other: &Self) -> bool { + self.code == other.code + && self.constants == other.constants + && self.identifiers == other.identifiers + } +} + #[derive(Debug, Clone, Eq, PartialEq, Serialize, Deserialize)] pub struct Local { pub identifier: Identifier, @@ -271,6 +295,10 @@ pub enum ChunkError { offset: usize, position: Span, }, + ConstantAlreadyUsed { + index: u8, + position: Span, + }, ConstantOverflow { position: Span, }, @@ -299,6 +327,7 @@ impl AnnotatedError for ChunkError { fn description(&self) -> &'static str { match self { ChunkError::CodeIndexOfBounds { .. } => "Code index out of bounds", + ChunkError::ConstantAlreadyUsed { .. } => "Constant already used", ChunkError::ConstantOverflow { .. } => "Constant overflow", ChunkError::ConstantIndexOutOfBounds { .. } => "Constant index out of bounds", ChunkError::IdentifierIndexOutOfBounds { .. } => "Identifier index out of bounds", @@ -310,6 +339,9 @@ impl AnnotatedError for ChunkError { fn details(&self) -> Option { match self { ChunkError::CodeIndexOfBounds { offset, .. } => Some(format!("Code index: {}", offset)), + ChunkError::ConstantAlreadyUsed { index, .. } => { + Some(format!("Constant index: {}", index)) + } ChunkError::ConstantIndexOutOfBounds { index, .. } => { Some(format!("Constant index: {}", index)) } @@ -326,6 +358,7 @@ impl AnnotatedError for ChunkError { fn position(&self) -> Span { match self { ChunkError::CodeIndexOfBounds { position, .. } => *position, + ChunkError::ConstantAlreadyUsed { position, .. } => *position, ChunkError::ConstantIndexOutOfBounds { position, .. } => *position, ChunkError::IdentifierNotFound { position, .. } => *position, _ => todo!(), diff --git a/dust-lang/src/instruction.rs b/dust-lang/src/instruction.rs index 5dc5a8e..b0641e8 100644 --- a/dust-lang/src/instruction.rs +++ b/dust-lang/src/instruction.rs @@ -80,29 +80,29 @@ impl Instruction { // Variables Instruction::DeclareVariable => { let (argument, _) = chunk.get_code(offset + 1, dummy_position).unwrap(); - let identifier_display = match chunk.get_identifier(*argument, dummy_position) { - Ok(identifier) => identifier.to_string(), - Err(error) => format!("{error:?}"), - }; + let identifier_display = chunk + .get_identifier(*argument) + .map(|identifier| identifier.to_string()) + .unwrap_or_else(|| "ERROR".to_string()); format!("DECLARE_VARIABLE {identifier_display}") } Instruction::GetVariable => { let (argument, _) = chunk.get_code(offset + 1, dummy_position).unwrap(); - let identifier_display = match chunk.get_identifier(*argument, dummy_position) { - Ok(identifier) => identifier.to_string(), - Err(error) => format!("{error:?}"), - }; + let identifier_display = chunk + .get_identifier(*argument) + .map(|identifier| identifier.to_string()) + .unwrap_or_else(|| "ERROR".to_string()); format!("GET_VARIABLE {identifier_display}") } Instruction::SetVariable => { let (argument, _) = chunk.get_code(offset + 1, dummy_position).unwrap(); - let identifier_display = match chunk.get_identifier(*argument, dummy_position) { - Ok(identifier) => identifier.to_string(), - Err(error) => format!("{error:?}"), - }; + let identifier_display = chunk + .get_identifier(*argument) + .map(|identifier| identifier.to_string()) + .unwrap_or_else(|| "ERROR".to_string()); format!("SET_VARIABLE {identifier_display}") } diff --git a/dust-lang/src/parser.rs b/dust-lang/src/parser.rs index 7ec0a0c..b7c3e0a 100644 --- a/dust-lang/src/parser.rs +++ b/dust-lang/src/parser.rs @@ -288,8 +288,7 @@ impl<'src> Parser<'src> { .chunk .identifiers() .iter() - .rev() - .next() + .next_back() .map_or(false, |local| local.depth > self.chunk.scope_depth()) { self.emit_byte(Instruction::Pop, self.current_position); @@ -390,7 +389,7 @@ impl<'src> Parser<'src> { self.previous_token, ); - if allow_assignment && self.allow(TokenKind::Equal)? { + if allow_assignment && self.current_token == Token::Equal { return Err(ParseError::InvalidAssignmentTarget { found: self.previous_token.to_owned(), position: self.previous_position, diff --git a/dust-lang/src/vm.rs b/dust-lang/src/vm.rs index 3c0ac90..e7cd931 100644 --- a/dust-lang/src/vm.rs +++ b/dust-lang/src/vm.rs @@ -1,5 +1,3 @@ -use std::rc::{Rc, Weak}; - use crate::{ dust_error::AnnotatedError, parse, Chunk, ChunkError, DustError, Identifier, Instruction, Span, Value, ValueError, @@ -16,7 +14,7 @@ pub fn run(source: &str) -> Result, DustError> { #[derive(Debug, Eq, PartialEq)] pub struct Vm { - chunk: Rc, + chunk: Chunk, ip: usize, stack: Vec, } @@ -26,7 +24,7 @@ impl Vm { pub fn new(chunk: Chunk) -> Self { Self { - chunk: Rc::new(chunk), + chunk, ip: 0, stack: Vec::with_capacity(Self::STACK_SIZE), } @@ -46,7 +44,7 @@ impl Vm { match instruction { Instruction::Constant => { let (argument, _) = *self.read(position)?; - let value = self.chunk.get_constant(argument, position)?.clone(); + let value = self.chunk.use_constant(argument, position)?; log::trace!("Pushing constant {value}"); @@ -68,7 +66,10 @@ impl Vm { // Variables Instruction::DeclareVariable => { let (argument, _) = *self.read(position)?; - let identifier = self.chunk.get_identifier(argument, position)?; + let identifier = self + .chunk + .get_identifier(argument) + .ok_or_else(|| VmError::UndeclaredVariable { position })?; let value = self.stack.remove(argument as usize); log::trace!("Declaring {identifier} as value {value}",); @@ -77,7 +78,10 @@ impl Vm { } Instruction::GetVariable => { let (argument, _) = *self.read(position)?; - let identifier = self.chunk.get_identifier(argument, position)?; + let identifier = self + .chunk + .get_identifier(argument) + .ok_or_else(|| VmError::UndeclaredVariable { position })?; let value = self.stack.remove(argument as usize); log::trace!("Getting {identifier} as value {value}",); @@ -86,7 +90,11 @@ impl Vm { } Instruction::SetVariable => { let (argument, _) = *self.read(position)?; - let identifier = self.chunk.get_identifier(argument, position)?.clone(); + let identifier = self + .chunk + .get_identifier(argument) + .ok_or_else(|| VmError::UndeclaredVariable { position })? + .clone(); if !self.chunk.contains_identifier(&identifier) { return Err(VmError::UndefinedVariable { @@ -273,6 +281,9 @@ pub enum VmError { InvalidInstruction(u8, Span), StackOverflow(Span), StackUnderflow(Span), + UndeclaredVariable { + position: Span, + }, UndefinedVariable { identifier: Identifier, position: Span, @@ -302,6 +313,7 @@ impl AnnotatedError for VmError { Self::InvalidInstruction(_, _) => "Invalid instruction", Self::StackOverflow(_) => "Stack overflow", Self::StackUnderflow(_) => "Stack underflow", + Self::UndeclaredVariable { .. } => "Undeclared variable", Self::UndefinedVariable { .. } => "Undefined variable", Self::Chunk(_) => "Chunk error", Self::Value { .. } => "Value error", @@ -315,6 +327,7 @@ impl AnnotatedError for VmError { )), Self::StackOverflow(position) => Some(format!("Stack overflow at {position}")), Self::StackUnderflow(position) => Some(format!("Stack underflow at {position}")), + Self::UndeclaredVariable { .. } => Some("Variable is not declared".to_string()), Self::UndefinedVariable { identifier, .. } => { Some(format!("{identifier} is not in scope")) } @@ -328,6 +341,7 @@ impl AnnotatedError for VmError { Self::InvalidInstruction(_, position) => *position, Self::StackUnderflow(position) => *position, Self::StackOverflow(position) => *position, + Self::UndeclaredVariable { position } => *position, Self::UndefinedVariable { position, .. } => *position, Self::Chunk(error) => error.position(), Self::Value { position, .. } => *position, diff --git a/dust-lang/tests/variables.rs b/dust-lang/tests/variables.rs index af6ad04..d4eff10 100644 --- a/dust-lang/tests/variables.rs +++ b/dust-lang/tests/variables.rs @@ -15,3 +15,19 @@ fn variable() { assert_eq!(result, Ok(Some(Value::integer(42)))); } + +#[test] +fn lots_of_variables() { + env_logger::builder().is_test(true).try_init().unwrap(); + + let source = " + let foo = 1; + let bar = 2; + let baz = 3; + let qux = 4; + let quux = 5; + foo + bar + baz + qux + quux"; + let result = run(source); + + assert_eq!(result, Ok(Some(Value::integer(15)))); +}