From 8179611c237e102e2528a3c465b27231ca4959ce Mon Sep 17 00:00:00 2001 From: Alek Ratzloff Date: Sun, 6 Oct 2024 20:26:14 -0700 Subject: [PATCH] Add slightly more readable error messages in the compiler Previously, the CompileError error messages were just `Debug::fmt` written to stdout and there wasn't really a backtrace in the code included. Now, when there is an error in an imported file, it will display a backtrace of the files included that caused this error. These are not perfect error messages and are a bit rough around the edges but they are good enough for now. Signed-off-by: Alek Ratzloff --- src/compiler.rs | 73 +++++++++++++++++++++++++++++++++++++------------ src/main.rs | 65 ++++++++++++++++++++++++++----------------- 2 files changed, 95 insertions(+), 43 deletions(-) diff --git a/src/compiler.rs b/src/compiler.rs index 2d3542d..fefcc6d 100644 --- a/src/compiler.rs +++ b/src/compiler.rs @@ -70,17 +70,44 @@ impl Scope { //////////////////////////////////////////////////////////////////////////////// #[derive(Error, Debug)] -pub struct CompileError { - pub line: Option, - pub message: String, +pub enum CompileError { + Error { + line: Option, + source_path: String, + message: String, + }, + Import { + error: Box, + line: LineRange, + source_path: String, + dest_path: String, + }, } impl Display for CompileError { fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { - if let Some(line) = &self.line { - write!(fmt, "line {:?}: {}", line, self.message) - } else { - write!(fmt, "{}", self.message) + match self { + CompileError::Error { + line: Some((line, _)), + source_path, + message, + } => write!(fmt, "{source_path}: line {line}: {message}"), + CompileError::Error { + line: None, + source_path, + message, + } => write!(fmt, "{source_path}: {}", message), + CompileError::Import { + error, + line: (line, _), + source_path, + dest_path, + } => { + write!( + fmt, + "error in {dest_path} (included from {source_path} on line {line}:\n\t{error}", + ) + } } } } @@ -138,8 +165,9 @@ impl<'c> Compiler<'c> { pub fn compile_path(self, path: impl AsRef) -> Result> { let path_str = &path.as_ref().as_os_str().to_str().unwrap(); - let mut file = File::open(path.as_ref()).map_err(|e| CompileError { + let mut file = File::open(path.as_ref()).map_err(|e| CompileError::Error { line: None, + source_path: self.path.display().to_string(), message: format!("could not open {}: {}", path.as_ref().display(), e), })?; let mut contents = String::new(); @@ -149,8 +177,9 @@ impl<'c> Compiler<'c> { let ast = parser.parse_all()?; if parser.was_error() { - return Err(CompileError { + return Err(CompileError::Error { line: None, + source_path: self.path.display().to_string(), message: format!("error in '{}'", path.as_ref().display()), } .into()); @@ -217,8 +246,9 @@ impl<'c> Compiler<'c> { let index = self.constants.len(); if index > (ConstantId::MAX as usize) { - return Err(CompileError { + return Err(CompileError::Error { line: None, + source_path: self.path.display().to_string(), message: format!("too many constants (maximum {})", ConstantId::MAX), } .into()); @@ -243,8 +273,9 @@ impl<'c> Compiler<'c> { let index = self.globals.len(); if index > (GlobalId::MAX as usize) { - return Err(CompileError { + return Err(CompileError::Error { line: None, + source_path: self.path.display().to_string(), message: format!("too many globals (maximum {})", GlobalId::MAX), } .into()); @@ -302,8 +333,9 @@ impl<'c> Compiler<'c> { fn insert_local(&mut self, name: String) -> Result<&Local> { let index = self.chunk().locals.len(); if index > (LocalIndex::MAX as usize) { - return Err(CompileError { + return Err(CompileError::Error { line: None, + source_path: self.path.display().to_string(), message: format!("too many locals (maximum: {})", LocalIndex::MAX), } .into()); @@ -327,8 +359,9 @@ impl<'c> Compiler<'c> { // get the last allocated slot and increment by one let last = &scope.scope.last().unwrap(); if last.slot == LocalSlot::MAX { - return Err(CompileError { + return Err(CompileError::Error { line: None, + source_path: self.path.display().to_string(), message: format!( "too many stack slots used by locals(maximum: {})", LocalSlot::MAX @@ -444,9 +477,11 @@ impl StmtVisitor for Compiler<'_> { // also into the modules cache let module = Compiler::new(path.clone(), self.constants, self.imported) .compile_path(&path) - .map_err(|e| CompileError { - line: Some(line), - message: format!("while importing module '{}': {}", stmt.module.text, e), + .map_err(|error| CompileError::Import { + error, + line, + source_path: self.path.display().to_string(), + dest_path: path_str.to_string(), })?; self.imported.insert(path_str.to_string(), module.clone()); module @@ -684,8 +719,9 @@ impl ExprVisitor for Compiler<'_> { self.compile_expr(arg)?; } if expr.args.len() > (Argc::MAX as usize) { - return Err(CompileError { + return Err(CompileError::Error { line: Some(expr_line_number(expr)), + source_path: self.path.display().to_string(), message: format!("too many function arguments (maximum: {})", Argc::MAX), } .into()); @@ -718,8 +754,9 @@ impl ExprVisitor for Compiler<'_> { if let Some(local) = self.get_local(name) { self.emit(expr_line_number(expr), Op::GetLocal(local.index)); } else { - let global = self.get_global(name).ok_or_else(|| CompileError { + let global = self.get_global(name).ok_or_else(|| CompileError::Error { line: Some(expr_line_number(expr)), + source_path: self.path.display().to_string(), message: if self.is_global_scope() { format!("unknown global {}", name) } else { diff --git a/src/main.rs b/src/main.rs index 50ea670..552f1ad 100644 --- a/src/main.rs +++ b/src/main.rs @@ -22,37 +22,52 @@ struct Args { path: PathBuf, } -fn main() -> Result<(), Box> { - let args = Args::parse(); +fn main() { + // While main() is allowed to return an error, it will always do `Debug::fmt` on the output wrather + // than `Display::fmt`. We want to display pretty errors, and the easiest way to do this (in my + // experience) is to just wrap it and output the error. + fn main_wrapper() -> Result<(), Box> { + let args = Args::parse(); - // initialize type system - obj::ty::init_types(); - crate::builtins::init_global_builtins(); + // initialize type system + obj::ty::init_types(); + crate::builtins::init_global_builtins(); - let absolute_path = std::path::absolute(&args.path).unwrap(); + let absolute_path = std::path::absolute(&args.path).unwrap(); - let mut constants = Default::default(); - let mut imported = Default::default(); + let mut constants = Default::default(); + let mut imported = Default::default(); - let module = compiler::Compiler::new(absolute_path, &mut constants, &mut imported) - .compile_path(&args.path)?; + let module = compiler::Compiler::new(absolute_path, &mut constants, &mut imported) + .compile_path(&args.path)?; - if args.disassemble { - disassemble::disassemble( - module.borrow().chunk(), - module.borrow().globals(), - &constants, - ); - return Ok(()); + if args.disassemble { + disassemble::disassemble( + module.borrow().chunk(), + module.borrow().globals(), + &constants, + ); + return Ok(()); + } + + // run + let module = upcast_obj(module); + let mut vm = vm::Vm::new(&constants); + // VM needs the module to be on top of the stack when it executes + vm.push(module.clone()); + vm.enter_module(module); + vm.run(); + + Ok(()) } - // run - let module = upcast_obj(module); - let mut vm = vm::Vm::new(&constants); - // VM needs the module to be on top of the stack when it executes - vm.push(module.clone()); - vm.enter_module(module); - vm.run(); + let result = main_wrapper(); - Ok(()) + match result { + Ok(()) => { /* return 0 OK */ } + Err(e) => { + println!("{}", e); + std::process::exit(1) + } + } }