From a2d49ee8d301766addebbdd3a9902949a5eb2ef4 Mon Sep 17 00:00:00 2001 From: ctsk <9384305+ctsk@users.noreply.github.com> Date: Fri, 29 Nov 2024 10:39:17 +0100 Subject: [PATCH] [rlox] Bugfixes, lots of bugfixes --- rlox/src/lc.rs | 57 ++++++++++++++++--- rlox/src/main.rs | 26 ++++++--- rlox/src/vm.rs | 145 ++++++++++++++++++++++++++++++----------------- 3 files changed, 159 insertions(+), 69 deletions(-) diff --git a/rlox/src/lc.rs b/rlox/src/lc.rs index b0b6a76..7563cb2 100644 --- a/rlox/src/lc.rs +++ b/rlox/src/lc.rs @@ -262,7 +262,7 @@ impl<'src> Iterator for Scanner<'src> { } else if start_ch == '"' { match self.scan_string() { Ok(end) => self.make_token(TokenType::String, start_line, start_pos, end), - Err(end) => self.make_token(TokenType::Error(ScanErrorKind::UndelimitedString), start_line, start_pos, end), + Err(end) => self.make_token(TokenType::Error(ScanErrorKind::UndelimitedString), start_line, start_pos, end - 1), } } else { panic!("Invalid character"); @@ -292,6 +292,9 @@ pub enum ParseErrorKind { NoSemicolonAfterExpression, NoVariableName, NoSemicolonAfterVarDecl, + InvalidAssignmentTarget, + InvalidVariableName, + ScanError(ScanErrorKind), } impl fmt::Display for ParseErrorKind { @@ -303,6 +306,10 @@ impl fmt::Display for ParseErrorKind { ParseErrorKind::NoSemicolonAfterExpression => write!(f, "Expect ';' after expression."), ParseErrorKind::NoVariableName => write!(f, "Expect variable name."), ParseErrorKind::NoSemicolonAfterVarDecl => write!(f, "Expect ';' after variable declaration."), + ParseErrorKind::InvalidAssignmentTarget => write!(f, "Invalid assignment target."), + ParseErrorKind::InvalidVariableName => write!(f, "Expect variable name."), + ParseErrorKind::ScanError(ScanErrorKind::UndelimitedString) => + write!(f, "Unterminated string.") } } } @@ -371,6 +378,7 @@ impl<'src> Parser<'src> { Star | Slash => Some(Precedence::Factor), EqualEqual | BangEqual => Some(Precedence::Equality), Greater | GreaterEqual | Less | LessEqual => Some(Precedence::Comparison), + Equal => Some(Precedence::Assignment), _ => None, } } @@ -470,13 +478,21 @@ impl<'src> Parser<'src> { let offset = self.add_string(chunk, token.span); if self.scanner.peek().is_some_and(|t| t.ttype == TokenType::Equal) { - self.scanner.next(); - self._expression(chunk, Precedence::Assignment)?; - chunk.add_op(Op::SetGlobal {offset}, token.line); + if (min_prec <= Precedence::Assignment) { + self.scanner.next(); + self._expression(chunk, Precedence::Assignment)?; + chunk.add_op(Op::SetGlobal {offset}, token.line); + } else { + let eq_token = self.scanner.next().unwrap(); + return Err(self.error_at(eq_token, ParseErrorKind::InvalidAssignmentTarget)); + } } else { chunk.add_op(Op::GetGlobal {offset}, token.line); }; } + TokenType::Error(err) => { + return Err(self.error_at(token, ParseErrorKind::ScanError(err))); + } _ => { return Err(self.error_at(token, ParseErrorKind::IncompleteExpression)); } @@ -514,6 +530,7 @@ impl<'src> Parser<'src> { TokenType::BangEqual => chunk.add_op(Op::Equal, op.line).add_op(Op::Not, op.line), TokenType::GreaterEqual => chunk.add_op(Op::Less, op.line).add_op(Op::Not, op.line), TokenType::LessEqual => chunk.add_op(Op::Greater, op.line).add_op(Op::Not, op.line), + TokenType::Equal => {return Err(self.error_at(op, ParseErrorKind::InvalidAssignmentTarget))}, _ => unreachable!(), }; } @@ -562,13 +579,32 @@ impl<'src> Parser<'src> { fn synchronize(&mut self) { use TokenType::*; - while let Some(_token) = self.scanner.next_if( - |tok| ![Semicolon, Class, Fun, Var, For, If, While, Print, Return].contains(&tok.ttype) - ) {} + while let Some(peek) = self.scanner.peek() { + if peek.ttype == TokenType::Semicolon { + self.scanner.next(); + return; + } + + if [Class, Fun, Var, For, If, While, Print, Return].contains(&peek.ttype) { + return; + } + + self.scanner.next(); + } + } + + fn variable(&mut self) -> Result<'src, Token<'src>> { + let ident = self.must_consume(TokenType::Identifier, ParseErrorKind::NoVariableName)?; + + if (ident.span == "nil") { + Err(self.error_at(ident, ParseErrorKind::InvalidVariableName)) + } else { + Ok(ident) + } } fn var_declaration(&mut self, var_token: Token<'src>, chunk: &mut Chunk) -> Result<'src, ()> { - let ident = self.must_consume(TokenType::Identifier, ParseErrorKind::NoVariableName)?; + let ident = self.variable()?; let offset = self.add_string(chunk, ident.span); match self.scanner.peek() { @@ -593,7 +629,10 @@ impl<'src> Parser<'src> { match peeked.ttype { TokenType::Var => { self.scanner.next(); - self.var_declaration(peeked, chunk); + if let Err(err) = self.var_declaration(peeked, chunk) { + self.errors.push(err); + self.synchronize(); + }; }, _ => { self.statement(chunk).unwrap_or_else( diff --git a/rlox/src/main.rs b/rlox/src/main.rs index 0863fc8..5f49516 100644 --- a/rlox/src/main.rs +++ b/rlox/src/main.rs @@ -6,12 +6,13 @@ mod gc; use std::env; use std::fs; use std::io; +use std::process::ExitCode; use bc::Chunk; use vm::VM; -fn compile_and_run(source: &str, do_trace: bool) { +fn compile_and_run(source: &str, do_trace: bool) -> ExitCode { let mut chunk = Chunk::new(); let errors = lc::compile(source, &mut chunk); @@ -19,12 +20,17 @@ fn compile_and_run(source: &str, do_trace: bool) { let mut vm = VM::new(); vm.set_trace(do_trace); if let Err(err) = vm.stdrun(&chunk) { - eprintln!("{:?}", err); + eprintln!("{}", err); + ExitCode::from(70) + } else { + ExitCode::from(0) } + } else { for error in errors { - eprintln!("{}", error) + eprintln!("{}", error); } + ExitCode::from(65) } } @@ -45,21 +51,23 @@ fn repl() { } } -fn run_file(path: String) { +fn run_file(path: String) -> ExitCode { let do_trace = env::var("LOX_TRACE").is_ok(); let source = fs::read_to_string(path).unwrap(); - compile_and_run(source.as_str(), do_trace); + compile_and_run(source.as_str(), do_trace) } -fn main() { +fn main() -> ExitCode { let num_args = env::args().len(); if num_args == 1 { - repl() + repl(); + ExitCode::SUCCESS } else if num_args == 2 { let source = env::args().nth(1).unwrap(); - run_file(source); + run_file(source) } else { - println!("Usage: rlox [path]") + println!("Usage: rlox [path]"); + ExitCode::from(64) } } diff --git a/rlox/src/vm.rs b/rlox/src/vm.rs index e7d9752..cceb162 100644 --- a/rlox/src/vm.rs +++ b/rlox/src/vm.rs @@ -1,19 +1,55 @@ use crate::bc::{Chunk, Op, TraceInfo, Value}; use crate::gc::{GcHandle, ObjString, ObjectType, GC}; use std::collections::{hash_map, HashMap, LinkedList}; -use std::io; -use std::rc::Rc; +use std::{fmt, io}; pub struct VM { pub trace: bool, stack: Vec, pc: usize, + line: usize, } -#[derive(Debug, PartialEq)] -pub enum VMError { - // Compile, - Runtime(Rc, usize), +#[derive(Debug, PartialEq, Eq)] +pub struct VMError { + line: usize, + kind: VMErrorKind, + msg: Option, +} + +impl VMError { + fn new(kind: VMErrorKind, line: usize) -> Self { + VMError { line, kind, msg: None } + } +} + +#[derive(Debug, PartialEq, Eq)] +pub enum VMErrorKind { + InvalidAddOperands, + InvalidMathOperands, + InvalidMathOperand, + UndefinedVariable, + PopFromEmptyStack, +} + +impl fmt::Display for VMError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self.kind { + VMErrorKind::InvalidAddOperands => + {write!(f, "Operands must be two numbers or two strings.")?; } + VMErrorKind::InvalidMathOperands => + {write!(f, "Operands must be numbers.")?; } + VMErrorKind::InvalidMathOperand => + {write!(f, "Operand must be a number.")?; } + _ => {} + }; + + if let Some(msg) = &self.msg { + write!(f, "{}", msg)? + } + + write!(f, "\n[line {}]", self.line) + } } type Result = std::result::Result; @@ -24,6 +60,7 @@ impl VM { trace: false, stack: Vec::new(), pc: 0, + line: 0, } } @@ -31,34 +68,40 @@ impl VM { self.trace = trace; } - fn runtime_err(&self, msg: &'static str) -> VMError { - VMError::Runtime(msg.into(), self.pc) - } - - fn type_err(&self, expected: &'static str, found: Value) -> VMError { - VMError::Runtime( - format!("Expected: {:}, Found: {:?}", expected, found).into(), - self.pc, - ) - } - fn push(&mut self, value: Value) { self.stack.push(value); } + fn err(&mut self, kind: VMErrorKind) -> VMError { + VMError::new( + kind, + self.line + ) + } + fn pop(&mut self) -> Result { self.stack .pop() - .ok_or(self.runtime_err("Attempt to pop of empty stack.")) + .ok_or(self.err(VMErrorKind::PopFromEmptyStack)) } fn pop_num(&mut self) -> Result { let top_of_stack = self.pop()?; top_of_stack .as_num() - .ok_or(self.type_err("Number", top_of_stack)) + .ok_or(self.err(VMErrorKind::InvalidMathOperand)) } + fn pop_nums(&mut self) -> Result<(f64, f64)> { + let a = self.pop()?; + let b = self.pop()?; + match (a.as_num(), b.as_num()) { + (Some(a), Some(b)) => Ok((a, b)), + _ => Err(self.err(VMErrorKind::InvalidMathOperands)) + } + } + + pub fn stdrun( &mut self, chunk: &Chunk, @@ -74,8 +117,17 @@ impl VM { let mut allocations: LinkedList = LinkedList::new(); let mut globals: HashMap = HashMap::new(); + let err = |kind: VMErrorKind, msg: Option| -> VMError { + VMError { + line: chunk.debug_info[self.pc - 1], + kind, + msg + } + }; + while self.pc < chunk.code.len() { let instr = chunk.code[self.pc]; + self.line = chunk.debug_info[self.pc]; self.pc += 1; if self.trace { @@ -110,7 +162,8 @@ impl VM { let new_val = match top_of_stack { Value::Nil => Ok(true), Value::Bool(val) => Ok(!val), - _ => Err(self.type_err("Boolean or Nil", top_of_stack)), + _ => Err(self.err(VMErrorKind::InvalidMathOperand)), + //_ => Err(self.type_err("Boolean or Nil", top_of_stack)), }?; self.push(new_val.into()); } @@ -118,8 +171,9 @@ impl VM { let b = self.pop()?; match b { Value::Number(num) => { - let a = self.pop_num()?; + let a = self.pop_num().or(Err(self.err(VMErrorKind::InvalidAddOperands)))?; self.push(Value::from(num + a)); + Ok(()) } Value::Obj(b) => match b.get_otype() { ObjectType::String => { @@ -134,21 +188,15 @@ impl VM { Ok(()) } }, - _ => Err(self.type_err("String", a)), - }? + _ => Err(self.err(VMErrorKind::InvalidAddOperands)), + } } }, - _ => { - return Err(VMError::Runtime( - "Operands of + need to be numbers or strings".into(), - self.pc, - )) - } - }; + _ => Err(self.err(VMErrorKind::InvalidAddOperands)), + }? } Op::Subtract | Op::Multiply | Op::Divide => { - let b = self.pop_num()?; - let a = self.pop_num()?; + let (b, a) = self.pop_nums()?; let r = match instr { Op::Subtract => a - b, Op::Multiply => a * b, @@ -158,8 +206,7 @@ impl VM { self.push(r.into()) } Op::Greater | Op::Less => { - let b = self.pop_num()?; - let a = self.pop_num()?; + let (b, a) = self.pop_nums()?; let r = match instr { Op::Greater => a > b, Op::Less => a < b, @@ -175,8 +222,7 @@ impl VM { } Op::Print => { let value = self.pop()?; - writeln!(output, "{}", value) - .map_err(|_| VMError::Runtime("Failed to print".into(), self.pc))? + writeln!(output, "{}", value).unwrap() }, Op::Pop => { self.pop()?; @@ -185,11 +231,10 @@ impl VM { let ident = chunk.constants[offset as usize].clone(); if let Value::Obj(name) = ident { let name = name.downcast::().unwrap(); - globals.entry(name).or_insert(self.pop()?); - Ok(()) + globals.entry(name).insert_entry(self.pop()?); } else { unreachable!() - }? + }; }, Op::GetGlobal { offset } => { let ident = match chunk.constants[offset as usize] { @@ -202,12 +247,11 @@ impl VM { Ok(()) } else { - Err( - VMError::Runtime( - format!("Undefined variable '{}'.", ident).into(), - self.pc, - ) - ) + Err(VMError { + line: self.line, + kind: VMErrorKind::UndefinedVariable, + msg: Some(format!("Undefined variable '{}'.", ident)), + }) }? }, Op::SetGlobal { offset } => { @@ -223,12 +267,11 @@ impl VM { Ok(()) }, hash_map::Entry::Vacant(_) => { - Err( - VMError::Runtime( - format!("Undefined variable '{}'.", ident).into(), - self.pc, - ) - ) + Err(VMError { + line: self.line, + kind: VMErrorKind::UndefinedVariable, + msg: Some(format!("Undefined variable '{}'.", ident)), + }) }, }? },