Improve error messages when collecting function arguments for execution

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<String>,
         arg_index: i32,
         arg_name: Option<String>,
         /// the error that describes what was wrong for this argument
+        #[source]
         error: Arc<Error>
     },
     #[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<Error>,
+    },
 }
 
 impl Error {
@@ -219,6 +232,20 @@ impl<'a> PushToLuaStack<'a> for () {
     }
 }
 
+impl<'a, T: PushToLuaStack<'a>> PushToLuaStack<'a> for Option<T> {
+    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<T: Userdata>(PhantomData<T>);
@@ -355,12 +403,14 @@ impl<T: Userdata> Userdata for UserdataProxy<T> {
         let mut other = UserdataBuilder::<T>::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
+    }
 }

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<const N: usize> From<&[StdLibrary; N]> for StdLibraries {
     }
 }
 
+#[derive(Default)]
+struct ExtraSpace {
+    userdata_metatables: HashMap<TypeId, LuaRef>,
+}
+
 pub struct State {
     lua: NonNull<lua::lua_State>,
 }
 
 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::<ExtraSpace>())
+                .cast::<ExtraSpace>();
+            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 {
+            let udmts = &mut self.get_extra_space().userdata_metatables;//.get(&TypeId::of::<T>())
-                lua::lua_pop(s, 1); // remove nil
 
-                // if the metatable is not made yet, make it
+            if !udmts.contains_key(&TypeId::of::<T>()) {
+                // create the userdata's metatable and store it in extra space
                 let mt = self.create_userdata_metatable::<T>()?;
                 mt.push_to_lua_stack(self)?;
-            } else if ty != lua::LUA_TTABLE {
+            } else {
-                return Err(Error::RegistryConflict(name.to_string()));
+                let mt = udmts.get(&TypeId::of::<T>()).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<Table<'a>> {
+        let extra = self.get_extra_space();
+        let mt = extra.userdata_metatables.get(&TypeId::of::<T>());
+
+        mt.map(|r| Table::with_ref(self, r.clone(), true).unwrap())
+    }
+
     pub(crate) fn create_userdata_metatable<'a, T: Userdata + 'static>(&'a self) -> Result<Table<'a>> {
         let mut builder = UserdataBuilder::<T>::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::<T>(), mt.lref.clone());
+
         Ok(mt)
     }
 }

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<dyn Fn(&'a State, ValueVec<'a>) -> crate::Result<Value<'a>>>;
 
 pub struct UserdataBuilder<'a, T> {
-    pub(crate) name: Option<String>,
+    pub(crate) name: String,
     pub(crate) field_getters: HashMap<String, UserdataFn<'a>>,
     pub(crate) field_setters: HashMap<String, UserdataFn<'a>>,
     pub(crate) functions: HashMap<String, UserdataFn<'a>>,
@@ -103,10 +103,10 @@ pub struct UserdataBuilder<'a, T> {
     _marker: PhantomData<T>,
 }
 
-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<F, R>(&mut self, name: &str, f: F) -> &mut Self
     where
         F: Fn(&'a State, &T) -> crate::Result<R> + '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<R>(res: crate::Result<R>, ud_name: &str, fn_name: &str) -> crate::Result<R> {
+        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<F, R, A>(&mut self, name: &str, f: F) -> &mut Self
     where
         F: Fn(&'a State, A) -> crate::Result<R> + '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<R> + '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::<T>()?;
-
+            
-            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<R> + '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<T>(&self) -> crate::Result<&'a T> {
+    pub fn as_ref<T: Userdata + 'static>(&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::<T>().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::<T>()
+                .push_to_lua_stack(self.state)?;
+
+            if lua::lua_rawequal(s, -2, -1) == 1 {
+                let cptr = lua::lua_touserdata(s, -3);
+                Ok(cptr.cast::<T>().as_ref().unwrap()) // TODO: Ensure this userdata matches the type of T
+            } else {
+                return Err(crate::Error::UserdataMismatch);
+            }
         }
     }
 
-    pub fn as_mut<T>(&self) -> crate::Result<&'a mut T> {
+    pub fn as_mut<T: Userdata + 'static>(&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::<T>().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::<T>()
+                .push_to_lua_stack(self.state)?;
+
+            if lua::lua_rawequal(s, -2, -1) == 1 {
+                let cptr = lua::lua_touserdata(s, -3);
+                Ok(cptr.cast::<T>().as_mut().unwrap())
+            } else {
+                Err(crate::Error::UserdataMismatch)
+            }
+        }
+    }
+
+    /// Returns the name of the userdata by accessing the metatable
+    pub fn name(&self) -> crate::Result<String> {
+        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<Self> {
         let ud = val.into_userdata()?;
         ud.as_ref::<T>()
     }
 }
 
-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<Self> {
         let ud = val.into_userdata()?;
         ud.as_mut::<T>()

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::None => "None".to_string(),
-            Value::Nil => "Nil",
+            Value::Nil => "Nil".to_string(),
-            Value::Number(_) => "Number",
+            Value::Number(_) => "Number".to_string(),
-            Value::String(_) => "String",
+            Value::String(_) => "String".to_string(),
-            Value::Function(_) => "Function",
+            Value::Function(_) => "Function".to_string(),
-            Value::Table(_) => "Table",
+            Value::Table(_) => "Table".to_string(),
-            Value::Userdata(_) => todo!(),
+            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<String> {
+        let mut vec = VecDeque::new();
+
+        for val in self.0.iter() {
+            vec.push_back(val.type_name().to_string());
+        }
+
+        vec
+    }
 }
 
 impl<'a> From<Value<'a>> 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 f = $first::from_lua(state, values.pop_front().unwrap())
-                let ($( $name, )+) = ( $( $name::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,) )
             }
         }
     };