Avoid cloning constant and move identifier stack to the chunk

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<Value>,
+    constants: Vec<Option<Value>>,
     identifiers: Vec<Local>,
     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<Value, ChunkError> {
+    pub fn use_constant(&mut self, index: u8, position: Span) -> Result<Value, ChunkError> {
         let index = index as usize;
 
-        if index >= self.constants.len() {
+        self.constants
-            Err(ChunkError::ConstantIndexOutOfBounds {
+            .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<u8, ChunkError> {
@@ -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> {
+    pub fn get_identifier(&self, index: u8) -> Option<&Identifier> {
-        self.identifiers
+        if let Some(local) = self.identifiers.get(index as usize) {
-            .get(index as usize)
+            Some(&local.identifier)
-            .map(|local| &local.identifier)
+        } else {
-            .ok_or(ChunkError::IdentifierIndexOutOfBounds { index, position })
+            None
+        }
     }
 
     pub fn get_identifier_index(
@@ -114,8 +123,8 @@ impl Chunk {
     ) -> Result<u8, ChunkError> {
         self.identifiers
             .iter()
-            .enumerate()
             .rev()
+            .enumerate()
             .find_map(|(index, local)| {
                 if &local.identifier == identifier {
                     Some(index as u8)
@@ -216,20 +225,25 @@ impl Chunk {
         output.push_str("INDEX KIND VALUE\n");
         output.push_str("----- ---- -----\n");
 
-        for (index, value) in self.constants.iter().enumerate() {
+        for (index, value_option) in self.constants.iter().enumerate() {
-            let value_kind_display = match value {
+            let value_kind_display = match value_option {
-                Value::Raw(_) => "RAW ",
+                Some(Value::Raw(_)) => "RAW ",
-                Value::Reference(_) => "REF ",
+                Some(Value::Reference(_)) => "REF ",
-                Value::Mutable(_) => "MUT ",
+                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");
-        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<String> {
         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!(),

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) {
+                let identifier_display = chunk
-                    Ok(identifier) => identifier.to_string(),
+                    .get_identifier(*argument)
-                    Err(error) => format!("{error:?}"),
+                    .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) {
+                let identifier_display = chunk
-                    Ok(identifier) => identifier.to_string(),
+                    .get_identifier(*argument)
-                    Err(error) => format!("{error:?}"),
+                    .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) {
+                let identifier_display = chunk
-                    Ok(identifier) => identifier.to_string(),
+                    .get_identifier(*argument)
-                    Err(error) => format!("{error:?}"),
+                    .map(|identifier| identifier.to_string())
-                };
+                    .unwrap_or_else(|| "ERROR".to_string());
 
                 format!("SET_VARIABLE {identifier_display}")
             }

dust-lang/src/parser.rs

@@ -288,8 +288,7 @@ impl<'src> Parser<'src> {
             .chunk
             .identifiers()
             .iter()
-            .rev()
+            .next_back()
-            .next()
             .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,

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<Option<Value>, DustError> {
 
 #[derive(Debug, Eq, PartialEq)]
 pub struct Vm {
-    chunk: Rc<Chunk>,
+    chunk: Chunk,
     ip: usize,
     stack: Vec<Value>,
 }
@@ -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,

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))));
+}