diff --git a/Cargo.lock b/Cargo.lock index e3476b71..a871df55 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "ahash" @@ -311,9 +311,9 @@ checksum = "5b40af805b3121feab8a3c29f04d8ad262fa8e0561883e7653e024ae4479e6de" [[package]] name = "proc-macro2" -version = "1.0.86" +version = "1.0.97" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5e719e8df665df0d1c8fbfd238015744736151d4445ec0836b8e628aae103b77" +checksum = "d61789d7719defeb74ea5fe81f2fdfdbd28a803847077cecce2ff14e1472f6f1" dependencies = [ "unicode-ident", ] @@ -442,9 +442,9 @@ checksum = "7da8b5736845d9f2fcb837ea5d9e2628564b3b043a70948a3f0b778838c5fb4f" [[package]] name = "syn" -version = "2.0.68" +version = "2.0.105" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "901fa70d88b9d6c98022e23b4136f9f3e54e4662c3bc1bd1d84a42a9a0f0c1e9" +checksum = "7bc3fcb250e53458e712715cf74285c1f889686520d79294a9ef3bd7aa1fc619" dependencies = [ "proc-macro2", "quote", @@ -464,18 +464,18 @@ dependencies = [ [[package]] name = "thiserror" -version = "1.0.61" +version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c546c80d6be4bc6a00c0f01730c08df82eaa7a7a61f11d656526506112cc1709" +checksum = "b6aaf5339b578ea85b50e080feb250a3e8ae8cfcdff9a461c9ec2904bc923f52" dependencies = [ "thiserror-impl", ] [[package]] name = "thiserror-impl" -version = "1.0.61" +version = "1.0.69" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "46c3384250002a6d5af4d114f2845d37b57521033f30d5c3f46c4d70e1197533" +checksum = "4fee6c4efc90059e10f81e6d42c60a18f76588c3d74cb83a0b242a2b6c7504c1" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 06712a6e..3dfccc0c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,12 +18,12 @@ repository = "https://github.com/kyren/piccolo" [workspace.dependencies] ahash = "0.8" allocator-api2 = "0.2" +thiserror = "1" anyhow = "1.0" gc-arena = { git = "https://github.com/kyren/gc-arena", rev = "5a7534b883b703f23cfb8c3cfdf033460aa77ea9", features = ["allocator-api2", "hashbrown"] } hashbrown = { version = "0.14", features = ["raw"] } rand = { version = "0.8", features = ["small_rng"] } serde = "1.0" -thiserror = "1.0" piccolo = { path = "./", version = "0.3.3" } diff --git a/examples/interpreter.rs b/examples/interpreter.rs index 91f18592..c1c1be92 100644 --- a/examples/interpreter.rs +++ b/examples/interpreter.rs @@ -5,16 +5,19 @@ use clap::{crate_description, crate_name, crate_version, Arg, ArgAction, Command use rustyline::DefaultEditor; use piccolo::{ - compiler::{ParseError, ParseErrorKind}, io, meta_ops, Callback, CallbackReturn, Closure, Executor, ExternError, Function, Lua, - StashedExecutor, + RuntimeError, StashedExecutor, }; fn run_code(lua: &mut Lua, executor: &StashedExecutor, code: &str) -> Result<(), ExternError> { + // Compile and set up the function lua.try_enter(|ctx| { let closure = match Closure::load(ctx, None, ("return ".to_owned() + code).as_bytes()) { - Ok(closure) => closure, - Err(_) => Closure::load(ctx, None, code.as_bytes())?, + Ok(c) => c, + Err(_) => match Closure::load(ctx, None, code.as_bytes()) { + Ok(c) => c, + Err(e) => return Err(e.into()), + }, }; let function = Function::compose( &ctx, @@ -37,7 +40,16 @@ fn run_code(lua: &mut Lua, executor: &StashedExecutor, code: &str) -> Result<(), Ok(()) })?; - lua.execute::<()>(executor) + lua.finish(executor).map_err(RuntimeError::new)?; + + lua.try_enter(|ctx| match ctx.fetch(executor).take_result::<()>(ctx) { + Ok(Ok(())) => Ok(()), + Ok(Err(err)) => { + eprintln!("{}", err.into_error_with_stack_trace(ctx.fetch(executor))); + Ok(()) + } + Err(e) => Err(RuntimeError::new(e).into()), + }) } fn run_repl(lua: &mut Lua) -> Result<(), Box> { @@ -45,7 +57,7 @@ fn run_repl(lua: &mut Lua) -> Result<(), Box> { let executor = lua.enter(|ctx| ctx.stash(Executor::new(ctx))); loop { - let mut prompt = "> "; + let prompt = "> "; let mut line = String::new(); loop { @@ -60,18 +72,6 @@ fn run_repl(lua: &mut Lua) -> Result<(), Box> { } match run_code(lua, &executor, &line) { - Err(err) - if !read_empty - && matches!( - err.root_cause().downcast_ref::(), - Some(ParseError { - kind: ParseErrorKind::EndOfStream { .. }, - .. - }) - ) => - { - prompt = ">> "; - } Err(e) => { editor.add_history_entry(line)?; eprintln!("{}", e); diff --git a/src/closure.rs b/src/closure.rs index 31325544..59fc0002 100644 --- a/src/closure.rs +++ b/src/closure.rs @@ -2,6 +2,7 @@ use std::hash::{Hash, Hasher}; use allocator_api2::{boxed, vec, SliceExt}; use gc_arena::{allocator_api::MetricsAlloc, lock::Lock, Collect, Gc, Mutation}; +use std::{error::Error as StdError, fmt}; use thiserror::Error; use crate::{ @@ -12,17 +13,44 @@ use crate::{ Constant, Context, String, Table, Value, }; -// Note: These errors must not have #[error(transparent)] so that -// anyhow::Error::root_cause and downcasting work as expected by the -// interpreter. (Even though that gives slightly cleaner error messages). -#[derive(Debug, Error)] +// Note: We format compiler errors like Lua: ":: " +#[derive(Debug, Clone)] pub enum CompilerError { - #[error("parse error")] - Parsing(#[from] compiler::ParseError), - #[error("compile error")] - Compilation(#[from] compiler::CompileError), + Parsing { + chunk: std::string::String, + line: LineNumber, + message: std::string::String, + }, + Compilation { + chunk: std::string::String, + line: LineNumber, + message: std::string::String, + }, } +impl fmt::Display for CompilerError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + CompilerError::Parsing { + chunk, + line, + message, + } => { + write!(f, "{}:{}: {}", chunk, line, message) + } + CompilerError::Compilation { + chunk, + line, + message, + } => { + write!(f, "{}:{}: {}", chunk, line, message) + } + } + } +} + +impl StdError for CompilerError {} + /// A compiled Lua function. /// /// In Lua jargon, a "prototype" is only executable code, it has none of its "upvalues" set and @@ -130,8 +158,26 @@ impl<'gc> FunctionPrototype<'gc> { let interner = Interner(ctx); - let chunk = compiler::parse_chunk(source, interner)?; - let compiled_function = compiler::compile_chunk(&chunk, interner)?; + let chunk = match compiler::parse_chunk(source, interner) { + Ok(c) => c, + Err(e) => { + return Err(CompilerError::Parsing { + chunk: source_name.to_string(), + line: e.line_number, + message: e.to_string(), + }) + } + }; + let compiled_function = match compiler::compile_chunk(&chunk, interner) { + Ok(c) => c, + Err(e) => { + return Err(CompilerError::Compilation { + chunk: source_name.to_string(), + line: e.line_number, + message: e.kind.to_string(), + }) + } + }; Ok(FunctionPrototype::from_compiled( &ctx, diff --git a/src/compiler/lexer.rs b/src/compiler/lexer.rs index f016b9f8..f310459d 100644 --- a/src/compiler/lexer.rs +++ b/src/compiler/lexer.rs @@ -211,24 +211,12 @@ impl> fmt::Debug for Token { } } -fn print_char(c: u8) -> char { - char::from_u32(c as u32).unwrap_or(char::REPLACEMENT_CHARACTER) -} - #[derive(Debug, Error)] pub enum LexError { - #[error("short string not finished, expected matching {}", print_char(*.0))] + #[error("unfinished string")] UnfinishedShortString(u8), - #[error("unexpected character: {}", print_char(*.0))] - UnexpectedCharacter(u8), #[error("hexadecimal digit expected")] HexDigitExpected, - #[error("missing '{{' in \\u{{xxxx}} escape")] - EscapeUnicodeStart, - #[error("missing '}}' in \\u{{xxxx}} escape")] - EscapeUnicodeEnd, - #[error("invalid unicode value in \\u{{xxxx}} escape")] - EscapeUnicodeInvalid, #[error("\\ddd escape out of 0-255 range")] EscapeDecimalTooLarge, #[error("invalid escape sequence")] @@ -480,7 +468,7 @@ where Token::Name(self.take_string()) } } else { - return Err(LexError::UnexpectedCharacter(c)); + return Err(LexError::InvalidEscape); } } })) @@ -623,28 +611,33 @@ where b'u' => { if self.peek(1)? != Some(b'{') { - return Err(LexError::EscapeUnicodeStart); + return Err(LexError::InvalidEscape); } self.advance(2); let mut u: u32 = 0; + let mut saw_digit = false; loop { if let Some(c) = self.peek(0)? { if c == b'}' { + if !saw_digit { + return Err(LexError::HexDigitExpected); + } self.advance(1); break; } else if let Some(h) = from_hex_digit(c) { + saw_digit = true; u = (u << 4) | h as u32; self.advance(1); } else { - return Err(LexError::EscapeUnicodeEnd); + return Err(LexError::HexDigitExpected); } } else { - return Err(LexError::EscapeUnicodeEnd); + return Err(LexError::UnfinishedShortString(start_quote)); } } - let c = char::from_u32(u).ok_or(LexError::EscapeUnicodeInvalid)?; + let c = char::from_u32(u).ok_or(LexError::InvalidEscape)?; let mut buf = [0; 4]; for &b in c.encode_utf8(&mut buf).as_bytes() { self.string_buffer.push(b); diff --git a/src/compiler/parser.rs b/src/compiler/parser.rs index 7ced0c28..bfd0cb81 100644 --- a/src/compiler/parser.rs +++ b/src/compiler/parser.rs @@ -1,6 +1,7 @@ +use std::error::Error as StdError; use std::{fmt, ops, rc::Rc}; -use thiserror::Error; +// use thiserror::Error; // no longer deriving Error here use super::{ lexer::{LexError, Lexer, LineNumber, Token}, @@ -315,37 +316,82 @@ pub enum RecordKey { Indexed(Expression), } -#[derive(Debug, Error)] +#[derive(Debug)] pub enum ParseErrorKind { - #[error("found {unexpected:?}, expected {expected:?}")] Unexpected { unexpected: String, expected: String, }, - #[error( - "unexpected end of token stream{}", - .expected.as_ref().map(|e| format!(", expected {e}")).unwrap_or_default() - )] - EndOfStream { expected: Option }, - #[error("cannot assign to expression")] + EndOfStream { + expected: Option, + }, AssignToExpression, - #[error("expression is not a statement")] ExpressionNotStatement, - #[error("recursion limit reached")] RecursionLimit, - #[error("lexer error")] - LexError(#[from] LexError), - #[error("invalid attribute {0:?}")] + LexError(LexError), InvalidAttribute(String), + ExpectedToClose { + what: String, + who: String, + where_line: LineNumber, + }, + UnexpectedSymbol, } -#[derive(Debug, Error)] -#[error("parse error at line {line_number}: {kind}")] +#[derive(Debug)] pub struct ParseError { pub kind: ParseErrorKind, pub line_number: LineNumber, + pub near: Option, +} + +impl fmt::Display for ParseErrorKind { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + ParseErrorKind::Unexpected { + unexpected: _, + expected, + } => { + write!(f, "{} expected", expected) + } + ParseErrorKind::EndOfStream { + expected: Some(expected), + } => { + write!(f, "{} expected", expected) + } + ParseErrorKind::EndOfStream { expected: None } => write!(f, "syntax error"), + ParseErrorKind::AssignToExpression => write!(f, "syntax error"), + ParseErrorKind::ExpressionNotStatement => write!(f, "unexpected symbol"), + ParseErrorKind::RecursionLimit => write!(f, "control structure too long"), + ParseErrorKind::LexError(e) => write!(f, "{}", e), + ParseErrorKind::InvalidAttribute(attr) => write!(f, "invalid attribute '{}'", attr), + ParseErrorKind::ExpectedToClose { + what, + who, + where_line, + } => { + write!( + f, + "{} expected (to close {} at line {})", + what, who, where_line + ) + } + ParseErrorKind::UnexpectedSymbol => write!(f, "unexpected symbol"), + } + } +} + +impl fmt::Display for ParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match &self.near { + Some(near) => write!(f, "{} near {}", self.kind, near), + None => write!(f, "{}", self.kind), + } + } } +impl StdError for ParseError {} + pub fn parse_chunk(source: &[u8], interner: S) -> Result, ParseError> where S: StringInterner, @@ -365,13 +411,53 @@ struct Parser<'a, S: StringInterner> { } impl Parser<'_, S> { + fn expect_close_at( + &mut self, + token: Token, + who: &str, + where_line: LineNumber, + ) -> Result { + self.read_ahead(1)?; + if self.read_buffer.is_empty() { + let near = Some("".to_owned()); + Err(build_parse_error( + ParseErrorKind::ExpectedToClose { + what: expected_token_str(&token), + who: who.to_owned(), + where_line, + }, + self.lexer.line_number(), + near, + )) + } else { + let next_token = self.read_buffer.remove(0); + if *next_token == token { + Ok(next_token.line_number) + } else { + Err(build_parse_error( + ParseErrorKind::ExpectedToClose { + what: expected_token_str(&token), + who: who.to_owned(), + where_line, + }, + next_token.line_number, + Some(token_to_text(&next_token.inner)), + )) + } + } + } + fn parse_chunk(&mut self) -> Result, ParseError> { let block = self.parse_block()?; if !self.look_ahead(0)?.is_none() { - Err(ParseError { - kind: ParseErrorKind::EndOfStream { expected: None }, - line_number: self.lexer.line_number(), - }) + Err(build_parse_error( + ParseErrorKind::EndOfStream { expected: None }, + self.lexer.line_number(), + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )) } else { Ok(Chunk { block }) } @@ -473,7 +559,7 @@ impl Parser<'_, S> { } fn parse_if_statement(&mut self) -> Result, ParseError> { - self.expect_next(Token::If)?; + let if_line = self.expect_next(Token::If)?; let if_cond = self.parse_expression()?; self.expect_next(Token::Then)?; let if_block = self.parse_block()?; @@ -494,7 +580,7 @@ impl Parser<'_, S> { None }; - self.expect_next(Token::End)?; + self.expect_close_at(Token::End, "'if'", if_line)?; Ok(IfStatement { if_part: (if_cond, if_block), @@ -506,15 +592,15 @@ impl Parser<'_, S> { fn parse_while_statement(&mut self) -> Result, ParseError> { self.expect_next(Token::While)?; let condition = self.parse_expression()?; - self.expect_next(Token::Do)?; + let do_line = self.expect_next(Token::Do)?; let block = self.parse_block()?; - self.expect_next(Token::End)?; + self.expect_close_at(Token::End, "'do'", do_line)?; Ok(WhileStatement { condition, block }) } fn parse_for_statement(&mut self) -> Result, ParseError> { - self.expect_next(Token::For)?; + let for_line = self.expect_next(Token::For)?; let name = self.expect_name()?.inner; let next = self.get_next()?; @@ -533,7 +619,7 @@ impl Parser<'_, S> { self.expect_next(Token::Do)?; let body = self.parse_block()?; - self.expect_next(Token::End)?; + self.expect_close_at(Token::End, "'for'", for_line)?; Ok(ForStatement::Numeric { name, @@ -556,7 +642,7 @@ impl Parser<'_, S> { self.expect_next(Token::Do)?; let body = self.parse_block()?; - self.expect_next(Token::End)?; + self.expect_close_at(Token::End, "'for'", for_line)?; Ok(ForStatement::Generic { names, @@ -565,26 +651,27 @@ impl Parser<'_, S> { }) } - token => Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), + token => Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(token), expected: "'=' or 'in'".to_owned(), }, - line_number: next.line_number, - }), + next.line_number, + Some(token_to_text(token)), + )), } } fn parse_repeat_statement(&mut self) -> Result, ParseError> { - self.expect_next(Token::Repeat)?; + let repeat_line = self.expect_next(Token::Repeat)?; let body = self.parse_block()?; - self.expect_next(Token::Until)?; + self.expect_close_at(Token::Until, "'repeat'", repeat_line)?; let until = self.parse_expression()?; Ok(RepeatStatement { body, until }) } fn parse_function_statement(&mut self) -> Result, ParseError> { - self.expect_next(Token::Function)?; + let function_line = self.expect_next(Token::Function)?; let name = self.expect_name()?.inner; let mut fields = Vec::new(); @@ -604,7 +691,7 @@ impl Parser<'_, S> { } } - let definition = self.parse_function_definition()?; + let definition = self.parse_function_definition_at(Some(function_line))?; Ok(FunctionStatement { name, @@ -617,10 +704,10 @@ impl Parser<'_, S> { fn parse_local_function_statement( &mut self, ) -> Result, ParseError> { - self.expect_next(Token::Function)?; + let function_line = self.expect_next(Token::Function)?; let name = self.expect_name()?.inner; - let definition = self.parse_function_definition()?; + let definition = self.parse_function_definition_at(Some(function_line))?; Ok(LocalFunctionStatement { name, definition }) } @@ -641,12 +728,13 @@ impl Parser<'_, S> { b"const" => LocalAttributes::CONST, b"close" => LocalAttributes::CONST_CLOSE, _ => { - return Err(ParseError { - kind: ParseErrorKind::InvalidAttribute( + return Err(build_parse_error( + ParseErrorKind::InvalidAttribute( String::from_utf8_lossy(attr.as_ref()).into_owned(), ), line_number, - }) + None, + )) } } } else { @@ -699,20 +787,28 @@ impl Parser<'_, S> { AssignmentTarget::Field(suffixed_expression, field_suffix) } SuffixPart::Call(_) => { - return Err(ParseError { - kind: ParseErrorKind::AssignToExpression, + return Err(build_parse_error( + ParseErrorKind::AssignToExpression, line_number, - }); + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )); } } } else { match suffixed_expression.primary { PrimaryExpression::Name(name) => AssignmentTarget::Name(name), _ => { - return Err(ParseError { - kind: ParseErrorKind::AssignToExpression, + return Err(build_parse_error( + ParseErrorKind::AssignToExpression, line_number, - }) + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )) } } }; @@ -741,16 +837,24 @@ impl Parser<'_, S> { call: call_suffix, })) } - SuffixPart::Field(_) => Err(ParseError { - kind: ParseErrorKind::ExpressionNotStatement, + SuffixPart::Field(_) => Err(build_parse_error( + ParseErrorKind::ExpressionNotStatement, line_number, - }), + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )), } } else { - Err(ParseError { - kind: ParseErrorKind::ExpressionNotStatement, + Err(build_parse_error( + ParseErrorKind::ExpressionNotStatement, line_number, - }) + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )) } } @@ -840,23 +944,24 @@ impl Parser<'_, S> { match next.inner { Token::LeftParen => { let expr = self.parse_expression()?; - self.expect_next(Token::RightParen)?; + self.expect_close_at(Token::RightParen, "'('", next.line_number)?; Ok(PrimaryExpression::GroupedExpression(expr)) } Token::Name(n) => Ok(PrimaryExpression::Name(n)), - token => Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), + token => Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), expected: "grouped expression or name".to_owned(), }, - line_number: next.line_number, - }), + next.line_number, + Some(token_to_text(&token)), + )), } } fn parse_field_suffix(&mut self) -> Result, ParseError> { - let next = self.get_next()?; - match &next.inner { + let next_la = self.get_next()?.clone(); + match &next_la.inner { Token::Dot => { self.take_next()?; Ok(FieldSuffix::Named(self.expect_name()?.inner)) @@ -864,16 +969,17 @@ impl Parser<'_, S> { Token::LeftBracket => { self.take_next()?; let expr = self.parse_expression()?; - self.expect_next(Token::RightBracket)?; + self.expect_close_at(Token::RightBracket, "'['", next_la.line_number)?; Ok(FieldSuffix::Indexed(expr)) } - token => Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), + token => Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), expected: "field or suffix".to_owned(), }, - line_number: next.line_number, - }), + next_la.line_number, + Some(token_to_text(&token)), + )), } } @@ -911,13 +1017,14 @@ impl Parser<'_, S> { tail: vec![], }], token => { - return Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), + return Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), expected: "function arguments".to_owned(), }, - line_number: next.line_number, - }); + next.line_number, + Some(token_to_text(&token)), + )); } }; @@ -935,13 +1042,14 @@ impl Parser<'_, S> { Token::Colon | Token::LeftParen | Token::LeftBrace | Token::String(_) => { Ok(SuffixPart::Call(self.parse_call_suffix()?)) } - token => Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), + token => Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), expected: "expression suffix".to_owned(), }, - line_number: next.line_number, - }), + next.line_number, + Some(token_to_text(&token)), + )), } } @@ -966,7 +1074,7 @@ impl Parser<'_, S> { } fn parse_function_definition(&mut self) -> Result, ParseError> { - self.expect_next(Token::LeftParen)?; + let lp_line = self.expect_next(Token::LeftParen)?; let mut parameters = Vec::new(); let mut has_varargs = false; @@ -980,13 +1088,14 @@ impl Parser<'_, S> { break; } token => { - return Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), + return Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), expected: "parameter name or '...'".to_owned(), }, - line_number: next.line_number, - }); + next.line_number, + Some(token_to_text(&token)), + )); } } if self.check_ahead(0, Token::Comma)? { @@ -996,7 +1105,7 @@ impl Parser<'_, S> { } } } - self.expect_next(Token::RightParen)?; + self.expect_close_at(Token::RightParen, "'('", lp_line)?; let body = self.parse_block()?; self.expect_next(Token::End)?; @@ -1008,8 +1117,59 @@ impl Parser<'_, S> { }) } + fn parse_function_definition_at( + &mut self, + start_line: Option, + ) -> Result, ParseError> { + let lp_line = self.expect_next(Token::LeftParen)?; + + let mut parameters = Vec::new(); + let mut has_varargs = false; + if !self.check_ahead(0, Token::RightParen)? { + loop { + let next = self.take_next()?; + match next.inner { + Token::Name(name) => parameters.push(name), + Token::Dots => { + has_varargs = true; + break; + } + token => { + return Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), + expected: "parameter name or '...'".to_owned(), + }, + next.line_number, + Some(token_to_text(&token)), + )); + } + } + if self.check_ahead(0, Token::Comma)? { + self.take_next()?; + } else { + break; + } + } + } + self.expect_close_at(Token::RightParen, "'('", lp_line)?; + + let body = self.parse_block()?; + if let Some(line) = start_line { + self.expect_close_at(Token::End, "'function'", line)?; + } else { + self.expect_next(Token::End)?; + } + + Ok(FunctionDefinition { + parameters, + has_varargs, + body, + }) + } + fn parse_table_constructor(&mut self) -> Result, ParseError> { - self.expect_next(Token::LeftBrace)?; + let lb_line = self.expect_next(Token::LeftBrace)?; let mut fields = Vec::new(); loop { if self.check_ahead(0, Token::RightBrace)? { @@ -1023,7 +1183,7 @@ impl Parser<'_, S> { _ => break, } } - self.expect_next(Token::RightBrace)?; + self.expect_close_at(Token::RightBrace, "'{'", lb_line)?; Ok(TableConstructor { fields }) } @@ -1053,14 +1213,18 @@ impl Parser<'_, S> { // Error if we have more than MAX_RECURSION guards live, otherwise return a new recursion guard // (a recursion guard is just an Rc used solely for its live count). - fn recursion_guard(&self) -> Result, ParseError> { + fn recursion_guard(&mut self) -> Result, ParseError> { if Rc::strong_count(&self.recursion_guard) < MAX_RECURSION { Ok(self.recursion_guard.clone()) } else { - Err(ParseError { - kind: ParseErrorKind::RecursionLimit, - line_number: self.lexer.line_number(), - }) + Err(build_parse_error( + ParseErrorKind::RecursionLimit, + self.lexer.line_number(), + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )) } } @@ -1070,10 +1234,11 @@ impl Parser<'_, S> { if let Some(token) = self.read_buffer.get(0) { Ok(token) } else { - Err(ParseError { - kind: ParseErrorKind::EndOfStream { expected: None }, - line_number: self.lexer.line_number(), - }) + Err(build_parse_error( + ParseErrorKind::EndOfStream { expected: None }, + self.lexer.line_number(), + Some("".to_owned()), + )) } } @@ -1081,24 +1246,31 @@ impl Parser<'_, S> { fn expect_next(&mut self, token: Token) -> Result { self.read_ahead(1)?; if self.read_buffer.is_empty() { - Err(ParseError { - kind: ParseErrorKind::EndOfStream { - expected: Some(format!("{:?}", token)), + let near = self + .read_buffer + .get(0) + .map(|t| token_to_text(&t.inner)) + .or(Some("".to_owned())); + Err(build_parse_error( + ParseErrorKind::EndOfStream { + expected: Some(expected_token_str(&token)), }, - line_number: self.lexer.line_number(), - }) + self.lexer.line_number(), + near, + )) } else { let next_token = self.read_buffer.remove(0); if *next_token == token { Ok(next_token.line_number) } else { - Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", next_token.inner), - expected: format!("{:?}", token), + Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&next_token.inner), + expected: expected_token_str(&token), }, - line_number: next_token.line_number, - }) + next_token.line_number, + Some(token_to_text(&next_token.inner)), + )) } } } @@ -1107,22 +1279,27 @@ impl Parser<'_, S> { fn expect_name(&mut self) -> Result, ParseError> { self.read_ahead(1)?; if self.read_buffer.is_empty() { - Err(ParseError { - kind: ParseErrorKind::EndOfStream { - expected: Some("name".to_owned()), + Err(build_parse_error( + ParseErrorKind::EndOfStream { + expected: Some("".to_owned()), }, - line_number: self.lexer.line_number(), - }) + self.lexer.line_number(), + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )) } else { self.read_buffer.remove(0).try_map(|t| match t { Token::Name(name) => Ok(name), - token => Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), - expected: "name".to_owned(), + token => Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), + expected: "".to_owned(), }, - line_number: self.lexer.line_number(), - }), + self.lexer.line_number(), + Some(token_to_text(&token)), + )), }) } } @@ -1131,22 +1308,27 @@ impl Parser<'_, S> { fn expect_string(&mut self) -> Result, ParseError> { self.read_ahead(1)?; if self.read_buffer.is_empty() { - Err(ParseError { - kind: ParseErrorKind::EndOfStream { - expected: Some("string".to_owned()), + Err(build_parse_error( + ParseErrorKind::EndOfStream { + expected: Some("".to_owned()), }, - line_number: self.lexer.line_number(), - }) + self.lexer.line_number(), + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + )) } else { self.read_buffer.remove(0).try_map(|t| match t { Token::String(string) => Ok(string), - token => Err(ParseError { - kind: ParseErrorKind::Unexpected { - unexpected: format!("{:?}", token), - expected: "string".to_owned(), + token => Err(build_parse_error( + ParseErrorKind::Unexpected { + unexpected: token_to_text(&token), + expected: "".to_owned(), }, - line_number: self.lexer.line_number(), - }), + self.lexer.line_number(), + Some(token_to_text(&token)), + )), }) } } @@ -1155,10 +1337,11 @@ impl Parser<'_, S> { fn take_next(&mut self) -> Result>, ParseError> { self.read_ahead(1)?; if self.read_buffer.is_empty() { - Err(ParseError { - kind: ParseErrorKind::EndOfStream { expected: None }, - line_number: self.lexer.line_number(), - }) + Err(build_parse_error( + ParseErrorKind::EndOfStream { expected: None }, + self.lexer.line_number(), + Some("".to_owned()), + )) } else { Ok(self.read_buffer.remove(0)) } @@ -1188,14 +1371,26 @@ impl Parser<'_, S> { // possible). fn read_ahead(&mut self, n: usize) -> Result<(), ParseError> { while self.read_buffer.len() <= n { - self.lexer.skip_whitespace().map_err(|e| ParseError { - kind: ParseErrorKind::LexError(e), - line_number: self.lexer.line_number(), + self.lexer.skip_whitespace().map_err(|e| { + build_parse_error( + ParseErrorKind::LexError(e), + self.lexer.line_number(), + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + ) })?; let line_number = self.lexer.line_number(); - if let Some(token) = self.lexer.read_token().map_err(|e| ParseError { - kind: ParseErrorKind::LexError(e), - line_number: self.lexer.line_number(), + if let Some(token) = self.lexer.read_token().map_err(|e| { + build_parse_error( + ParseErrorKind::LexError(e), + self.lexer.line_number(), + self.look_ahead(0) + .ok() + .flatten() + .map(|t| token_to_text(&t.inner)), + ) })? { self.read_buffer .push(LineAnnotated::new(line_number, token)); @@ -1207,6 +1402,95 @@ impl Parser<'_, S> { } } +// Format a token similarly to Lua's luaX_token2str (for "expected" messages): +// reserved words and symbols are quoted (e.g., "'='"), names/strings show as /. +fn expected_token_str>(token: &Token) -> String { + match token { + Token::Name(_) => "".to_owned(), + Token::String(_) => "".to_owned(), + Token::Integer(_) | Token::Float(_) => "".to_owned(), + Token::Break => "'break'".to_owned(), + Token::Do => "'do'".to_owned(), + Token::Else => "'else'".to_owned(), + Token::ElseIf => "'elseif'".to_owned(), + Token::End => "'end'".to_owned(), + Token::Function => "'function'".to_owned(), + Token::Goto => "'goto'".to_owned(), + Token::If => "'if'".to_owned(), + Token::In => "'in'".to_owned(), + Token::Local => "'local'".to_owned(), + Token::Nil => "'nil'".to_owned(), + Token::For => "'for'".to_owned(), + Token::While => "'while'".to_owned(), + Token::Repeat => "'repeat'".to_owned(), + Token::Until => "'until'".to_owned(), + Token::Return => "'return'".to_owned(), + Token::Then => "'then'".to_owned(), + Token::True => "'true'".to_owned(), + Token::False => "'false'".to_owned(), + Token::Not => "'not'".to_owned(), + Token::And => "'and'".to_owned(), + Token::Or => "'or'".to_owned(), + Token::Minus => "'-'".to_owned(), + Token::Add => "'+'".to_owned(), + Token::Mul => "'*'".to_owned(), + Token::Div => "'/'".to_owned(), + Token::IDiv => "'//'".to_owned(), + Token::Pow => "'^'".to_owned(), + Token::Mod => "'%'".to_owned(), + Token::Len => "'#'".to_owned(), + Token::BitNotXor => "'~'".to_owned(), + Token::BitAnd => "'&'".to_owned(), + Token::BitOr => "'|'".to_owned(), + Token::ShiftRight => "'>>'".to_owned(), + Token::ShiftLeft => "'<<'".to_owned(), + Token::Concat => "'..'".to_owned(), + Token::Dots => "'...'".to_owned(), + Token::Assign => "'='".to_owned(), + Token::LessThan => "'<'".to_owned(), + Token::LessEqual => "'<='".to_owned(), + Token::GreaterThan => "'>'".to_owned(), + Token::GreaterEqual => "'>='".to_owned(), + Token::Equal => "'=='".to_owned(), + Token::NotEqual => "'~='".to_owned(), + Token::Dot => "'.'".to_owned(), + Token::SemiColon => "';'".to_owned(), + Token::Colon => "':'".to_owned(), + Token::DoubleColon => "'::'".to_owned(), + Token::Comma => "','".to_owned(), + Token::LeftParen => "'('".to_owned(), + Token::RightParen => "')'".to_owned(), + Token::LeftBracket => "'['".to_owned(), + Token::RightBracket => "']'".to_owned(), + Token::LeftBrace => "'{'".to_owned(), + Token::RightBrace => "'}'".to_owned(), + } +} + +// Format a token like Lua's txtToken (for "near" messages): names/strings/numerals show actual +// text in single quotes; symbols/keywords show quoted symbol text. +fn token_to_text>(token: &Token) -> String { + match token { + Token::Name(n) => format!("'{}'", String::from_utf8_lossy(n.as_ref())), + Token::String(s) => format!("'{}'", String::from_utf8_lossy(s.as_ref())), + Token::Integer(i) => format!("'{}'", i), + Token::Float(n) => format!("'{}'", n), + _ => expected_token_str(token), + } +} + +fn build_parse_error( + kind: ParseErrorKind, + line_number: LineNumber, + near: Option, +) -> ParseError { + ParseError { + kind, + line_number, + near, + } +} + const MAX_RECURSION: usize = 200; // Priority lower than any unary or binary operator. diff --git a/src/error.rs b/src/error.rs index 462c9cd3..a7868482 100644 --- a/src/error.rs +++ b/src/error.rs @@ -4,8 +4,8 @@ use gc_arena::{Collect, Gc, Rootable}; use thiserror::Error; use crate::{ - Callback, CallbackReturn, Context, FromValue, Function, IntoValue, MetaMethod, Singleton, - Table, UserData, Value, + compiler::LineNumber, thread::Frame, Callback, CallbackReturn, Context, Executor, FromValue, + Function, IntoValue, MetaMethod, Singleton, Table, UserData, Value, }; #[derive(Debug, Clone, Copy, Error)] @@ -143,6 +143,104 @@ impl AsRef for RuntimeError { } } +/// The same as [`Error`], but with a formatted stack trace. +#[derive(Debug, Clone, Collect)] +#[collect(no_drop)] +pub struct ErrorWithStackTrace<'gc> { + original: Error<'gc>, + stack_trace: String, +} + +impl<'gc> ErrorWithStackTrace<'gc> { + /// Construct a formatted stack trace from [`Error`] and [`Executor`] + pub fn new(original: Error<'gc>, exec: Executor<'gc>) -> Self { + use crate::compiler::FunctionRef; + use std::fmt::Write; + + let mut stack_trace = format!("stack traceback:"); + + let Ok(state) = exec.into_inner().try_borrow() else { + return Self { + original, + stack_trace, + }; + }; + + let Some(thread) = state.thread_stack().last() else { + return Self { + original, + stack_trace, + }; + }; + + let thread_state = thread.into_inner().borrow(); + + for frame in thread_state.frames().iter().rev() { + if let Frame::Lua { closure, pc, .. } = frame { + let prototype = closure.prototype(); + let current_pc = pc.saturating_sub(1); + let line = match prototype + .opcode_line_numbers + .binary_search_by_key(¤t_pc, |(opi, _)| *opi) + { + Ok(index) => prototype + .opcode_line_numbers + .get(index) + .map(|(_, line)| *line) + .unwrap_or(LineNumber(0)), + Err(index) => prototype + .opcode_line_numbers + .get(index.wrapping_sub(1)) + .map(|(_, line)| *line) + .unwrap_or(LineNumber(0)), + }; + + let location = match prototype.reference { + FunctionRef::Chunk => "in main chunk".into(), + FunctionRef::Named(name, _) => { + format!("in function '{}'", name.display_lossy()) + } + FunctionRef::Expression(line) => format!( + "in function <{}:{}>", + prototype.chunk_name.display_lossy(), + line + ), + }; + + write!( + stack_trace, + "\n\t{}:{}: {}", + prototype.chunk_name.display_lossy(), + line, + location + ) + .ok(); + } else { + write!(stack_trace, "\n\t[C]: in ?").ok(); + } + } + + Self { + original, + stack_trace, + } + } + + pub fn stack_trace(&self) -> &str { + &self.stack_trace + } + + pub fn original(&self) -> &Error<'gc> { + &self.original + } +} + +impl<'gc> fmt::Display for ErrorWithStackTrace<'gc> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}\n{}", self.original, self.stack_trace) + } +} + /// An error that can be raised from Lua code. /// /// This can be either a [`LuaError`] containing a Lua [`Value`], or a [`RuntimeError`] containing a @@ -157,8 +255,8 @@ pub enum Error<'gc> { impl<'gc> fmt::Display for Error<'gc> { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - Error::Lua(err) => write!(f, "lua error: {err}"), - Error::Runtime(err) => write!(f, "runtime error: {err:#}"), + Error::Lua(err) => write!(f, "{}", err), + Error::Runtime(err) => write!(f, "{}", err.root_cause()), } } } @@ -249,6 +347,10 @@ impl<'gc> Error<'gc> { } } + pub fn into_error_with_stack_trace(self, exec: Executor<'gc>) -> ErrorWithStackTrace<'gc> { + ErrorWithStackTrace::new(self, exec) + } + pub fn to_extern(&self) -> ExternError { self.clone().into_extern() } @@ -280,8 +382,9 @@ pub enum ExternError { impl fmt::Display for ExternError { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { match self { - ExternError::Lua(err) => write!(f, "lua error: {err}"), - ExternError::Runtime(err) => write!(f, "runtime error: {err:#}"), + // Mirror internal `Error` display: no prefixes, just the message. + ExternError::Lua(err) => write!(f, "{}", err), + ExternError::Runtime(err) => write!(f, "{}", err.root_cause()), } } } diff --git a/src/lib.rs b/src/lib.rs index 0df7831e..fc004d63 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -30,7 +30,7 @@ pub use self::{ closure::{Closure, CompilerError, FunctionPrototype}, constant::Constant, conversion::{FromMultiValue, FromValue, IntoMultiValue, IntoValue, Variadic}, - error::{Error, ExternError, RuntimeError, TypeError}, + error::{Error, ErrorWithStackTrace, ExternError, RuntimeError, TypeError}, fuel::Fuel, function::Function, lua::{Context, Lua}, diff --git a/src/meta_ops.rs b/src/meta_ops.rs index 6fd0517a..cebb2eba 100644 --- a/src/meta_ops.rs +++ b/src/meta_ops.rs @@ -1,7 +1,8 @@ use std::io::Write; use gc_arena::Collect; -use thiserror::Error; +use std::error::Error as StdError; +use std::fmt; use crate::async_callback::{AsyncSequence, Locals}; use crate::{async_sequence, SequenceReturn, Stack}; @@ -143,24 +144,90 @@ impl<'gc, const N: usize> From> for MetaResult<'gc, N> { } } -#[derive(Debug, Clone, Error)] +#[derive(Debug, Clone)] pub enum MetaOperatorError { - #[error("could not call metamethod {}: {}", .0.name(), .1)] - Call(MetaMethod, #[source] MetaCallError), - #[error("could not {} a {} value", .0.verb(), .1)] + Call(MetaMethod, MetaCallError), Unary(MetaMethod, &'static str), - #[error("could not {} values of type {} and {}", .0.verb(), .1, .2)] Binary(MetaMethod, &'static str, &'static str), - #[error("invalid table key")] - IndexKeyError(#[from] InvalidTableKey), - #[error("concatenation result is too long")] + IndexKeyError(InvalidTableKey), ConcatOverflow, } -#[derive(Debug, Copy, Clone, Error)] -#[error("could not call a {} value", .0)] +impl fmt::Display for MetaOperatorError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use MetaMethod::*; + match self { + MetaOperatorError::Call(method, err) => { + write!(f, "could not call metamethod {}: {}", method.name(), err) + } + MetaOperatorError::Unary(method, ty) => { + let msg = match method { + Index | NewIndex => format!("attempt to index a {} value", ty), + Len => format!("attempt to get length of a {} value", ty), + BNot => format!("attempt to perform bitwise operation on a {} value", ty), + Unm | Add | Sub | Mul | Div | Mod | Pow | IDiv => { + format!("attempt to perform arithmetic on a {} value", ty) + } + ToString | Pairs | Eq | Call | Concat | Lt | Le | BAnd | BOr | BXor | Shl + | Shr => format!("attempt to {} a {} value", method.verb(), ty), + }; + f.write_str(&msg) + } + MetaOperatorError::Binary(method, lty, rty) => { + let msg = match method { + Concat => { + // choose the non-string side, like PUC-Lua does + let ty = if *lty == "string" { *rty } else { *lty }; + format!("attempt to concatenate a {} value", ty) + } + Lt | Le => format!("attempt to compare {} with {}", lty, rty), + BAnd | BOr | BXor | Shl | Shr => { + // choose non-integer side when possible + let ty = if *lty == "number" { *rty } else { *lty }; + format!("attempt to perform bitwise operation on a {} value", ty) + } + Add | Sub | Mul | Div | Mod | Pow | IDiv => { + let ty = if *lty == "number" { *rty } else { *lty }; + format!("attempt to perform arithmetic on a {} value", ty) + } + Eq | Index | NewIndex | Len | ToString | Pairs | Call | Unm | BNot => { + format!( + "attempt to {} values of type {} and {}", + method.verb(), + lty, + rty + ) + } + }; + f.write_str(&msg) + } + MetaOperatorError::IndexKeyError(_) => f.write_str("invalid table key"), + MetaOperatorError::ConcatOverflow => f.write_str("concatenation result is too long"), + } + } +} + +impl StdError for MetaOperatorError { + fn source(&self) -> Option<&(dyn StdError + 'static)> { + match self { + MetaOperatorError::Call(_, e) => Some(e), + MetaOperatorError::IndexKeyError(e) => Some(e), + _ => None, + } + } +} + +#[derive(Debug, Copy, Clone)] pub struct MetaCallError(&'static str); +impl fmt::Display for MetaCallError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "attempt to call a {} value", self.0) + } +} + +impl StdError for MetaCallError {} + fn get_metatable<'gc>(val: Value<'gc>) -> Option> { match val { Value::Table(t) => t.metatable(), @@ -273,7 +340,10 @@ pub fn index<'gc>( args: [table, key], }, _ => MetaCall { - function: call(ctx, idx).map_err(|e| MetaOperatorError::Call(MetaMethod::Index, e))?, + function: match call(ctx, idx) { + Ok(f) => f, + Err(e) => return Err(MetaOperatorError::Call(MetaMethod::Index, e)), + }, args: [table, key], }, })) @@ -290,7 +360,10 @@ pub fn new_index<'gc>( let v = table.get_value(ctx, key); if !v.is_nil() { // If the value is present in the table, then we do not invoke the metamethod. - table.set_raw(&ctx, key, value)?; + // set_raw returns Result<(), InvalidTableKey> + table + .set_raw(&ctx, key, value) + .map_err(MetaOperatorError::IndexKeyError)?; return Ok(None); } @@ -303,7 +376,9 @@ pub fn new_index<'gc>( if idx.is_nil() { // If we do not have a __newindex metamethod, then just set the table value // directly. - table.set_raw(&ctx, key, value)?; + table + .set_raw(&ctx, key, value) + .map_err(MetaOperatorError::IndexKeyError)?; return Ok(None); } @@ -348,8 +423,10 @@ pub fn new_index<'gc>( args: [table, key, value], }, _ => MetaCall { - function: call(ctx, idx) - .map_err(|e| MetaOperatorError::Call(MetaMethod::NewIndex, e))?, + function: match call(ctx, idx) { + Ok(f) => f, + Err(e) => return Err(MetaOperatorError::Call(MetaMethod::NewIndex, e)), + }, args: [table, key, value], }, })) diff --git a/src/stdlib/base.rs b/src/stdlib/base.rs index 5a18ca80..84934d14 100644 --- a/src/stdlib/base.rs +++ b/src/stdlib/base.rs @@ -2,11 +2,12 @@ use std::pin::Pin; use gc_arena::Collect; +use super::{argument_error, type_error}; use crate::{ meta_ops::{self, MetaResult}, table::NextValue, BoxSequence, Callback, CallbackReturn, Context, Error, Execution, IntoValue, MetaMethod, - Sequence, SequencePoll, Stack, String, Table, TypeError, Value, Variadic, + Sequence, SequencePoll, Stack, String, Table, Value, Variadic, }; pub fn load_base<'gc>(ctx: Context<'gc>) { @@ -21,8 +22,8 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { (bytes, is_neg) } - if stack.is_empty() { - Err("Missing argument(s) to tonumber".into_value(ctx))? + if stack.len() == 0 { + Err(argument_error(ctx, "tonumber", 1, "value expected"))? } else if stack.len() == 1 || stack.get(1).is_nil() { let prenumber = stack.consume::(ctx)?; stack.replace(ctx, prenumber.to_numeric().unwrap_or(Value::Nil)); @@ -31,16 +32,10 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { // Avoid implicitly converting value to a string let s = match value { Value::String(s) => s, - _ => { - return Err(TypeError { - expected: "string", - found: value.type_name(), - } - .into()) - } + _ => return Err(type_error(ctx, "tonumber", 1, "string", value.type_name())), }; if !(2..=36).contains(&base) { - Err("base out of range".into_value(ctx))?; + Err(argument_error(ctx, "tonumber", 2, "base out of range"))?; } let (bytes, is_neg) = extract_number_data(s.as_bytes()); let result = bytes @@ -72,7 +67,7 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { "tostring", Callback::from_fn(&ctx, |ctx, _, mut stack| { if stack.is_empty() { - Err("Bad argument to tostring".into_value(ctx).into()) + Err(argument_error(ctx, "tostring", 1, "value expected")) } else { match meta_ops::tostring(ctx, stack.get(0))? { MetaResult::Value(v) => { @@ -126,7 +121,7 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { "type", Callback::from_fn(&ctx, |ctx, _, mut stack| { if stack.is_empty() { - Err("Missing argument to type".into_value(ctx).into()) + Err(argument_error(ctx, "type", 1, "value expected")) } else { stack.replace(ctx, stack.get(0).type_name()); Ok(CallbackReturn::Return) @@ -158,7 +153,7 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { return Ok(CallbackReturn::Return); } - Err("Bad argument to 'select'".into_value(ctx).into()) + Err(argument_error(ctx, "select", 1, "number expected")) }), ); @@ -197,9 +192,13 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { stack.replace(ctx, t.metatable()); Ok(CallbackReturn::Return) } else { - Err("'getmetatable' can only be used on table types" - .into_value(ctx) - .into()) + Err(type_error( + ctx, + "getmetatable", + 1, + "table", + stack.get(0).type_name(), + )) } }), ); @@ -332,7 +331,7 @@ pub fn load_base<'gc>(ctx: Context<'gc>) { stack.into_back(ctx, ctx.metrics().total_allocation() as f64 / 1024.0); } Some(_) => { - return Err("bad argument to 'collectgarbage'".into_value(ctx).into()); + return Err(argument_error(ctx, "collectgarbage", 1, "string expected")); } None => {} } diff --git a/src/stdlib/math.rs b/src/stdlib/math.rs index 99ae3e65..e6d17e5a 100644 --- a/src/stdlib/math.rs +++ b/src/stdlib/math.rs @@ -1,5 +1,6 @@ use gc_arena::Mutation; +use super::argument_error; use crate::{ async_sequence, meta_ops, Callback, CallbackReturn, Context, FromMultiValue, IntoMultiValue, IntoValue, SequenceReturn, Table, Value, @@ -11,13 +12,16 @@ where A: FromMultiValue<'gc>, R: IntoMultiValue<'gc>, { - Callback::from_fn(mc, move |ctx, _, mut stack| { - if let Some(res) = f(ctx, stack.consume(ctx)?) { - stack.replace(ctx, res); - Ok(CallbackReturn::Return) - } else { - Err(format!("Bad argument to {name}").into_value(ctx).into()) + Callback::from_fn(mc, move |ctx, _, mut stack| match stack.consume::(ctx) { + Ok(args) => { + if let Some(res) = f(ctx, args) { + stack.replace(ctx, res); + Ok(CallbackReturn::Return) + } else { + Err(argument_error(ctx, name, 1, "value expected")) + } } + Err(_) => Err(argument_error(ctx, name, 1, "value expected")), }) } diff --git a/src/stdlib/mod.rs b/src/stdlib/mod.rs index aa766153..1d3f29b3 100644 --- a/src/stdlib/mod.rs +++ b/src/stdlib/mod.rs @@ -1,3 +1,5 @@ +use crate::{Context, Error, IntoValue, Value}; + mod base; mod coroutine; mod io; @@ -9,3 +11,76 @@ pub use self::{ base::load_base, coroutine::load_coroutine, io::load_io, math::load_math, string::load_string, table::load_table, }; + +/// Build `bad argument #N to 'func' (message)` like [`luaL_argerror`](https://www.lua.org/manual/5.4/manual.html#luaL_argerror) +pub fn argument_error<'gc>(ctx: Context<'gc>, func: &str, arg: usize, msg: &str) -> Error<'gc> { + format!("bad argument #{} to '{}' ({})", arg, func, msg) + .into_value(ctx) + .into() +} + +/// Build `bad argument #N to 'func' (expected, got)` like [`luaL_typeerror`](https://www.lua.org/manual/5.4/manual.html#luaL_typeerror) +pub fn type_error<'gc>( + ctx: Context<'gc>, + func: &str, + arg: usize, + expected: &str, + found: &str, +) -> Error<'gc> { + argument_error( + ctx, + func, + arg, + &format!("{} expected, got {}", expected, found), + ) +} + +/// Build the special integer-conversion message used by [`luaL_checkinteger`](https://www.lua.org/manual/5.4/manual.html#luaL_checkinteger) +pub fn integer_representation_error<'gc>(ctx: Context<'gc>, func: &str, arg: usize) -> Error<'gc> { + argument_error(ctx, func, arg, "number has no integer representation") +} + +/// Equivalent of [`luaL_optinteger`](https://www.lua.org/manual/5.4/manual.html#luaL_optinteger). +/// # Semantics +/// - `None` if `value` is `nil` +/// - `Ok(i64)` if `value` converts to `integer` +/// - `Err` with precise message otherwise +pub fn parse_opt_integer_arg<'gc>( + ctx: Context<'gc>, + func: &str, + arg: usize, + value: Option>, +) -> Result, Error<'gc>> { + let Some(v) = value else { return Ok(None) }; + if v.is_nil() { + return Ok(None); + } + if let Some(i) = v.to_integer() { + return Ok(Some(i)); + } + if v.to_number().is_some() { + return Err(integer_representation_error(ctx, func, arg)); + } + Err(type_error(ctx, func, arg, "number", v.type_name())) +} + +/// Equivalent of [`luaL_optlstring`](https://www.lua.org/manual/5.4/manual.html#luaL_optlstring) semantics used for separators in [`table.concat`](https://www.lua.org/manual/5.4/manual.html#pdf-table.concat). +/// # Semantics +/// - `None` if `value` is `nil` +/// - `Some(Value)` if `value` is a string or a number +/// - `Err` with precise message otherwise +pub fn parse_opt_stringlike_arg<'gc>( + ctx: Context<'gc>, + func: &str, + arg: usize, + value: Option>, +) -> Result>, Error<'gc>> { + let Some(v) = value else { return Ok(None) }; + if v.is_nil() { + return Ok(None); + } + if v.into_string(ctx).is_some() { + return Ok(Some(v)); + } + Err(type_error(ctx, func, arg, "string", v.type_name())) +} diff --git a/src/stdlib/string.rs b/src/stdlib/string.rs index 556537c8..8d1192e6 100644 --- a/src/stdlib/string.rs +++ b/src/stdlib/string.rs @@ -1,3 +1,4 @@ +use super::{argument_error, type_error}; use crate::{Callback, CallbackReturn, Context, FromValue, String, Table, Value}; pub fn load_string<'gc>(ctx: Context<'gc>) { @@ -7,8 +8,16 @@ pub fn load_string<'gc>(ctx: Context<'gc>) { ctx, "len", Callback::from_fn(&ctx, |ctx, _, mut stack| { - let string = stack.consume::(ctx)?; - let len = string.len(); + if stack.len() == 0 { + return Err(argument_error(ctx, "len", 1, "string expected")); + } + // Accept numbers like Lua string library does via implicit tostring for some APIs + let value = stack.get(0); + let len = if let Some(s) = value.into_string(ctx) { + s.len() + } else { + return Err(type_error(ctx, "len", 1, "string", value.type_name())); + }; stack.replace(ctx, len); Ok(CallbackReturn::Return) }), @@ -18,6 +27,9 @@ pub fn load_string<'gc>(ctx: Context<'gc>) { ctx, "byte", Callback::from_fn(&ctx, |ctx, _, mut stack| { + if stack.len() == 0 { + return Err(argument_error(ctx, "byte", 1, "string expected")); + } let (string, i, j) = stack.consume::<(String, Option, Option)>(ctx)?; let i = i.unwrap_or(1); let substr = sub(string.as_bytes(), i, j.or(Some(i)))?; @@ -33,7 +45,10 @@ pub fn load_string<'gc>(ctx: Context<'gc>) { let string = ctx.intern( &stack .into_iter() - .map(|c| u8::from_value(ctx, c)) + .map(|c| { + u8::from_value(ctx, c) + .map_err(|_| argument_error(ctx, "char", 1, "value expected")) + }) .collect::, _>>()?, ); stack.replace(ctx, string); @@ -45,6 +60,9 @@ pub fn load_string<'gc>(ctx: Context<'gc>) { ctx, "sub", Callback::from_fn(&ctx, |ctx, _, mut stack| { + if stack.len() == 0 { + return Err(argument_error(ctx, "sub", 1, "string expected")); + } let (string, i, j) = stack.consume::<(String, i64, Option)>(ctx)?; let substr = ctx.intern(sub(string.as_bytes(), i, j)?); stack.replace(ctx, substr); @@ -56,6 +74,9 @@ pub fn load_string<'gc>(ctx: Context<'gc>) { ctx, "lower", Callback::from_fn(&ctx, |ctx, _, mut stack| { + if stack.len() == 0 { + return Err(argument_error(ctx, "lower", 1, "string expected")); + } let string = stack.consume::(ctx)?; let lowered = ctx.intern( &string @@ -73,6 +94,9 @@ pub fn load_string<'gc>(ctx: Context<'gc>) { ctx, "reverse", Callback::from_fn(&ctx, |ctx, _, mut stack| { + if stack.len() == 0 { + return Err(argument_error(ctx, "reverse", 1, "string expected")); + } let string = stack.consume::(ctx)?; let reversed = ctx.intern(&string.as_bytes().iter().copied().rev().collect::>()); stack.replace(ctx, reversed); @@ -84,6 +108,9 @@ pub fn load_string<'gc>(ctx: Context<'gc>) { ctx, "upper", Callback::from_fn(&ctx, |ctx, _, mut stack| { + if stack.len() == 0 { + return Err(argument_error(ctx, "upper", 1, "string expected")); + } let string = stack.consume::(ctx)?; let uppered = ctx.intern( &string diff --git a/src/stdlib/table.rs b/src/stdlib/table.rs index d06dbe1d..247645a1 100644 --- a/src/stdlib/table.rs +++ b/src/stdlib/table.rs @@ -4,11 +4,12 @@ use std::pin::Pin; use anyhow::Context as _; use gc_arena::Collect; +use super::type_error; use crate::{ async_callback::{AsyncSequence, Locals}, async_sequence, fuel::count_fuel, - meta_ops::{self, concat_separated, ConcatMetaResult, MetaResult}, + meta_ops::{self, ConcatMetaResult, MetaResult}, table::RawTable, BoxSequence, Callback, CallbackReturn, Closure, Context, Error, Execution, Function, IntoValue, MetaMethod, Sequence, SequencePoll, SequenceReturn, Stack, StashedError, StashedFunction, @@ -43,7 +44,7 @@ pub fn load_table<'gc>(ctx: Context<'gc>) { } let length = try_compute_length(start, end) - .ok_or_else(|| "Too many values to unpack".into_value(ctx))?; + .ok_or("too many results to unpack".into_value(ctx))?; Unpack::MainLoop { start, table, @@ -70,7 +71,7 @@ pub fn load_table<'gc>(ctx: Context<'gc>) { let then_impl = Callback::from_fn_with(&ctx, sep, |sep, ctx, _, mut stack| { let values = &stack[..]; - match concat_separated(ctx, values, *sep)? { + match meta_ops::concat_separated(ctx, values, *sep)? { ConcatMetaResult::Value(v) => { stack.replace(ctx, v); Ok(CallbackReturn::Return) @@ -97,7 +98,6 @@ pub fn load_table<'gc>(ctx: Context<'gc>) { } } - // Defer to table.unpack for indexing implementation. Ok(CallbackReturn::Call { function: *unpack, then: Some(BoxSequence::new(&ctx, CallSequence(then_impl.into()))), @@ -120,6 +120,99 @@ pub fn load_table<'gc>(ctx: Context<'gc>) { ctx.set_global("table", table); } +#[derive(Collect)] +#[collect(no_drop)] +struct ConcatThen<'gc> { + sep: Value<'gc>, + list: Value<'gc>, + i: i64, +} + +impl<'gc> Sequence<'gc> for ConcatThen<'gc> { + fn poll( + self: Pin<&mut Self>, + ctx: Context<'gc>, + _exec: Execution<'gc, '_>, + mut stack: Stack<'gc, '_>, + ) -> Result, Error<'gc>> { + // after __len, there should be one value on top + let jv = stack.consume::(ctx)?; + let j = jv + .to_integer() + .ok_or_else(|| type_error(ctx, "concat", 4, "number", jv.type_name()))?; + let res = concat_range(ctx, self.list, self.sep, self.i, j)?; + // res returns a CallbackReturn; translate to SequencePoll + match res { + CallbackReturn::Return => Ok(SequencePoll::Return), + CallbackReturn::Call { function, .. } => Ok(SequencePoll::TailCall(function)), + CallbackReturn::Sequence(_seq) => { + // We do not expect concat to produce a Sequence here in practice because + // we only call concat_separated_with_validation which either returns a Value + // or a function to call next. Conservatively return. + Ok(SequencePoll::Return) + } + CallbackReturn::Yield { .. } | CallbackReturn::Resume { .. } => unreachable!(), + } + } +} + +fn concat_range<'gc>( + ctx: Context<'gc>, + list: Value<'gc>, + sep: Value<'gc>, + mut i: i64, + j: i64, +) -> Result, Error<'gc>> { + // Empty interval: return "" + if i > j { + return Ok(CallbackReturn::Return); + } + + // Gather values via meta_ops::index for range [i..j] + let s = async_sequence(&ctx, |locals, mut seq| { + let list = locals.stash(&ctx, list); + let sep = locals.stash(&ctx, sep); + async move { + // Collect range values into the stack + while i <= j { + let call = seq.try_enter(|ctx, locals, _, mut stack| { + let call = meta_ops::index(ctx, locals.fetch(&list), Value::Integer(i))?; + let p = match call { + MetaResult::Value(v) => { + stack.push_back(v); + None + } + MetaResult::Call(call) => { + stack.extend(call.args); + Some(locals.stash(&ctx, call.function)) + } + }; + Ok(p) + })?; + if let Some(call) = call { + seq.call(&call, 0).await?; + } + i = i.saturating_add(1); + } + + // Validate element types and perform concatenation with separator + seq.try_enter(|ctx, locals, _, mut stack| { + let sep = locals.fetch(&sep); + match meta_ops::concat_separated(ctx, &stack[..], sep)? { + ConcatMetaResult::Value(v) => { + stack.replace(ctx, v); + Ok(SequenceReturn::Return) + } + ConcatMetaResult::Call(func) => { + Ok(SequenceReturn::Call(locals.stash(&ctx, func))) + } + } + }) + } + }); + Ok(CallbackReturn::Sequence(s)) +} + fn prep_metaop_call<'gc, const N: usize>( ctx: Context<'gc>, mut stack: Stack<'gc, '_>, @@ -195,6 +288,7 @@ fn table_remove_impl<'gc>( mut exec: Execution<'gc, '_>, mut stack: Stack<'gc, '_>, ) -> Result, Error<'gc>> { + // Lua errors if the first argument is not a table-like value for write/len let (table, index): (Table, Option) = stack.consume(ctx)?; let length; @@ -225,9 +319,7 @@ fn table_remove_impl<'gc>( length = Some(len); } (RawArrayOpResult::Failed, _) => { - return Err("Invalid index passed to table.remove" - .into_value(ctx) - .into()); + return Err("position out of bounds".into_value(ctx).into()); } } } else { @@ -293,11 +385,7 @@ fn table_remove_impl<'gc>( // The last value is still on the stack Ok(SequenceReturn::Return) } else { - seq.try_enter(|ctx, _, _, _| { - Err("Invalid index passed to table.remove" - .into_value(ctx) - .into()) - }) + seq.try_enter(|ctx, _, _, _| Err("position out of bounds".into_value(ctx).into())) } } }); @@ -313,7 +401,11 @@ fn table_insert_impl<'gc>( let index: Option; let value: Value; match stack.len() { - 0..=1 => return Err("Missing arguments to insert".into_value(ctx).into()), + 0..=1 => { + return Err("wrong number of arguments to 'insert'" + .into_value(ctx) + .into()) + } 2 => { (table, value) = stack.consume(ctx)?; index = None; @@ -361,9 +453,7 @@ fn table_insert_impl<'gc>( length = Some(len); } (RawArrayOpResult::Failed, _) => { - return Err("Invalid index passed to table.insert" - .into_value(ctx) - .into()); + return Err("position out of bounds".into_value(ctx).into()); } } } else { @@ -410,9 +500,7 @@ fn table_insert_impl<'gc>( if !(1..=end_index).contains(&index) { return seq.try_enter(|ctx, _, _, _| { - Err("Invalid index passed to table.insert" - .into_value(ctx) - .into()) + Err("position out of bounds".into_value(ctx).into()) }); } diff --git a/src/thread/executor.rs b/src/thread/executor.rs index 4f4630bd..87772d3a 100644 --- a/src/thread/executor.rs +++ b/src/thread/executor.rs @@ -45,6 +45,12 @@ pub struct ExecutorState<'gc> { thread_stack: vec::Vec, MetricsAlloc<'gc>>, } +impl<'gc> ExecutorState<'gc> { + pub(crate) fn thread_stack(&self) -> &[Thread<'gc>] { + &self.thread_stack + } +} + pub type ExecutorInner<'gc> = RefLock>; /// The entry-point for the Lua VM. diff --git a/src/thread/mod.rs b/src/thread/mod.rs index 8e6a0591..70d975b7 100644 --- a/src/thread/mod.rs +++ b/src/thread/mod.rs @@ -14,6 +14,8 @@ pub use self::{ thread::{BadThreadMode, OpenUpValue, Thread, ThreadInner, ThreadMode}, }; +pub(crate) use thread::Frame; + #[derive(Debug, Clone, Error)] pub enum VMError { #[error("{}", if *.0 { diff --git a/src/thread/thread.rs b/src/thread/thread.rs index a03d4c2a..d60aa18d 100644 --- a/src/thread/thread.rs +++ b/src/thread/thread.rs @@ -264,7 +264,7 @@ impl<'gc> OpenUpValue<'gc> { #[derive(Debug, Copy, Clone, Collect)] #[collect(require_static)] -pub(super) enum MetaReturn { +pub(crate) enum MetaReturn { /// No return value is expected. None, /// Place a single return value at an index relative to the returned to function's stack bottom. @@ -275,7 +275,7 @@ pub(super) enum MetaReturn { #[derive(Debug, Copy, Clone, Collect)] #[collect(require_static)] -pub(super) enum LuaReturn { +pub(crate) enum LuaReturn { /// Normal function call, place return values at the bottom of the returning function's stack, /// as normal. Normal(VarCount), @@ -285,7 +285,7 @@ pub(super) enum LuaReturn { #[derive(Debug, Collect)] #[collect(no_drop)] -pub(super) enum Frame<'gc> { +pub(crate) enum Frame<'gc> { /// A running Lua frame. Lua { bottom: usize, @@ -357,6 +357,10 @@ impl<'gc> ThreadState<'gc> { } } + pub(crate) fn frames(&self) -> &[Frame<'gc>] { + &self.frames + } + /// Pushes a new function call frame. /// /// Arguments are taken from the top of the stack starting at `bottom`, which will become the diff --git a/tests/goldenscripts.rs b/tests/goldenscripts.rs index 57029c45..75876358 100644 --- a/tests/goldenscripts.rs +++ b/tests/goldenscripts.rs @@ -192,8 +192,10 @@ fn test_goldenscripts() { match mode { GoldenScriptMode::CompileError => { if let Some(error) = compile_error { + // New compiler error format is already Lua-like without the runtime/compile prefixes let formatted_error = format!("{error}\n"); - if formatted_error != expected_output { + let expected_norm = expected_output.replace("\r\n", "\n"); + if formatted_error != expected_norm { eprintln!("{path:?}: did not match expected output\n\nexpected:\n{expected_output}\noutput:\n{formatted_error}"); failed_scripts.push(path); continue; @@ -220,8 +222,9 @@ fn test_goldenscripts() { } // Stitch together our expected byte output, and compare let output: Vec<_> = rx.try_iter().flatten().collect(); - if output != expected_output.as_bytes() { - // Technically `output` is ASCII, but UTF8 is compatible + let expected_norm = expected_output.replace("\r\n", "\n"); + if output != expected_norm.as_bytes() { + // Show a clean string for output to avoid path separator issues eprintln!("{path:?}: did not match expected output\n\nexpected:\n{expected_output}\noutput:\n{}\n---\n{output:?}", String::from_utf8_lossy(&output)); failed_scripts.push(path); continue; diff --git a/tests/goldenscripts/close-multiple.lua b/tests/goldenscripts/close-multiple.lua index 7d07227a..98b6b823 100644 --- a/tests/goldenscripts/close-multiple.lua +++ b/tests/goldenscripts/close-multiple.lua @@ -1,4 +1,4 @@ --- error ---- runtime error: compile error: compiler error at line 4: multiple to-be-closed variables in local list +--- ./tests/goldenscripts/close-multiple.lua:4: multiple to-be-closed variables in local list local a , b = {}, {} diff --git a/tests/goldenscripts/close-unimpl.lua b/tests/goldenscripts/close-unimpl.lua index 97f48799..772bdd06 100644 --- a/tests/goldenscripts/close-unimpl.lua +++ b/tests/goldenscripts/close-unimpl.lua @@ -1,4 +1,4 @@ --- error ---- runtime error: compile error: compiler error at line 4: close attribute currently unsupported +--- ./tests/goldenscripts/close-unimpl.lua:4: close attribute currently unsupported local c = {} diff --git a/tests/goldenscripts/const1.lua b/tests/goldenscripts/const1.lua index a5b52e84..bf9c3a33 100644 --- a/tests/goldenscripts/const1.lua +++ b/tests/goldenscripts/const1.lua @@ -1,5 +1,5 @@ --- error ---- runtime error: compile error: compiler error at line 5: cannot assign to a const variable +--- ./tests/goldenscripts/const1.lua:5: cannot assign to a const variable local c = 3 c = 4 diff --git a/tests/goldenscripts/const3.lua b/tests/goldenscripts/const3.lua index aa6f5fae..e6a6255e 100644 --- a/tests/goldenscripts/const3.lua +++ b/tests/goldenscripts/const3.lua @@ -1,5 +1,5 @@ --- error ---- runtime error: compile error: compiler error at line 5: cannot assign to a const variable +--- ./tests/goldenscripts/const3.lua:5: cannot assign to a const variable local a = 3 function a() diff --git a/tests/goldenscripts/const4.lua b/tests/goldenscripts/const4.lua index 396bd61a..1e2d3563 100644 --- a/tests/goldenscripts/const4.lua +++ b/tests/goldenscripts/const4.lua @@ -1,5 +1,5 @@ --- error ---- runtime error: compile error: compiler error at line 7: cannot assign to a const variable +--- ./tests/goldenscripts/const4.lua:7: cannot assign to a const variable local a = 3 diff --git a/tests/goldenscripts/const5.lua b/tests/goldenscripts/const5.lua index b441dae6..93486044 100644 --- a/tests/goldenscripts/const5.lua +++ b/tests/goldenscripts/const5.lua @@ -1,5 +1,5 @@ --- error ---- runtime error: compile error: compiler error at line 7: cannot assign to a const variable +--- ./tests/goldenscripts/const5.lua:7: cannot assign to a const variable local a = 5