Update Table::set_metatable

- Return Err (instead of panic) when trying to change readonly table (Luau)
- Slightly optimize performance

Author: khvzak

mlua_derive/src/lib.rs

@@ -129,13 +129,13 @@ pub fn chunk(input: TokenStream) -> TokenStream {
             let globals = lua.globals();
             let env = lua.create_table()?;
             let meta = lua.create_table()?;
-            meta.raw_set("__index", globals.clone())?;
-            meta.raw_set("__newindex", globals)?;
+            meta.raw_set("__index", &globals)?;
+            meta.raw_set("__newindex", &globals)?;
 
             // Add captured variables
             #(#caps)*
 
-            env.set_metatable(Some(meta));
+            env.set_metatable(Some(meta))?;
             Ok(env)
         };
 

src/serde/ser.rs

@@ -256,7 +256,7 @@ impl<'a> ser::Serializer for Serializer<'a> {
     fn serialize_seq(self, len: Option<usize>) -> Result<Self::SerializeSeq> {
         let table = self.lua.create_table_with_capacity(len.unwrap_or(0), 0)?;
         if self.options.set_array_metatable {
-            table.set_metatable(Some(self.lua.array_metatable()));
+            table.set_metatable(Some(self.lua.array_metatable()))?;
         }
         Ok(SerializeSeq::new(self.lua, table, self.options))
     }

src/table.rs

@@ -211,7 +211,7 @@ impl Table {
     ///
     /// let always_equals_mt = lua.create_table()?;
     /// always_equals_mt.set("__eq", lua.create_function(|_, (_t1, _t2): (Table, Table)| Ok(true))?)?;
-    /// table2.set_metatable(Some(always_equals_mt));
+    /// table2.set_metatable(Some(always_equals_mt))?;
     ///
     /// assert!(table1.equals(&table1.clone())?);
     /// assert!(table1.equals(&table2)?);
@@ -501,27 +501,23 @@ impl Table {
     ///
     /// If `metatable` is `None`, the metatable is removed (if no metatable is set, this does
     /// nothing).
-    pub fn set_metatable(&self, metatable: Option<Table>) {
-        // Workaround to throw readonly error without returning Result
+    pub fn set_metatable(&self, metatable: Option<Table>) -> Result<()> {
         #[cfg(feature = "luau")]
         if self.is_readonly() {
-            panic!("attempt to modify a readonly table");
+            return Err(Error::runtime("attempt to modify a readonly table"));
         }
 
         let lua = self.0.lua.lock();
-        let state = lua.state();
+        let ref_thread = lua.ref_thread();
         unsafe {
-            let _sg = StackGuard::new(state);
-            assert_stack(state, 2);
-
-            lua.push_ref(&self.0);
             if let Some(metatable) = metatable {
-                lua.push_ref(&metatable.0);
+                ffi::lua_pushvalue(ref_thread, metatable.0.index);
             } else {
-                ffi::lua_pushnil(state);
+                ffi::lua_pushnil(ref_thread);
             }
-            ffi::lua_setmetatable(state, -2);
+            ffi::lua_setmetatable(ref_thread, self.0.index);
         }
+        Ok(())
     }
 
     /// Returns true if the table has metatable attached.

tests/async.rs

@@ -386,7 +386,7 @@ async fn test_async_table_object_like() -> Result<()> {
             table.get::<i64>("val")
         })?,
     )?;
-    table.set_metatable(Some(metatable));
+    table.set_metatable(Some(metatable))?;
     assert_eq!(table.call_async::<i64>(()).await.unwrap(), 15);
 
     match table.call_async_method::<()>("non_existent", ()).await {

tests/luau.rs

@@ -3,7 +3,6 @@
 use std::cell::Cell;
 use std::fmt::Debug;
 use std::os::raw::c_void;
-use std::panic::{catch_unwind, AssertUnwindSafe};
 use std::sync::atomic::{AtomicBool, AtomicPtr, AtomicU64, Ordering};
 use std::sync::Arc;
 
@@ -120,7 +119,7 @@ fn test_vector_metatable() -> Result<()> {
     "#,
         )
         .eval::<Table>()?;
-    vector_mt.set_metatable(Some(vector_mt.clone()));
+    vector_mt.set_metatable(Some(vector_mt.clone()))?;
     lua.set_type_metatable::<Vector>(Some(vector_mt.clone()));
     lua.globals().set("Vector3", vector_mt)?;
 
@@ -167,9 +166,9 @@ fn test_readonly_table() -> Result<()> {
     check_readonly_error(t.raw_pop::<Value>());
 
     // Special case
-    match catch_unwind(AssertUnwindSafe(|| t.set_metatable(None))) {
-        Ok(_) => panic!("expected panic, got nothing"),
-        Err(_) => {}
+    match t.set_metatable(None) {
+        Err(Error::RuntimeError(e)) if e.contains("attempt to modify a readonly table") => {}
+        r => panic!("expected RuntimeError(...) with a specific message, got {r:?}"),
     }
 
     Ok(())

tests/serde.rs

@@ -25,7 +25,7 @@ fn test_serialize() -> Result<(), Box<dyn StdError>> {
     globals.set("null", lua.null())?;
 
     let empty_array = lua.create_table()?;
-    empty_array.set_metatable(Some(lua.array_metatable()));
+    empty_array.set_metatable(Some(lua.array_metatable()))?;
     globals.set("empty_array", empty_array)?;
 
     let val = lua
@@ -173,7 +173,7 @@ fn test_serialize_sorted() -> LuaResult<()> {
     globals.set("null", lua.null())?;
 
     let empty_array = lua.create_table()?;
-    empty_array.set_metatable(Some(lua.array_metatable()));
+    empty_array.set_metatable(Some(lua.array_metatable()))?;
     globals.set("empty_array", empty_array)?;
 
     let value = lua

tests/table.rs

@@ -298,10 +298,10 @@ fn test_metatable() -> Result<()> {
     let table = lua.create_table()?;
     let metatable = lua.create_table()?;
     metatable.set("__index", lua.create_function(|_, ()| Ok("index_value"))?)?;
-    table.set_metatable(Some(metatable));
+    table.set_metatable(Some(metatable))?;
     assert_eq!(table.get::<String>("any_key")?, "index_value");
     assert_eq!(table.raw_get::<Value>("any_key")?, Value::Nil);
-    table.set_metatable(None);
+    table.set_metatable(None)?;
     assert_eq!(table.get::<Value>("any_key")?, Value::Nil);
 
     Ok(())