From 716b54797db8c2008e105833ce9445a97ed258df Mon Sep 17 00:00:00 2001 From: SeanOMik Date: Sun, 28 Jan 2024 11:26:30 -0500 Subject: [PATCH] Improve error messages when collecting function arguments for execution --- src/main.rs | 66 +++++++++++++++++++++++---- src/state.rs | 77 +++++++++++++++++++++++++------ src/userdata.rs | 119 +++++++++++++++++++++++++++++++++++++++--------- src/value.rs | 40 +++++++++++----- 4 files changed, 249 insertions(+), 53 deletions(-) diff --git a/src/main.rs b/src/main.rs index 077f25b..3ce86c8 100755 --- a/src/main.rs +++ b/src/main.rs @@ -99,7 +99,7 @@ fn main() -> Result<()> { local v2 = Vec2.new(500, 500) print("v2 is (" .. v2.x .. ", " .. v2.y .. ")") - local v_add = v1 + v2 + local v_add = v1:add(v2) v_add.x = 90 print("v_add is (" .. v_add.x .. ", " .. v_add.y .. ")") "#).unwrap(); @@ -165,12 +165,15 @@ pub enum Error { Nil, #[error("Unexpected type, expected {0} but got {1}")] UnexpectedType(String, String), - #[error("Bad argument provided to {func:?}! Argument #{arg_index} (name: {arg_name:?}), cause: {error}")] + #[error("Bad argument provided to `{}` at position {arg_index}{}, cause: {error}", + .func.clone().unwrap_or("Unknown".to_string()), + .arg_name.clone().map(|a| format!(" (name: {})", a)).unwrap_or("".to_string()))] BadArgument { func: Option, arg_index: i32, arg_name: Option, /// the error that describes what was wrong for this argument + #[source] error: Arc }, #[error("Incorrect number of arguments, expected {arg_expected}, got {arg_count}")] @@ -179,7 +182,17 @@ pub enum Error { arg_count: i32, }, #[error("There is already a registry entry with the key {0}")] - RegistryConflict(String) + RegistryConflict(String), + #[error("Userdata types did not match")] + UserdataMismatch, + #[error("Missing meta table for userdata")] + MissingMetatable, + #[error("An error occurred when attempting to convert from a ValueVec at value index {value_idx}, cause: {error}")] + ValueVecError { + value_idx: i32, + #[source] + error: Arc, + }, } impl Error { @@ -219,6 +232,20 @@ impl<'a> PushToLuaStack<'a> for () { } } +impl<'a, T: PushToLuaStack<'a>> PushToLuaStack<'a> for Option { + unsafe fn push_to_lua_stack(&self, state: &'a State) -> Result<()> { + if let Some(v) = self { + v.push_to_lua_stack(state)?; + } else { + unsafe { + lua::lua_pushnil(state.state_ptr()); + } + } + + Ok(()) + } +} + /// Implements PushToLuaStack for a number macro_rules! impl_as_lua_number { ($ty: ident) => { @@ -292,14 +319,31 @@ impl<'a> PushToLuaStack<'a> for &str { } } +pub struct Vec3 { + x: f32, + y: f32, + z: f32, +} + +impl Userdata for Vec3 { + fn build<'a>(builder: &mut UserdataBuilder<'a, Self>) -> crate::Result<()> { + todo!() + } + + fn name() -> String { + todo!() + } +} + pub struct Vec2 { x: f32, y: f32, } impl Userdata for Vec2 { + fn build<'a>(builder: &mut UserdataBuilder<'a, Vec2>) -> crate::Result<()> { - builder.name("Vec2") + builder .field_getter("x", |_, this| Ok(this.x)) .field_getter("y", |_, this| Ok(this.y)) @@ -311,7 +355,7 @@ impl Userdata for Vec2 { }) // method test - .method("add", |lua, lhs: &Vec2, (rhs,): (&Vec2,)| { + .method("add", |lua, lhs: &Vec2, (rhs,): (&Vec3,)| { let lx = lhs.x; let ly = lhs.y; @@ -333,6 +377,10 @@ impl Userdata for Vec2 { Ok(()) } + + fn name() -> String { + "Vec2".to_string() + } } pub struct UserdataProxy(PhantomData); @@ -355,12 +403,14 @@ impl Userdata for UserdataProxy { let mut other = UserdataBuilder::::new(); T::build(&mut other)?; - let name = format!("{}Proxy", other.name.unwrap()); - builder.name = Some(name); - // only the functions need to be added since they're the only thing usable from a proxy builder.functions = other.functions; Ok(()) } + + fn name() -> String { + let name = format!("{}Proxy", T::name()); + name + } } \ No newline at end of file diff --git a/src/state.rs b/src/state.rs index 7ebd956..9d529fe 100755 --- a/src/state.rs +++ b/src/state.rs @@ -1,5 +1,5 @@ use core::ffi; -use std::{collections::HashMap, ffi::{CString, CStr}, mem, ptr::{self, NonNull}, str::Utf8Error}; +use std::{alloc::{self, Layout}, any::TypeId, collections::HashMap, ffi::{CString, CStr}, mem, ptr::{self, NonNull}, str::Utf8Error}; use mlua_sys as lua; @@ -81,21 +81,34 @@ impl From<&[StdLibrary; N]> for StdLibraries { } } +#[derive(Default)] +struct ExtraSpace { + userdata_metatables: HashMap, +} + pub struct State { lua: NonNull, } impl Drop for State { fn drop(&mut self) { - unsafe { lua::lua_close(self.lua.as_ptr()); } + unsafe { + let extra = self.get_extra_space_ptr(); + extra.drop_in_place(); + + lua::lua_close(self.lua.as_ptr()); + } } } impl State { pub fn new() -> Self { - Self { + let s = Self { lua: unsafe { NonNull::new_unchecked(lua::luaL_newstate()) } - } + }; + + s.alloc_extra_space(); + s } pub fn from_ptr(lua: *mut lua::lua_State) -> Self { @@ -104,10 +117,39 @@ impl State { } } - pub fn state_ptr(&self) -> *mut lua::lua_State { + pub(crate) fn state_ptr(&self) -> *mut lua::lua_State { self.lua.as_ptr() } + fn alloc_extra_space(&self) { + unsafe { + let extra_ptr = alloc::alloc(Layout::new::()) + .cast::(); + ptr::write(extra_ptr, ExtraSpace::default()); + + let s = self.state_ptr(); + let loc = lua::lua_getextraspace(s); + let loc = loc.cast::<*mut ExtraSpace>(); + *loc = extra_ptr; + } + } + + fn get_extra_space_ptr(&self) -> *mut ExtraSpace { + unsafe { + let s = self.state_ptr(); + let extra = lua::lua_getextraspace(s) + .cast::<*mut ExtraSpace>(); + *extra + } + } + + fn get_extra_space(&self) -> &mut ExtraSpace { + unsafe { + self.get_extra_space_ptr().as_mut() + .expect("Somehow the Lua state's extra data got deleted!") + } + } + pub fn execute(&self, text: &str) -> Result<()> { let text = format!("{}\0", text); @@ -291,17 +333,16 @@ impl State { let name_cstr = name_cstr.as_ptr() as *const i8; // attempt to get the metatable - lua::lua_pushstring(s, name_cstr); - let ty = lua::lua_rawget(s, lua::LUA_REGISTRYINDEX); - if ty == lua::LUA_TNIL { - lua::lua_pop(s, 1); // remove nil + let udmts = &mut self.get_extra_space().userdata_metatables;//.get(&TypeId::of::()) - // if the metatable is not made yet, make it + if !udmts.contains_key(&TypeId::of::()) { + // create the userdata's metatable and store it in extra space let mt = self.create_userdata_metatable::()?; mt.push_to_lua_stack(self)?; - } else if ty != lua::LUA_TTABLE { - return Err(Error::RegistryConflict(name.to_string())); + } else { + let mt = udmts.get(&TypeId::of::()).unwrap(); + mt.push_to_lua_stack(self)?; } lua::lua_setmetatable(s, -2); @@ -310,13 +351,20 @@ impl State { } } + pub(crate) fn get_userdata_metatable<'a, T: Userdata + 'static>(&'a self) -> Option> { + let extra = self.get_extra_space(); + let mt = extra.userdata_metatables.get(&TypeId::of::()); + + mt.map(|r| Table::with_ref(self, r.clone(), true).unwrap()) + } + pub(crate) fn create_userdata_metatable<'a, T: Userdata + 'static>(&'a self) -> Result> { let mut builder = UserdataBuilder::::new(); T::build(&mut builder)?; let getters = builder.field_getters; let setters = builder.field_setters; - let name = builder.name.unwrap(); + let name = builder.name; let mut fns_table = HashMap::new(); for (func_name, func) in builder.functions.into_iter() { @@ -375,6 +423,9 @@ impl State { mt.set_meta(mm_name, mm_func)?; } + let extra = self.get_extra_space(); + extra.userdata_metatables.insert(TypeId::of::(), mt.lref.clone()); + Ok(mt) } } \ No newline at end of file diff --git a/src/userdata.rs b/src/userdata.rs index be2dad0..906a2cf 100755 --- a/src/userdata.rs +++ b/src/userdata.rs @@ -1,6 +1,6 @@ -use std::{collections::HashMap, marker::PhantomData}; +use std::{collections::HashMap, ffi::CStr, marker::PhantomData}; -use crate::{ensure_type, AsLua, FromLua, FromLuaStack, FromLuaVec, LuaRef, PushToLuaStack, State, Value, ValueVec}; +use crate::{ensure_type, AsLua, FromLua, FromLuaStack, FromLuaVec, LuaRef, PushToLuaStack, StackGuard, State, Value, ValueVec}; use mlua_sys as lua; @@ -95,7 +95,7 @@ pub trait FieldGetter { type UserdataFn<'a> = Box) -> crate::Result>>; pub struct UserdataBuilder<'a, T> { - pub(crate) name: Option, + pub(crate) name: String, pub(crate) field_getters: HashMap>, pub(crate) field_setters: HashMap>, pub(crate) functions: HashMap>, @@ -103,10 +103,10 @@ pub struct UserdataBuilder<'a, T> { _marker: PhantomData, } -impl<'a, T> UserdataBuilder<'a, T> { +impl<'a, T: Userdata> UserdataBuilder<'a, T> { pub fn new() -> Self { Self { - name: None, + name: T::name(), field_getters: HashMap::new(), field_setters: HashMap::new(), functions: HashMap::new(), @@ -115,15 +115,11 @@ impl<'a, T> UserdataBuilder<'a, T> { } } - pub fn name(&mut self, name: &str) -> &mut Self { - self.name = Some(name.to_string()); - self - } - pub fn field_getter(&mut self, name: &str, f: F) -> &mut Self where F: Fn(&'a State, &T) -> crate::Result + 'static, R: AsLua<'a>, + T: Userdata + 'static { let wrap = move |lua: &'a State, mut val: ValueVec<'a>| { let val = val.pop_front().unwrap(); @@ -140,6 +136,7 @@ impl<'a, T> UserdataBuilder<'a, T> { where F: Fn(&'a State, &mut T, V) -> () + 'static, V: FromLua<'a>, + T: Userdata + 'static { let wrap = move |lua: &'a State, mut val: ValueVec<'a>| { let lua_val = val.pop_front().unwrap(); @@ -156,14 +153,30 @@ impl<'a, T> UserdataBuilder<'a, T> { self } + fn result_to_bad_arg(res: crate::Result, ud_name: &str, fn_name: &str) -> crate::Result { + res.map_err(|e| match e { + crate::Error::ValueVecError { value_idx, error } => { + let full_name = format!("{}.{}", ud_name, fn_name); + // added 2 to value index since the error idx starts at 0, + // and `this` counts as the first argument in the lua function. + crate::Error::BadArgument { func: Some(full_name), arg_index: value_idx + 2, arg_name: None, error, } + }, + _ => e + }) + } + pub fn function(&mut self, name: &str, f: F) -> &mut Self where F: Fn(&'a State, A) -> crate::Result + 'static, A: FromLuaVec<'a>, R: AsLua<'a>, { + let fn_name = name.to_string(); let wrap = move |lua: &'a State, val: ValueVec<'a>| { - let args = A::from_lua_value_vec(lua, val)?; + let args = Self::result_to_bad_arg( + A::from_lua_value_vec(lua, val), + &T::name(), &fn_name + )?; f(lua, args).and_then(|r| r.as_lua(lua)) }; self.functions.insert(name.to_string(), Box::new(wrap)); @@ -176,13 +189,19 @@ impl<'a, T> UserdataBuilder<'a, T> { F: Fn(&'a State, &T, A) -> crate::Result + 'static, A: FromLuaVec<'a>, R: AsLua<'a>, + T: Userdata + 'static { + let fn_name = name.to_string(); let wrap = move |lua: &'a State, mut val: ValueVec<'a>| { let this_val = val.pop_front().unwrap(); let this = this_val.as_userdata().unwrap(); // if this panics, its a bug let this = this.as_ref::()?; - - let args = A::from_lua_value_vec(lua, val)?; + + let this_name = T::name(); + let args = Self::result_to_bad_arg( + A::from_lua_value_vec(lua, val), + &this_name, &fn_name + )?; f(lua, this, args).and_then(|r| r.as_lua(lua)) }; @@ -197,6 +216,7 @@ impl<'a, T> UserdataBuilder<'a, T> { F: Fn(&'a State, &T, A) -> crate::Result + 'static, A: FromLuaVec<'a>, R: AsLua<'a>, + T: Userdata + 'static { let wrap = move |lua: &'a State, mut val: ValueVec<'a>| { let this_val = val.pop_front().unwrap(); @@ -214,6 +234,8 @@ impl<'a, T> UserdataBuilder<'a, T> { } pub trait Userdata: Sized { + fn name() -> String; + fn build<'a>(builder: &mut UserdataBuilder<'a, Self>) -> crate::Result<()>; } @@ -232,23 +254,78 @@ impl<'a> AnyUserdata<'a> { } } - pub fn as_ref(&self) -> crate::Result<&'a T> { + pub fn as_ref(&self) -> crate::Result<&'a T> { unsafe { + self.state.ensure_stack(3)?; + let _g = StackGuard::new(self.state); let s = self.state.state_ptr(); self.lref.push_to_lua_stack(self.state)?; - let cptr = lua::lua_touserdata(s, -1); - Ok(cptr.cast::().as_ref().unwrap()) // TODO: Ensure this userdata matches the type of T + + if lua::lua_getmetatable(s, -1) == 0 { + return Err(crate::Error::UserdataMismatch); + } + + self.state.get_userdata_metatable::() + .push_to_lua_stack(self.state)?; + + if lua::lua_rawequal(s, -2, -1) == 1 { + let cptr = lua::lua_touserdata(s, -3); + Ok(cptr.cast::().as_ref().unwrap()) // TODO: Ensure this userdata matches the type of T + } else { + return Err(crate::Error::UserdataMismatch); + } } } - pub fn as_mut(&self) -> crate::Result<&'a mut T> { + pub fn as_mut(&self) -> crate::Result<&'a mut T> { unsafe { + self.state.ensure_stack(3)?; + let _g = StackGuard::new(self.state); let s = self.state.state_ptr(); self.lref.push_to_lua_stack(self.state)?; - let cptr = lua::lua_touserdata(s, -1); - Ok(cptr.cast::().as_mut().unwrap()) // TODO: Ensure this userdata matches the type of T + + if lua::lua_getmetatable(s, -1) == 0 { + return Err(crate::Error::MissingMetatable); + } + + self.state.get_userdata_metatable::() + .push_to_lua_stack(self.state)?; + + if lua::lua_rawequal(s, -2, -1) == 1 { + let cptr = lua::lua_touserdata(s, -3); + Ok(cptr.cast::().as_mut().unwrap()) + } else { + Err(crate::Error::UserdataMismatch) + } + } + } + + /// Returns the name of the userdata by accessing the metatable + pub fn name(&self) -> crate::Result { + unsafe { + self.state.ensure_stack(3)?; + let _g = StackGuard::new(self.state); + + self.lref.push_to_lua_stack(self.state)?; + let s = self.state.state_ptr(); + + if lua::lua_getmetatable(s, -1) == 0 { + return Err(crate::Error::MissingMetatable); + } + + lua::lua_pushliteral(s, "__name"); + lua::lua_rawget(s, -2); + + ensure_type(self.state, lua::LUA_TSTRING, -1)?; + + let cstr = CStr::from_ptr(lua::lua_tostring(s, -1)); + let cstr = cstr.to_str() + // on panic, this should be considered a bug + .expect("Metatable name has invalid utf8 bytes!"); + + Ok(cstr.to_string()) } } } @@ -282,14 +359,14 @@ impl<'a> FromLua<'a> for AnyUserdata<'a> { } } -impl<'a, T: Userdata> FromLua<'a> for &'a T { +impl<'a, T: Userdata + 'static> FromLua<'a> for &'a T { fn from_lua(_lua: &'a State, val: Value<'a>) -> crate::Result { let ud = val.into_userdata()?; ud.as_ref::() } } -impl<'a, T: Userdata> FromLua<'a> for &'a mut T { +impl<'a, T: Userdata + 'static> FromLua<'a> for &'a mut T { fn from_lua(_lua: &'a State, val: Value<'a>) -> crate::Result { let ud = val.into_userdata()?; ud.as_mut::() diff --git a/src/value.rs b/src/value.rs index 92950b6..133ad1f 100755 --- a/src/value.rs +++ b/src/value.rs @@ -22,15 +22,15 @@ impl<'a> Value<'a> { matches!(self, Value::Nil) } - pub fn type_name(&self) -> &'static str { + pub fn type_name(&self) -> String { match self { - Value::None => "None", - Value::Nil => "Nil", - Value::Number(_) => "Number", - Value::String(_) => "String", - Value::Function(_) => "Function", - Value::Table(_) => "Table", - Value::Userdata(_) => todo!(), + Value::None => "None".to_string(), + Value::Nil => "Nil".to_string(), + Value::Number(_) => "Number".to_string(), + Value::String(_) => "String".to_string(), + Value::Function(_) => "Function".to_string(), + Value::Table(_) => "Table".to_string(), + Value::Userdata(ud) => ud.name().unwrap(), } } @@ -204,6 +204,16 @@ impl<'a> ValueVec<'a> { pub fn new() -> Self { Self::default() } + + pub fn names(&self) -> VecDeque { + let mut vec = VecDeque::new(); + + for val in self.0.iter() { + vec.push_back(val.type_name().to_string()); + } + + vec + } } impl<'a> From> for ValueVec<'a> { @@ -244,8 +254,14 @@ macro_rules! impl_from_lua_vec_tuple { }); } - let f = $first::from_lua(state, values.pop_front().unwrap())?; - let ($( $name, )+) = ( $( $name::from_lua(state, values.pop_front().unwrap())?, )+ ); + let f = $first::from_lua(state, values.pop_front().unwrap()) + .map_err(|e| crate::Error::ValueVecError { value_idx: 0, error: std::sync::Arc::new(e) })?; + + let mut idx = 0; + let ($( $name, )+) = ( $( + $name::from_lua(state, values.pop_front().unwrap()) + .map_err(|e| crate::Error::ValueVecError { value_idx: {idx += 1; idx}, error: std::sync::Arc::new(e) })?, + )+ ); Ok( (f, $( $name, )+) ) } @@ -265,7 +281,9 @@ macro_rules! impl_from_lua_vec_tuple { }); } - Ok( ( $only::from_lua(state, values.pop_front().unwrap())?, ) ) + let o = $only::from_lua(state, values.pop_front().unwrap()) + .map_err(|e| crate::Error::ValueVecError { value_idx: 0, error: std::sync::Arc::new(e) })?; + Ok( (o,) ) } } };