From 04d81066765b2e8158cb8be3450bc3beb5e66246 Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Wed, 2 Oct 2024 12:16:48 +0100 Subject: [PATCH] Remove `SubtypeId` from `AnyUserData` and instead add `Value::Other` variant that will cover any unknown types (eg. LuaJIT CData) --- src/scope.rs | 4 ++-- src/serde/de.rs | 3 ++- src/state/raw.rs | 15 ++++++--------- src/types.rs | 8 -------- src/userdata.rs | 10 ++-------- src/userdata/object.rs | 2 +- src/util/mod.rs | 7 ++++--- src/value.rs | 31 +++++++++++++------------------ tests/tests.rs | 3 +-- 9 files changed, 31 insertions(+), 52 deletions(-) diff --git a/src/scope.rs b/src/scope.rs index 57f6598..725cd62 100644 --- a/src/scope.rs +++ b/src/scope.rs @@ -6,7 +6,7 @@ use std::os::raw::c_void; use crate::error::{Error, Result}; use crate::function::Function; use crate::state::{Lua, LuaGuard, RawLua}; -use crate::types::{Callback, CallbackUpvalue, ScopedCallback, SubtypeId, ValueRef}; +use crate::types::{Callback, CallbackUpvalue, ScopedCallback, ValueRef}; use crate::userdata::{AnyUserData, UserData, UserDataRegistry, UserDataStorage}; use crate::util::{self, assert_stack, check_stack, get_userdata, take_userdata, StackGuard}; use crate::value::{FromLuaMulti, IntoLuaMulti}; @@ -186,7 +186,7 @@ impl<'scope, 'env: 'scope> Scope<'scope, 'env> { std::ptr::write(ud_ptr, UserDataStorage::new_scoped(data)); ffi::lua_setmetatable(state, -2); - let ud = AnyUserData(self.lua.pop_ref(), SubtypeId::None); + let ud = AnyUserData(self.lua.pop_ref()); let destructor: DestructorCallback = Box::new(|rawlua, vref| { let state = rawlua.state(); diff --git a/src/serde/de.rs b/src/serde/de.rs index 6d3d513..4e4da58 100644 --- a/src/serde/de.rs +++ b/src/serde/de.rs @@ -150,7 +150,8 @@ impl<'de> serde::Deserializer<'de> for Deserializer { | Value::Thread(_) | Value::UserData(_) | Value::LightUserData(_) - | Value::Error(_) => { + | Value::Error(_) + | Value::Other(_) => { if self.options.deny_unsupported_types { let msg = format!("unsupported value type `{}`", self.value.type_name()); Err(de::Error::custom(msg)) diff --git a/src/state/raw.rs b/src/state/raw.rs index ae8095e..36a54f9 100644 --- a/src/state/raw.rs +++ b/src/state/raw.rs @@ -18,7 +18,7 @@ use crate::table::Table; use crate::thread::Thread; use crate::types::{ AppDataRef, AppDataRefMut, Callback, CallbackUpvalue, DestructedUserdata, Integer, LightUserData, - MaybeSend, ReentrantMutex, RegistryKey, SubtypeId, ValueRef, XRc, + MaybeSend, ReentrantMutex, RegistryKey, ValueRef, XRc, }; use crate::userdata::{AnyUserData, MetaMethod, UserData, UserDataRegistry, UserDataStorage}; use crate::util::{ @@ -560,6 +560,7 @@ impl RawLua { let protect = !self.unlikely_memory_error(); push_internal_userdata(state, WrappedFailure::Error(*err.clone()), protect)?; } + Value::Other(vref) => self.push_ref(vref), } Ok(()) } @@ -644,7 +645,7 @@ impl RawLua { } _ => { ffi::lua_xpush(state, self.ref_thread(), idx); - Value::UserData(AnyUserData(self.pop_ref_thread(), SubtypeId::None)) + Value::UserData(AnyUserData(self.pop_ref_thread())) } } } @@ -661,14 +662,10 @@ impl RawLua { Value::Buffer(crate::Buffer(self.pop_ref_thread())) } - #[cfg(feature = "luajit")] - ffi::LUA_TCDATA => { - // CData is represented as a userdata type + _ => { ffi::lua_xpush(state, self.ref_thread(), idx); - Value::UserData(AnyUserData(self.pop_ref_thread(), SubtypeId::CData)) + Value::Other(self.pop_ref_thread()) } - - _ => mlua_panic!("unexpected value type on stack"), } } @@ -806,7 +803,7 @@ impl RawLua { ffi::lua_setuservalue(state, -2); } - Ok(AnyUserData(self.pop_ref(), SubtypeId::None)) + Ok(AnyUserData(self.pop_ref())) } pub(crate) unsafe fn create_userdata_metatable( diff --git a/src/types.rs b/src/types.rs index dee5631..12f4083 100644 --- a/src/types.rs +++ b/src/types.rs @@ -28,14 +28,6 @@ pub type Integer = ffi::lua_Integer; /// Type of Lua floating point numbers. pub type Number = ffi::lua_Number; -// Represents different subtypes wrapped in AnyUserData -#[derive(Debug, Copy, Clone, Eq, PartialEq)] -pub(crate) enum SubtypeId { - None, - #[cfg(feature = "luajit")] - CData, -} - /// A "light" userdata value. Equivalent to an unmanaged raw pointer. #[derive(Debug, Copy, Clone, Eq, PartialEq)] pub struct LightUserData(pub *mut c_void); diff --git a/src/userdata.rs b/src/userdata.rs index e57e063..71995eb 100644 --- a/src/userdata.rs +++ b/src/userdata.rs @@ -19,7 +19,7 @@ use crate::function::Function; use crate::state::Lua; use crate::string::String; use crate::table::{Table, TablePairs}; -use crate::types::{MaybeSend, SubtypeId, ValueRef}; +use crate::types::{MaybeSend, ValueRef}; use crate::util::{check_stack, get_userdata, take_userdata, StackGuard}; use crate::value::{FromLua, FromLuaMulti, IntoLua, IntoLuaMulti, Value}; @@ -643,7 +643,7 @@ pub trait UserData: Sized { /// [`is`]: crate::AnyUserData::is /// [`borrow`]: crate::AnyUserData::borrow #[derive(Clone, Debug)] -pub struct AnyUserData(pub(crate) ValueRef, pub(crate) SubtypeId); +pub struct AnyUserData(pub(crate) ValueRef); impl AnyUserData { /// Checks whether the type of this userdata is `T`. @@ -935,12 +935,6 @@ impl AnyUserData { /// Returns a type name of this `UserData` (from a metatable field). pub(crate) fn type_name(&self) -> Result> { - match self.1 { - SubtypeId::None => {} - #[cfg(feature = "luajit")] - SubtypeId::CData => return Ok(Some("cdata".to_owned())), - } - let lua = self.0.lua.lock(); let state = lua.state(); unsafe { diff --git a/src/userdata/object.rs b/src/userdata/object.rs index faa68f7..2b9597f 100644 --- a/src/userdata/object.rs +++ b/src/userdata/object.rs @@ -88,6 +88,6 @@ impl ObjectLike for AnyUserData { #[inline] fn to_string(&self) -> Result { - Value::UserData(AnyUserData(self.0.copy(), self.1)).to_string() + Value::UserData(AnyUserData(self.0.copy())).to_string() } } diff --git a/src/util/mod.rs b/src/util/mod.rs index 9722fbc..148e1c8 100644 --- a/src/util/mod.rs +++ b/src/util/mod.rs @@ -275,9 +275,10 @@ pub(crate) unsafe fn to_string(state: *mut ffi::lua_State, index: c_int) -> Stri ffi::LUA_TTHREAD => format!("", ffi::lua_topointer(state, index)), #[cfg(feature = "luau")] ffi::LUA_TBUFFER => format!("", ffi::lua_topointer(state, index)), - #[cfg(feature = "luajit")] - ffi::LUA_TCDATA => format!("", ffi::lua_topointer(state, index)), - _ => "".to_string(), + type_id => { + let type_name = CStr::from_ptr(ffi::lua_typename(state, type_id)).to_string_lossy(); + format!("<{type_name} {:?}>", ffi::lua_topointer(state, index)) + } } } diff --git a/src/value.rs b/src/value.rs index 50a1bd4..4d855e7 100644 --- a/src/value.rs +++ b/src/value.rs @@ -14,7 +14,7 @@ use crate::state::{Lua, RawLua}; use crate::string::{BorrowedStr, String}; use crate::table::Table; use crate::thread::Thread; -use crate::types::{Integer, LightUserData, Number, SubtypeId, ValueRef}; +use crate::types::{Integer, LightUserData, Number, ValueRef}; use crate::userdata::AnyUserData; use crate::util::{check_stack, StackGuard}; @@ -28,7 +28,7 @@ use { /// A dynamically typed Lua value. /// -/// The `String`, `Table`, `Function`, `Thread`, and `UserData` variants contain handle types +/// The non-primitive variants (eg. string/table/function/thread/userdata) contain handle types /// into the internal Lua state. It is a logic error to mix handle types between separate /// `Lua` instances, and doing so will result in a panic. #[derive(Clone)] @@ -69,6 +69,9 @@ pub enum Value { Buffer(crate::Buffer), /// `Error` is a special builtin userdata type. When received from Lua it is implicitly cloned. Error(Box), + /// Any other value not known to mlua (eg. LuaJIT CData). + #[allow(private_interfaces)] + Other(ValueRef), } pub use self::Value::Nil; @@ -93,12 +96,11 @@ impl Value { Value::Table(_) => "table", Value::Function(_) => "function", Value::Thread(_) => "thread", - Value::UserData(AnyUserData(_, SubtypeId::None)) => "userdata", - #[cfg(feature = "luajit")] - Value::UserData(AnyUserData(_, SubtypeId::CData)) => "cdata", + Value::UserData(_) => "userdata", #[cfg(feature = "luau")] Value::Buffer(_) => "buffer", Value::Error(_) => "error", + Value::Other(_) => "other", } } @@ -173,7 +175,8 @@ impl Value { Value::Table(Table(vref)) | Value::Function(Function(vref)) | Value::Thread(Thread(vref, ..)) - | Value::UserData(AnyUserData(vref, ..)) => unsafe { invoke_to_string(vref) }, + | Value::UserData(AnyUserData(vref)) + | Value::Other(vref) => unsafe { invoke_to_string(vref) }, #[cfg(feature = "luau")] Value::Buffer(crate::Buffer(vref)) => unsafe { invoke_to_string(vref) }, Value::Error(err) => Ok(err.to_string()), @@ -448,17 +451,6 @@ impl Value { self.as_buffer().is_some() } - /// Returns `true` if the value is a CData wrapped in [`AnyUserData`]. - #[cfg(any(feature = "luajit", doc))] - #[cfg_attr(docsrs, doc(cfg(feature = "luajit")))] - #[doc(hidden)] - #[inline] - pub fn is_cdata(&self) -> bool { - self.as_userdata() - .map(|ud| ud.1 == SubtypeId::CData) - .unwrap_or_default() - } - /// Wrap reference to this Value into [`SerializableValue`]. /// /// This allows customizing serialization behavior using serde. @@ -548,6 +540,7 @@ impl Value { buf @ Value::Buffer(_) => write!(fmt, "buffer: {:?}", buf.to_pointer()), Value::Error(e) if recursive => write!(fmt, "{e:?}"), Value::Error(_) => write!(fmt, "error"), + Value::Other(v) => write!(fmt, "other: {:?}", v.to_pointer()), } } } @@ -574,6 +567,7 @@ impl fmt::Debug for Value { #[cfg(feature = "luau")] Value::Buffer(buf) => write!(fmt, "{buf:?}"), Value::Error(e) => write!(fmt, "Error({e:?})"), + Value::Other(v) => write!(fmt, "Other({v:?})"), } } } @@ -711,7 +705,8 @@ impl<'a> Serialize for SerializableValue<'a> { | Value::Thread(_) | Value::UserData(_) | Value::LightUserData(_) - | Value::Error(_) => { + | Value::Error(_) + | Value::Other(_) => { if self.options.deny_unsupported_types { let msg = format!("cannot serialize <{}>", self.value.type_name()); Err(ser::Error::custom(msg)) diff --git a/tests/tests.rs b/tests/tests.rs index fb6e893..a819d06 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -1256,8 +1256,7 @@ fn test_luajit_cdata() -> Result<()> { "#, ) .eval::()?; - assert!(cdata.is_userdata() && cdata.is_cdata()); - assert_eq!(cdata.type_name(), "cdata"); + assert_eq!(cdata.type_name(), "other"); assert!(cdata.to_string()?.starts_with("cdata:")); Ok(())