From 03787668fd35419cdc2111431a3c6f3db01c0c55 Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Tue, 14 Mar 2023 23:23:46 +0000 Subject: [PATCH] Improve error reporting when calling Rust functions from Lua. In particular new error type `Error::BadArgument` added to help identify bad argument position or name (eg `self` for userdata). --- src/error.rs | 52 +++++++- src/lua.rs | 6 +- src/multi.rs | 28 +++- src/userdata_impl.rs | 303 +++++++++++++++++++++++++++---------------- src/value.rs | 43 +++++- tests/userdata.rs | 29 +++++ 6 files changed, 337 insertions(+), 124 deletions(-) diff --git a/src/error.rs b/src/error.rs index 2c53d6c..9b05bf6 100644 --- a/src/error.rs +++ b/src/error.rs @@ -69,6 +69,20 @@ pub enum Error { StackError, /// Too many arguments to `Function::bind` BindError, + /// Bad argument received from Lua (usually when calling a function). + /// + /// This error can help to identify the argument that caused the error + /// (which is stored in the corresponding field). + BadArgument { + /// Function that was called. + to: Option, + /// Argument position (usually starts from 1). + pos: usize, + /// Argument name. + name: Option, + /// Underlying error returned when converting argument to a Lua value. + error: Arc, + }, /// A Rust value could not be converted to a Lua value. ToLuaConversionError { /// Name of the Rust type that could not be converted. @@ -216,6 +230,17 @@ impl fmt::Display for Error { fmt, "too many arguments to Function::bind" ), + Error::BadArgument { ref to, pos, ref name, ref error } => { + if let Some(name) = name { + write!(fmt, "bad argument `{name}`")?; + } else { + write!(fmt, "bad argument #{pos}")?; + } + if let Some(to) = to { + write!(fmt, " to `{to}`")?; + } + write!(fmt, ": {error}") + }, Error::ToLuaConversionError { from, to, ref message } => { write!(fmt, "error converting {from} to Lua {to}")?; match *message { @@ -247,13 +272,13 @@ impl fmt::Display for Error { write!(fmt, "RegistryKey used from different Lua state") } Error::CallbackError { ref cause, ref traceback } => { - writeln!(fmt, "callback error")?; // Trace errors down to the root let (mut cause, mut full_traceback) = (cause, None); while let Error::CallbackError { cause: ref cause2, traceback: ref traceback2 } = **cause { cause = cause2; full_traceback = Some(traceback2); } + writeln!(fmt, "{cause}")?; if let Some(full_traceback) = full_traceback { let traceback = traceback.trim_start_matches("stack traceback:"); let traceback = traceback.trim_start().trim_end(); @@ -267,7 +292,7 @@ impl fmt::Display for Error { } else { writeln!(fmt, "{}", traceback.trim_end())?; } - write!(fmt, "caused by: {cause}") + Ok(()) } Error::PreviouslyResumedPanic => { write!(fmt, "previously resumed panic returned again") @@ -300,9 +325,30 @@ impl StdError for Error { } impl Error { - pub fn external>>(err: T) -> Error { + pub fn external>>(err: T) -> Self { Error::ExternalError(err.into().into()) } + + pub(crate) fn bad_self_argument(to: &str, error: Error) -> Self { + Error::BadArgument { + to: Some(to.to_string()), + pos: 1, + name: Some("self".to_string()), + error: Arc::new(error), + } + } + + pub(crate) fn from_lua_conversion<'a>( + from: &'static str, + to: &'static str, + message: impl Into>, + ) -> Self { + Error::FromLuaConversionError { + from, + to, + message: message.into().map(|s| s.into()), + } + } } pub trait ExternalError { diff --git a/src/lua.rs b/src/lua.rs index 8d437f1..48d40b4 100644 --- a/src/lua.rs +++ b/src/lua.rs @@ -785,7 +785,7 @@ impl Lua { // We create callback rather than call `func` directly to catch errors // with attached stacktrace. let callback = lua.create_callback(Box::new(move |lua, args| { - func(lua, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + func(lua, A::from_lua_multi_args(args, 1, None, lua)?)?.into_lua_multi(lua) }))?; callback.call(args) }; @@ -1538,7 +1538,7 @@ impl Lua { F: 'static + MaybeSend + Fn(&'lua Lua, A) -> Result, { self.create_callback(Box::new(move |lua, args| { - func(lua, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + func(lua, A::from_lua_multi_args(args, 1, None, lua)?)?.into_lua_multi(lua) })) } @@ -1623,7 +1623,7 @@ impl Lua { FR: 'lua + Future>, { self.create_async_callback(Box::new(move |lua, args| { - let args = match A::from_lua_multi(args, lua) { + let args = match A::from_lua_multi_args(args, 1, None, lua) { Ok(args) => args, Err(e) => return Box::pin(future::err(e)), }; diff --git a/src/multi.rs b/src/multi.rs index 507aac0..d2f5908 100644 --- a/src/multi.rs +++ b/src/multi.rs @@ -39,6 +39,18 @@ impl<'lua, T: FromLua<'lua>> FromLuaMulti<'lua> for T { MultiValue::return_to_pool(values, lua); res } + + #[inline] + fn from_lua_multi_args( + mut values: MultiValue<'lua>, + i: usize, + to: Option<&str>, + lua: &'lua Lua, + ) -> Result { + let res = T::from_lua_arg(values.pop_front().unwrap_or(Nil), i, to, lua); + MultiValue::return_to_pool(values, lua); + res + } } impl<'lua> IntoLuaMulti<'lua> for MultiValue<'lua> { @@ -191,9 +203,21 @@ macro_rules! impl_tuple { #[allow(non_snake_case)] #[inline] fn from_lua_multi(mut values: MultiValue<'lua>, lua: &'lua Lua) -> Result { - $(let $name = values.pop_front().unwrap_or(Nil);)* + $(let $name = FromLua::from_lua(values.pop_front().unwrap_or(Nil), lua)?;)* let $last = FromLuaMulti::from_lua_multi(values, lua)?; - Ok(($(FromLua::from_lua($name, lua)?,)* $last,)) + Ok(($($name,)* $last,)) + } + + #[allow(unused_mut)] + #[allow(non_snake_case)] + #[inline] + fn from_lua_multi_args(mut values: MultiValue<'lua>, mut i: usize, to: Option<&str>, lua: &'lua Lua) -> Result { + $( + let $name = FromLua::from_lua_arg(values.pop_front().unwrap_or(Nil), i, to, lua)?; + i += 1; + )* + let $last = FromLuaMulti::from_lua_multi_args(values, i, to, lua)?; + Ok(($($name,)* $last,)) } } ); diff --git a/src/userdata_impl.rs b/src/userdata_impl.rs index d204e4a..f8e126e 100644 --- a/src/userdata_impl.rs +++ b/src/userdata_impl.rs @@ -1,6 +1,7 @@ -use std::any::TypeId; +use std::any::{self, TypeId}; use std::cell::{Ref, RefCell, RefMut}; use std::marker::PhantomData; +use std::string::String as StdString; use std::sync::{Arc, Mutex, RwLock}; use crate::error::{Error, Result}; @@ -60,138 +61,171 @@ impl<'lua, T: 'static> UserDataRegistrar<'lua, T> { } } - fn box_method(method: M) -> Callback<'lua, 'static> + fn box_method(name: &str, method: M) -> Callback<'lua, 'static> where M: Fn(&'lua Lua, &T, A) -> Result + MaybeSend + 'static, A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = get_function_name::(name); + macro_rules! try_self_arg { + ($res:expr) => { + $res.map_err(|err| Error::bad_self_argument(&name, err))? + }; + ($res:expr, $err:expr) => { + $res.map_err(|_| Error::bad_self_argument(&name, $err))? + }; + } + Box::new(move |lua, mut args| { - if let Some(front) = args.pop_front() { + let front = args.pop_front(); + let call = |ud| { + // Self was at index 1, so we pass 2 here + let args = A::from_lua_multi_args(args, 2, Some(&name), lua)?; + method(lua, ud, args)?.into_lua_multi(lua) + }; + + if let Some(front) = front { let state = lua.state(); - let userdata = AnyUserData::from_lua(front, lua)?; + let userdata = try_self_arg!(AnyUserData::from_lua(front, lua)); unsafe { let _sg = StackGuard::new(state); check_stack(state, 2)?; - let type_id = lua.push_userdata_ref(&userdata.0)?; + let type_id = try_self_arg!(lua.push_userdata_ref(&userdata.0)); match type_id { Some(id) if id == TypeId::of::() => { - let ud = get_userdata_ref::(state)?; - method(lua, &ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = try_self_arg!(get_userdata_ref::(state)); + call(&ud) } #[cfg(not(feature = "send"))] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_borrow().map_err(|_| Error::UserDataBorrowError)?; - method(lua, &ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = try_self_arg!(get_userdata_ref::>>(state)); + let ud = try_self_arg!(ud.try_borrow(), Error::UserDataBorrowError); + call(&ud) } Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_lock().map_err(|_| Error::UserDataBorrowError)?; - method(lua, &ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = try_self_arg!(get_userdata_ref::>>(state)); + let ud = try_self_arg!(ud.try_lock(), Error::UserDataBorrowError); + call(&ud) } #[cfg(feature = "parking_lot")] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_lock().ok_or(Error::UserDataBorrowError)?; - method(lua, &ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = get_userdata_ref::>>(state); + let ud = try_self_arg!(ud); + let ud = try_self_arg!(ud.try_lock().ok_or(Error::UserDataBorrowError)); + call(&ud) } Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_read().map_err(|_| Error::UserDataBorrowError)?; - method(lua, &ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = try_self_arg!(get_userdata_ref::>>(state)); + let ud = try_self_arg!(ud.try_read(), Error::UserDataBorrowError); + call(&ud) } #[cfg(feature = "parking_lot")] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_read().ok_or(Error::UserDataBorrowError)?; - method(lua, &ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = get_userdata_ref::>>(state); + let ud = try_self_arg!(ud); + let ud = try_self_arg!(ud.try_read().ok_or(Error::UserDataBorrowError)); + call(&ud) } - _ => Err(Error::UserDataTypeMismatch), + _ => Err(Error::bad_self_argument(&name, Error::UserDataTypeMismatch)), } } } else { - Err(Error::FromLuaConversionError { - from: "missing argument", - to: "userdata", - message: None, - }) + let err = Error::from_lua_conversion("missing argument", "userdata", None); + Err(Error::bad_self_argument(&name, err)) } }) } - fn box_method_mut(method: M) -> Callback<'lua, 'static> + fn box_method_mut(name: &str, method: M) -> Callback<'lua, 'static> where M: FnMut(&'lua Lua, &mut T, A) -> Result + MaybeSend + 'static, A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = get_function_name::(name); + macro_rules! try_self_arg { + ($res:expr) => { + $res.map_err(|err| Error::bad_self_argument(&name, err))? + }; + ($res:expr, $err:expr) => { + $res.map_err(|_| Error::bad_self_argument(&name, $err))? + }; + } + let method = RefCell::new(method); Box::new(move |lua, mut args| { - if let Some(front) = args.pop_front() { + let mut method = method + .try_borrow_mut() + .map_err(|_| Error::RecursiveMutCallback)?; + let front = args.pop_front(); + let call = |ud| { + // Self was at index 1, so we pass 2 here + let args = A::from_lua_multi_args(args, 2, Some(&name), lua)?; + method(lua, ud, args)?.into_lua_multi(lua) + }; + + if let Some(front) = front { let state = lua.state(); - let userdata = AnyUserData::from_lua(front, lua)?; - let mut method = method - .try_borrow_mut() - .map_err(|_| Error::RecursiveMutCallback)?; + let userdata = try_self_arg!(AnyUserData::from_lua(front, lua)); unsafe { let _sg = StackGuard::new(state); check_stack(state, 2)?; - let type_id = lua.push_userdata_ref(&userdata.0)?; + let type_id = try_self_arg!(lua.push_userdata_ref(&userdata.0)); match type_id { Some(id) if id == TypeId::of::() => { - let mut ud = get_userdata_mut::(state)?; - method(lua, &mut ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let mut ud = try_self_arg!(get_userdata_mut::(state)); + call(&mut ud) } #[cfg(not(feature = "send"))] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_mut::>>(state)?; - let mut ud = ud - .try_borrow_mut() - .map_err(|_| Error::UserDataBorrowMutError)?; - method(lua, &mut ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = try_self_arg!(get_userdata_mut::>>(state)); + let mut ud = + try_self_arg!(ud.try_borrow_mut(), Error::UserDataBorrowMutError); + call(&mut ud) } Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_mut::>>(state)?; + let ud = try_self_arg!(get_userdata_mut::>>(state)); let mut ud = - ud.try_lock().map_err(|_| Error::UserDataBorrowMutError)?; - method(lua, &mut ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + try_self_arg!(ud.try_lock(), Error::UserDataBorrowMutError); + call(&mut ud) } #[cfg(feature = "parking_lot")] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_mut::>>(state)?; - let mut ud = ud.try_lock().ok_or(Error::UserDataBorrowMutError)?; - method(lua, &mut ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = get_userdata_mut::>>(state); + let ud = try_self_arg!(ud); + let mut ud = + try_self_arg!(ud.try_lock().ok_or(Error::UserDataBorrowMutError)); + call(&mut ud) } Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_mut::>>(state)?; + let ud = try_self_arg!(get_userdata_mut::>>(state)); let mut ud = - ud.try_write().map_err(|_| Error::UserDataBorrowMutError)?; - method(lua, &mut ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + try_self_arg!(ud.try_write(), Error::UserDataBorrowMutError); + call(&mut ud) } #[cfg(feature = "parking_lot")] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_mut::>>(state)?; - let mut ud = ud.try_write().ok_or(Error::UserDataBorrowMutError)?; - method(lua, &mut ud, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + let ud = get_userdata_mut::>>(state); + let ud = try_self_arg!(ud); + let mut ud = + try_self_arg!(ud.try_write().ok_or(Error::UserDataBorrowMutError)); + call(&mut ud) } - _ => Err(Error::UserDataTypeMismatch), + _ => Err(Error::bad_self_argument(&name, Error::UserDataTypeMismatch)), } } } else { - Err(Error::FromLuaConversionError { - from: "missing argument", - to: "userdata", - message: None, - }) + let err = Error::from_lua_conversion("missing argument", "userdata", None); + Err(Error::bad_self_argument(&name, err)) } }) } #[cfg(feature = "async")] - fn box_async_method(method: M) -> AsyncCallback<'lua, 'static> + fn box_async_method(name: &str, method: M) -> AsyncCallback<'lua, 'static> where T: Clone, M: Fn(&'lua Lua, T, A) -> MR + MaybeSend + 'static, @@ -199,58 +233,76 @@ impl<'lua, T: 'static> UserDataRegistrar<'lua, T> { MR: Future> + 'lua, R: IntoLuaMulti<'lua>, { + let name = get_function_name::(name); + macro_rules! try_self_arg { + ($res:expr) => { + $res.map_err(|err| Error::bad_self_argument(&name, err))? + }; + ($res:expr, $err:expr) => { + $res.map_err(|_| Error::bad_self_argument(&name, $err))? + }; + } + Box::new(move |lua, mut args| { + let front = args.pop_front(); + let call = |ud| { + // Self was at index 1, so we pass 2 here + let args = A::from_lua_multi_args(args, 2, Some(&name), lua)?; + Ok(method(lua, ud, args)) + }; + let fut_res = || { - if let Some(front) = args.pop_front() { + if let Some(front) = front { let state = lua.state(); let userdata = AnyUserData::from_lua(front, lua)?; unsafe { let _sg = StackGuard::new(state); check_stack(state, 2)?; - let type_id = lua.push_userdata_ref(&userdata.0)?; + let type_id = try_self_arg!(lua.push_userdata_ref(&userdata.0)); match type_id { Some(id) if id == TypeId::of::() => { let ud = get_userdata_ref::(state)?; - Ok(method(lua, ud.clone(), A::from_lua_multi(args, lua)?)) + call(ud.clone()) } #[cfg(not(feature = "send"))] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_borrow().map_err(|_| Error::UserDataBorrowError)?; - Ok(method(lua, ud.clone(), A::from_lua_multi(args, lua)?)) + let ud = try_self_arg!(get_userdata_ref::>>(state)); + let ud = try_self_arg!(ud.try_borrow(), Error::UserDataBorrowError); + call(ud.clone()) } Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_lock().map_err(|_| Error::UserDataBorrowError)?; - Ok(method(lua, ud.clone(), A::from_lua_multi(args, lua)?)) + let ud = try_self_arg!(get_userdata_ref::>>(state)); + let ud = try_self_arg!(ud.try_lock(), Error::UserDataBorrowError); + call(ud.clone()) } #[cfg(feature = "parking_lot")] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_lock().ok_or(Error::UserDataBorrowError)?; - Ok(method(lua, ud.clone(), A::from_lua_multi(args, lua)?)) + let ud = get_userdata_ref::>>(state); + let ud = try_self_arg!(ud); + let ud = + try_self_arg!(ud.try_lock().ok_or(Error::UserDataBorrowError)); + call(ud.clone()) } Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_read().map_err(|_| Error::UserDataBorrowError)?; - Ok(method(lua, ud.clone(), A::from_lua_multi(args, lua)?)) + let ud = try_self_arg!(get_userdata_ref::>>(state)); + let ud = try_self_arg!(ud.try_read(), Error::UserDataBorrowError); + call(ud.clone()) } #[cfg(feature = "parking_lot")] Some(id) if id == TypeId::of::>>() => { - let ud = get_userdata_ref::>>(state)?; - let ud = ud.try_read().ok_or(Error::UserDataBorrowError)?; - Ok(method(lua, ud.clone(), A::from_lua_multi(args, lua)?)) + let ud = get_userdata_ref::>>(state); + let ud = try_self_arg!(ud); + let ud = + try_self_arg!(ud.try_read().ok_or(Error::UserDataBorrowError)); + call(ud.clone()) } - _ => Err(Error::UserDataTypeMismatch), + _ => Err(Error::bad_self_argument(&name, Error::UserDataTypeMismatch)), } } } else { - Err(Error::FromLuaConversionError { - from: "missing argument", - to: "userdata", - message: None, - }) + let err = Error::from_lua_conversion("missing argument", "userdata", None); + Err(Error::bad_self_argument(&name, err)) } }; match fut_res() { @@ -262,40 +314,45 @@ impl<'lua, T: 'static> UserDataRegistrar<'lua, T> { }) } - fn box_function(function: F) -> Callback<'lua, 'static> + fn box_function(name: &str, function: F) -> Callback<'lua, 'static> where F: Fn(&'lua Lua, A) -> Result + MaybeSend + 'static, A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { - Box::new(move |lua, args| function(lua, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua)) + let name = get_function_name::(name); + Box::new(move |lua, args| { + function(lua, A::from_lua_multi_args(args, 1, Some(&name), lua)?)?.into_lua_multi(lua) + }) } - fn box_function_mut(function: F) -> Callback<'lua, 'static> + fn box_function_mut(name: &str, function: F) -> Callback<'lua, 'static> where F: FnMut(&'lua Lua, A) -> Result + MaybeSend + 'static, A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = get_function_name::(name); let function = RefCell::new(function); Box::new(move |lua, args| { let function = &mut *function .try_borrow_mut() .map_err(|_| Error::RecursiveMutCallback)?; - function(lua, A::from_lua_multi(args, lua)?)?.into_lua_multi(lua) + function(lua, A::from_lua_multi_args(args, 1, Some(&name), lua)?)?.into_lua_multi(lua) }) } #[cfg(feature = "async")] - fn box_async_function(function: F) -> AsyncCallback<'lua, 'static> + fn box_async_function(name: &str, function: F) -> AsyncCallback<'lua, 'static> where F: Fn(&'lua Lua, A) -> FR + MaybeSend + 'static, A: FromLuaMulti<'lua>, FR: Future> + 'lua, R: IntoLuaMulti<'lua>, { + let name = get_function_name::(name); Box::new(move |lua, args| { - let args = match A::from_lua_multi(args, lua) { + let args = match A::from_lua_multi_args(args, 1, Some(&name), lua) { Ok(args) => args, Err(e) => return Box::pin(future::err(e)), }; @@ -306,14 +363,21 @@ impl<'lua, T: 'static> UserDataRegistrar<'lua, T> { } } +// Returns function name for the type `T`, without the module path +fn get_function_name(name: &str) -> StdString { + let type_name = any::type_name::().rsplitn(2, "::").next().unwrap(); + format!("{type_name}.{name}",) +} + impl<'lua, T: 'static> UserDataFields<'lua, T> for UserDataRegistrar<'lua, T> { fn add_field_method_get(&mut self, name: impl AsRef, method: M) where M: Fn(&'lua Lua, &T) -> Result + MaybeSend + 'static, R: IntoLua<'lua>, { - let method = Self::box_method(move |lua, data, ()| method(lua, data)); - self.field_getters.push((name.as_ref().into(), method)); + let name = name.as_ref(); + let method = Self::box_method(name, move |lua, data, ()| method(lua, data)); + self.field_getters.push((name.into(), method)); } fn add_field_method_set(&mut self, name: impl AsRef, method: M) @@ -321,8 +385,9 @@ impl<'lua, T: 'static> UserDataFields<'lua, T> for UserDataRegistrar<'lua, T> { M: FnMut(&'lua Lua, &mut T, A) -> Result<()> + MaybeSend + 'static, A: FromLua<'lua>, { - let method = Self::box_method_mut(method); - self.field_setters.push((name.as_ref().into(), method)); + let name = name.as_ref(); + let method = Self::box_method_mut(name, method); + self.field_setters.push((name.into(), method)); } fn add_field_function_get(&mut self, name: impl AsRef, function: F) @@ -330,8 +395,9 @@ impl<'lua, T: 'static> UserDataFields<'lua, T> for UserDataRegistrar<'lua, T> { F: Fn(&'lua Lua, AnyUserData<'lua>) -> Result + MaybeSend + 'static, R: IntoLua<'lua>, { - let func = Self::box_function(function); - self.field_getters.push((name.as_ref().into(), func)); + let name = name.as_ref(); + let func = Self::box_function(name, function); + self.field_getters.push((name.into(), func)); } fn add_field_function_set(&mut self, name: impl AsRef, mut function: F) @@ -339,8 +405,9 @@ impl<'lua, T: 'static> UserDataFields<'lua, T> for UserDataRegistrar<'lua, T> { F: FnMut(&'lua Lua, AnyUserData<'lua>, A) -> Result<()> + MaybeSend + 'static, A: FromLua<'lua>, { - let func = Self::box_function_mut(move |lua, (data, val)| function(lua, data, val)); - self.field_setters.push((name.as_ref().into(), func)); + let name = name.as_ref(); + let func = Self::box_function_mut(name, move |lua, (data, val)| function(lua, data, val)); + self.field_setters.push((name.into(), func)); } fn add_meta_field_with(&mut self, name: impl AsRef, f: F) @@ -358,7 +425,7 @@ impl<'lua, T: 'static> UserDataFields<'lua, T> for UserDataRegistrar<'lua, T> { Value::Nil | Value::Table(_) | Value::Function(_) => {} _ => { return Err(Error::MetaMethodTypeError { - method: name.to_string(), + method: name.clone(), type_name: value.type_name(), message: Some("expected nil, table or function".to_string()), }) @@ -388,8 +455,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.methods - .push((name.as_ref().into(), Self::box_method(method))); + .push((name.into(), Self::box_method(name, method))); } fn add_method_mut(&mut self, name: impl AsRef, method: M) @@ -398,8 +466,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.methods - .push((name.as_ref().into(), Self::box_method_mut(method))); + .push((name.into(), Self::box_method_mut(name, method))); } #[cfg(feature = "async")] @@ -411,8 +480,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { MR: Future> + 'lua, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.async_methods - .push((name.as_ref().into(), Self::box_async_method(method))); + .push((name.into(), Self::box_async_method(name, method))); } fn add_function(&mut self, name: impl AsRef, function: F) @@ -421,8 +491,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.methods - .push((name.as_ref().into(), Self::box_function(function))); + .push((name.into(), Self::box_function(name, function))); } fn add_function_mut(&mut self, name: impl AsRef, function: F) @@ -431,8 +502,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.methods - .push((name.as_ref().into(), Self::box_function_mut(function))); + .push((name.into(), Self::box_function_mut(name, function))); } #[cfg(feature = "async")] @@ -443,8 +515,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { FR: Future> + 'lua, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.async_methods - .push((name.as_ref().into(), Self::box_async_function(function))); + .push((name.into(), Self::box_async_function(name, function))); } fn add_meta_method(&mut self, name: impl AsRef, method: M) @@ -453,8 +526,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.meta_methods - .push((name.as_ref().into(), Self::box_method(method))); + .push((name.into(), Self::box_method(name, method))); } fn add_meta_method_mut(&mut self, name: impl AsRef, method: M) @@ -463,8 +537,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.meta_methods - .push((name.as_ref().into(), Self::box_method_mut(method))); + .push((name.into(), Self::box_method_mut(name, method))); } #[cfg(all(feature = "async", not(any(feature = "lua51", feature = "luau"))))] @@ -476,8 +551,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { MR: Future> + 'lua, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.async_meta_methods - .push((name.as_ref().into(), Self::box_async_method(method))); + .push((name.into(), Self::box_async_method(name, method))); } fn add_meta_function(&mut self, name: impl AsRef, function: F) @@ -486,8 +562,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.meta_methods - .push((name.as_ref().into(), Self::box_function(function))); + .push((name.into(), Self::box_function(name, function))); } fn add_meta_function_mut(&mut self, name: impl AsRef, function: F) @@ -496,8 +573,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { A: FromLuaMulti<'lua>, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.meta_methods - .push((name.as_ref().into(), Self::box_function_mut(function))); + .push((name.into(), Self::box_function_mut(name, function))); } #[cfg(all(feature = "async", not(any(feature = "lua51", feature = "luau"))))] @@ -508,8 +586,9 @@ impl<'lua, T: 'static> UserDataMethods<'lua, T> for UserDataRegistrar<'lua, T> { FR: Future> + 'lua, R: IntoLuaMulti<'lua>, { + let name = name.as_ref(); self.async_meta_methods - .push((name.as_ref().into(), Self::box_async_function(function))); + .push((name.into(), Self::box_async_function(name, function))); } // Below are internal methods used in generated code diff --git a/src/value.rs b/src/value.rs index bfd8dcf..91b9d19 100644 --- a/src/value.rs +++ b/src/value.rs @@ -1,6 +1,7 @@ use std::iter::{self, FromIterator}; use std::ops::Index; use std::os::raw::c_void; +use std::sync::Arc; use std::{ptr, slice, str, vec}; #[cfg(feature = "serialize")] @@ -93,7 +94,7 @@ impl<'lua> Value<'lua> { match (self, other.as_ref()) { (Value::Table(a), Value::Table(b)) => a.equals(b), (Value::UserData(a), Value::UserData(b)) => a.equals(b), - _ => Ok(self == other.as_ref()), + (a, b) => Ok(a == b), } } @@ -162,8 +163,7 @@ impl<'lua> Serialize for Value<'lua> { Value::Boolean(b) => serializer.serialize_bool(*b), #[allow(clippy::useless_conversion)] Value::Integer(i) => serializer - .serialize_i64((*i).try_into().expect("cannot convert lua_Integer to i64")), - #[allow(clippy::useless_conversion)] + .serialize_i64((*i).try_into().expect("cannot convert Lua Integer to i64")), Value::Number(n) => serializer.serialize_f64(*n), #[cfg(feature = "luau")] Value::Vector(x, y, z) => (x, y, z).serialize(serializer), @@ -188,7 +188,26 @@ pub trait IntoLua<'lua> { /// Trait for types convertible from `Value`. pub trait FromLua<'lua>: Sized { /// Performs the conversion. - fn from_lua(lua_value: Value<'lua>, lua: &'lua Lua) -> Result; + fn from_lua(value: Value<'lua>, lua: &'lua Lua) -> Result; + + /// Performs the conversion for an argument (eg. function argument). + /// + /// `i` is the argument index (position), + /// `to` is a function name that received the argument. + #[doc(hidden)] + fn from_lua_arg( + value: Value<'lua>, + i: usize, + to: Option<&str>, + lua: &'lua Lua, + ) -> Result { + Self::from_lua(value, lua).map_err(|err| Error::BadArgument { + to: to.map(|s| s.to_string()), + pos: i, + name: None, + error: Arc::new(err), + }) + } } /// Multiple Lua values used for both argument passing and also for multiple return values. @@ -362,6 +381,22 @@ pub trait FromLuaMulti<'lua>: Sized { /// assigning values. Similarly, if not enough values are given, conversions should assume that /// any missing values are nil. fn from_lua_multi(values: MultiValue<'lua>, lua: &'lua Lua) -> Result; + + /// Performs the conversion for a list of arguments. + /// + /// `i` is an index (position) of the first argument, + /// `to` is a function name that received the arguments. + #[doc(hidden)] + #[inline] + fn from_lua_multi_args( + values: MultiValue<'lua>, + i: usize, + to: Option<&str>, + lua: &'lua Lua, + ) -> Result { + let _ = (i, to); + Self::from_lua_multi(values, lua) + } } #[cfg(test)] diff --git a/tests/userdata.rs b/tests/userdata.rs index 718a736..15d27f8 100644 --- a/tests/userdata.rs +++ b/tests/userdata.rs @@ -791,3 +791,32 @@ fn test_userdata_ext() -> Result<()> { Ok(()) } + +#[test] +fn test_userdata_method_errors() -> Result<()> { + struct MyUserData(i64); + + impl UserData for MyUserData { + fn add_methods<'lua, M: UserDataMethods<'lua, Self>>(methods: &mut M) { + methods.add_method("get_value", |_, data, ()| Ok(data.0)); + } + } + + let lua = Lua::new(); + + let ud = lua.create_userdata(MyUserData(123))?; + let res = ud.call_function::<_, ()>("get_value", ()); + let Err(Error::CallbackError { cause, .. }) = res else { + panic!("expected CallbackError, got {res:?}"); + }; + assert!(matches!( + &*cause, + Error::BadArgument { + to, + name, + .. + } if to.as_deref() == Some("MyUserData.get_value") && name.as_deref() == Some("self") + )); + + Ok(()) +}