[rlox] Bugfixes, lots of bugfixes

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();
+                        if (min_prec <= Precedence::Assignment) {
-                        self._expression(chunk, Precedence::Assignment)?;
+                            self.scanner.next();
-                        chunk.add_op(Op::SetGlobal {offset}, token.line);
+                            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(
+        while let Some(peek) = self.scanner.peek() {
-            |tok| ![Semicolon, Class, Fun, Var, For, If, While, Print, Return].contains(&tok.ttype)
+            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(

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

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::{fmt, io};
-use std::rc::Rc;
 
 pub struct VM {
     pub trace: bool,
     stack: Vec<Value>,
     pc: usize,
+    line: usize,
 }
 
-#[derive(Debug, PartialEq)]
+#[derive(Debug, PartialEq, Eq)]
-pub enum VMError {
+pub struct VMError {
-    // Compile,
+    line: usize,
-    Runtime(Rc<str>, usize),
+    kind: VMErrorKind,
+    msg: Option<String>,
+}
+
+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<T> = std::result::Result<T, VMError>;
@@ -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<Value> {
         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<f64> {
         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<GcHandle> = LinkedList::new();
         let mut globals: HashMap<ObjString, Value> = HashMap::new();
 
+        let err = |kind: VMErrorKind, msg: Option<String>| -> 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)),
-                                }?
+                                }
                             }
                         },
-                        _ => {
+                        _ => Err(self.err(VMErrorKind::InvalidAddOperands)),
-                            return Err(VMError::Runtime(
+                    }?
-                                "Operands of + need to be numbers or strings".into(),
-                                self.pc,
-                            ))
-                        }
-                    };
                 }
                 Op::Subtract | Op::Multiply | Op::Divide => {
-                    let b = self.pop_num()?;
+                    let (b, a) = self.pop_nums()?;
-                    let a = self.pop_num()?;
                     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 (b, a) = self.pop_nums()?;
-                    let a = self.pop_num()?;
                     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)
+                    writeln!(output, "{}", value).unwrap()
-                        .map_err(|_| VMError::Runtime("Failed to print".into(), self.pc))?
                 },
                 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::<ObjString>().unwrap();
-                        globals.entry(name).or_insert(self.pop()?);
+                        globals.entry(name).insert_entry(self.pop()?);
-                        Ok(())
                     } else {
                         unreachable!()
-                    }?
+                    };
                 },
                 Op::GetGlobal { offset } => {
                     let ident = match chunk.constants[offset as usize] {
@@ -202,12 +247,11 @@ impl VM {
 
                         Ok(())
                     } else {
-                        Err(
+                        Err(VMError {
-                            VMError::Runtime(
+                            line: self.line,
-                                format!("Undefined variable '{}'.", ident).into(),
+                            kind: VMErrorKind::UndefinedVariable,
-                                self.pc,
+                            msg: Some(format!("Undefined variable '{}'.", ident)),
-                            )
+                        })
-                        )
                     }?
                 },
                 Op::SetGlobal { offset } => {
@@ -223,12 +267,11 @@ impl VM {
                             Ok(())
                         },
                         hash_map::Entry::Vacant(_) => {
-                            Err(
+                            Err(VMError {
-                                VMError::Runtime(
+                                line: self.line,
-                                    format!("Undefined variable '{}'.", ident).into(),
+                                kind: VMErrorKind::UndefinedVariable,
-                                    self.pc,
+                                msg: Some(format!("Undefined variable '{}'.", ident)),
-                                )
+                            })
-                            )
                         },
                     }?
                 },