Improve code quality (#243)

crates/lune-std-ffi/README.md

@@ -7,7 +7,6 @@ See [tests/ffi](../../tests/ffi/README.md)
 ## TODO
 
 - Rewrite error messages
-- Deref
 - CString
 - Add buffer for owned data support
 - Add math operation.

crates/lune-std-ffi/src/c/arr_info.rs

@@ -9,12 +9,10 @@ use crate::ffi::{association, libffi_helper::get_ensured_size, FfiConvert, FfiDa
 // This is a series of some type.
 // It provides the final size and the offset of the index,
 // but does not allow multidimensional arrays because of API complexity.
-// However, multidimensional arrays are not impossible to implement
+// Multidimensional arrays can be implemented
 // because they are a series of transcribed one-dimensional arrays. (flatten)
-
 // We can simply provide array type with struct.
 // See: https://stackoverflow.com/a/43525176
-
 pub struct CArrInfo {
     struct_type: Type,
     length: usize,
@@ -33,7 +31,6 @@ impl CArrInfo {
         let struct_type = Type::structure(vec![element_type.clone(); length]);
 
         Ok(Self {
-            // element_type,
             struct_type,
             length,
             size: inner_size * length,
@@ -63,8 +60,8 @@ impl CArrInfo {
         self.struct_type.clone()
     }
 
-    // Stringify for pretty printing like:
-    // <CArr( u8, length = 8 )>
+    // Stringify for pretty-print
+    // ex: <CArr( u8, length = 8 )>
     pub fn stringify(lua: &Lua, userdata: &LuaAnyUserData) -> LuaResult<String> {
         let this = userdata.borrow::<CArrInfo>()?;
         if let Some(LuaValue::UserData(inner_userdata)) =
@@ -76,7 +73,7 @@ impl CArrInfo {
                 this.length,
             ))
         } else {
-            Err(LuaError::external("failed to get inner type userdata."))
+            Err(LuaError::external("Failed to retrieve inner type"))
         }
     }
 }
@@ -150,13 +147,11 @@ impl FfiConvert for CArrInfo {
 
 impl LuaUserData for CArrInfo {
     fn add_fields<'lua, F: LuaUserDataFields<'lua, Self>>(fields: &mut F) {
-        fields.add_field_method_get("size", |_, this| Ok(this.get_size()));
-        fields.add_field_method_get("length", |_, this| Ok(this.get_length()));
+        fields.add_field_method_get("size", |_lua, this| Ok(this.get_size()));
+        fields.add_field_method_get("length", |_lua, this| Ok(this.get_length()));
         fields.add_field_function_get("inner", |lua, this: LuaAnyUserData| {
-            let inner: LuaValue = association::get(lua, CARR_INNER, this)?
-                // It shouldn't happen.
-                .ok_or_else(|| LuaError::external("inner field not found"))?;
-            Ok(inner)
+            association::get(lua, CARR_INNER, this)?
+                .ok_or_else(|| LuaError::external("Failed to retrieve inner field"))
         });
     }
 
@@ -173,7 +168,7 @@ impl LuaUserData for CArrInfo {
         method_provider::provide_write_data(methods);
         method_provider::provide_copy_data(methods);
 
-        methods.add_method("offset", |_, this, offset: isize| {
+        methods.add_method("offset", |_lua, this, offset: isize| {
             if this.length > (offset as usize) && offset >= 0 {
                 Ok(this.inner_size * (offset as usize))
             } else {

crates/lune-std-ffi/src/c/fn_info.rs

@@ -67,7 +67,7 @@ fn create_arg_info(userdata: &LuaAnyUserData) -> LuaResult<FfiArg> {
     } else if userdata.is::<CFnInfo>() {
         CALLBACK_ARG_REF_FLAG_CFN
     } else {
-        return Err(LuaError::external("unexpected type userdata"));
+        return Err(LuaError::external("Unexpected argument type"));
     };
     Ok(FfiArg {
         size: helper::get_size(userdata)?,
@@ -116,8 +116,8 @@ impl CFnInfo {
         Ok(cfn)
     }
 
-    // Stringify for pretty printing like:
-    // <CFn( (u8, i32) -> u8 )>
+    // Stringify for pretty-print
+    // ex: <CFn( (u8, i32) -> u8 )>
     pub fn stringify(lua: &Lua, userdata: &LuaAnyUserData) -> LuaResult<String> {
         let mut result = String::from(" (");
         if let (Some(LuaValue::Table(arg_table)), Some(LuaValue::UserData(result_userdata))) = (
@@ -142,7 +142,7 @@ impl CFnInfo {
             );
             Ok(result)
         } else {
-            Err(LuaError::external("failed to get inner type userdata."))
+            Err(LuaError::external("Failed to retrieve inner type"))
         }
     }
 
@@ -173,12 +173,14 @@ impl CFnInfo {
         target_ref: &LuaAnyUserData,
     ) -> LuaResult<LuaAnyUserData<'lua>> {
         if !target_ref.is::<RefData>() {
-            return Err(LuaError::external("argument 0 must be ffiref"));
+            return Err(LuaError::external("Argument 'functionRef' must be RefData"));
         }
 
         let ffi_ref = target_ref.borrow::<RefData>()?;
         if u8_test_not(ffi_ref.flags, RefFlag::Function.value()) {
-            return Err(LuaError::external("not a function ref"));
+            return Err(LuaError::external(
+                "Argument 'functionRef' is not a valid function reference",
+            ));
         }
 
         let callable = lua.create_userdata(unsafe {
@@ -203,7 +205,7 @@ impl CFnInfo {
 
 impl LuaUserData for CFnInfo {
     fn add_fields<'lua, F: LuaUserDataFields<'lua, Self>>(fields: &mut F) {
-        fields.add_field_method_get("size", |_, _| Ok(SIZE_OF_POINTER));
+        fields.add_field_method_get("size", |_lua, _this| Ok(SIZE_OF_POINTER));
     }
     fn add_methods<'lua, M: LuaUserDataMethods<'lua, Self>>(methods: &mut M) {
         // Subtype

crates/lune-std-ffi/src/c/helper.rs

@@ -53,7 +53,7 @@ pub mod method_provider {
                     return Err(LuaError::external("Out of bounds"));
                 }
                 if !data_handle.is_readable() {
-                    return Err(LuaError::external("Unreadable data handle"));
+                    return Err(LuaError::external("Unreadable data"));
                 }
 
                 unsafe { this.value_from_data(lua, offset, data_handle) }
@@ -77,7 +77,7 @@ pub mod method_provider {
                     return Err(LuaError::external("Out of bounds"));
                 }
                 if !data_handle.is_writable() {
-                    return Err(LuaError::external("Unwritable data handle"));
+                    return Err(LuaError::external("Unwritable data"));
                 }
 
                 unsafe { this.value_into_data(lua, offset, data_handle, value) }
@@ -106,18 +106,18 @@ pub mod method_provider {
                 let dst = &dst.get_ffi_data()?;
                 // use or functions
                 if !dst.check_inner_boundary(dst_offset, this.get_size()) {
-                    return Err(LuaError::external("Out of bounds"));
+                    return Err(LuaError::external("Destination out of bounds"));
                 }
                 if !dst.is_writable() {
-                    return Err(LuaError::external("Unwritable data handle"));
+                    return Err(LuaError::external("Destination is unwritable"));
                 }
 
                 let src = &src.get_ffi_data()?;
                 if !src.check_inner_boundary(dst_offset, this.get_size()) {
-                    return Err(LuaError::external("Out of bounds"));
+                    return Err(LuaError::external("Source out of bounds"));
                 }
                 if !src.is_readable() {
-                    return Err(LuaError::external("Unreadable value data handle"));
+                    return Err(LuaError::external("Source is unreadable"));
                 }
 
                 unsafe { this.copy_data(lua, dst_offset, src_offset, dst, src) }
@@ -196,22 +196,6 @@ pub fn get_userdata(value: LuaValue) -> LuaResult<LuaAnyUserData> {
     }
 }
 
-// Get the NativeConvert handle from the type UserData
-// this is intended to avoid lookup userdata and lua table every time. (eg: struct)
-// userdata must live longer than the NativeConvert handle.
-// However, c_struct is a strong reference to each field, so this is not a problem.
-pub unsafe fn get_conv(userdata: &LuaAnyUserData) -> LuaResult<*const dyn FfiConvert> {
-    if userdata.is::<CStructInfo>() {
-        Ok(userdata.to_pointer().cast::<CStructInfo>() as *const dyn FfiConvert)
-    } else if userdata.is::<CArrInfo>() {
-        Ok(userdata.to_pointer().cast::<CArrInfo>() as *const dyn FfiConvert)
-    } else if userdata.is::<CPtrInfo>() {
-        Ok(userdata.to_pointer().cast::<CPtrInfo>() as *const dyn FfiConvert)
-    } else {
-        ctype_helper::get_conv(userdata)
-    }
-}
-
 // Create vec<T> from table with (userdata)->T
 pub fn create_list<T>(
     table: &LuaTable,
@@ -228,27 +212,23 @@ pub fn create_list<T>(
     Ok(list)
 }
 
-//Get
-pub unsafe fn get_conv_list(table: &LuaTable) -> LuaResult<Vec<*const dyn FfiConvert>> {
-    create_list(table, |userdata| get_conv(userdata))
-}
-
-// Get type size from ctype userdata
-pub fn get_size(userdata: &LuaAnyUserData) -> LuaResult<usize> {
+// Get the NativeConvert handle from the ctype userData
+// This is intended to avoid lookup userdata and lua table every time. (eg: struct)
+// The userdata must live longer than the NativeConvert handle
+pub unsafe fn get_conv(userdata: &LuaAnyUserData) -> LuaResult<*const dyn FfiConvert> {
     if userdata.is::<CStructInfo>() {
-        Ok(userdata.borrow::<CStructInfo>()?.get_size())
+        Ok(userdata.to_pointer().cast::<CStructInfo>() as *const dyn FfiConvert)
     } else if userdata.is::<CArrInfo>() {
-        Ok(userdata.borrow::<CArrInfo>()?.get_size())
+        Ok(userdata.to_pointer().cast::<CArrInfo>() as *const dyn FfiConvert)
     } else if userdata.is::<CPtrInfo>() {
-        Ok(userdata.borrow::<CPtrInfo>()?.get_size())
-    } else if userdata.is::<CVoidInfo>() {
-        Ok(userdata.borrow::<CVoidInfo>()?.get_size())
-    } else if userdata.is::<CFnInfo>() {
-        Ok(userdata.borrow::<CFnInfo>()?.get_size())
+        Ok(userdata.to_pointer().cast::<CPtrInfo>() as *const dyn FfiConvert)
     } else {
-        ctype_helper::get_size(userdata)
+        ctype_helper::get_conv(userdata)
     }
 }
+pub unsafe fn get_conv_list(table: &LuaTable) -> LuaResult<Vec<*const dyn FfiConvert>> {
+    create_list(table, |userdata| get_conv(userdata))
+}
 
 // Get libffi_type from ctype userdata
 pub fn get_middle_type(userdata: &LuaAnyUserData) -> LuaResult<Type> {
@@ -276,12 +256,28 @@ pub fn get_middle_type(userdata: &LuaAnyUserData) -> LuaResult<Type> {
         )))
     }
 }
-
-// get Vec<libffi_type> from table(array) of c-type userdata
 pub fn get_middle_type_list(table: &LuaTable) -> LuaResult<Vec<Type>> {
     create_list(table, get_middle_type)
 }
 
+// Get type size from ctype userdata
+pub fn get_size(userdata: &LuaAnyUserData) -> LuaResult<usize> {
+    if userdata.is::<CStructInfo>() {
+        Ok(userdata.borrow::<CStructInfo>()?.get_size())
+    } else if userdata.is::<CArrInfo>() {
+        Ok(userdata.borrow::<CArrInfo>()?.get_size())
+    } else if userdata.is::<CPtrInfo>() {
+        Ok(userdata.borrow::<CPtrInfo>()?.get_size())
+    } else if userdata.is::<CVoidInfo>() {
+        Ok(userdata.borrow::<CVoidInfo>()?.get_size())
+    } else if userdata.is::<CFnInfo>() {
+        Ok(userdata.borrow::<CFnInfo>()?.get_size())
+    } else {
+        ctype_helper::get_size(userdata)
+    }
+}
+
+// Check lua table has void ctype userdata
 pub fn has_void(table: &LuaTable) -> LuaResult<bool> {
     for i in 0..table.raw_len() {
         let value: LuaValue = table.raw_get(i + 1)?;
@@ -292,7 +288,7 @@ pub fn has_void(table: &LuaTable) -> LuaResult<bool> {
     Ok(false)
 }
 
-// stringify any c-type userdata (for recursive)
+// Stringify any c-type userdata recursively
 pub fn stringify(lua: &Lua, userdata: &LuaAnyUserData) -> LuaResult<String> {
     if userdata.is::<CStructInfo>() {
         CStructInfo::stringify(lua, userdata)
@@ -311,7 +307,7 @@ pub fn stringify(lua: &Lua, userdata: &LuaAnyUserData) -> LuaResult<String> {
     }
 }
 
-// get name tag for any c-type userdata
+// Get name tag from ctype userdata
 pub fn get_tag_name(userdata: &LuaAnyUserData) -> LuaResult<String> {
     Ok(if userdata.is::<CStructInfo>() {
         String::from("CStructInfo")
@@ -330,7 +326,8 @@ pub fn get_tag_name(userdata: &LuaAnyUserData) -> LuaResult<String> {
     })
 }
 
-// emulate 'print' for ctype userdata, but ctype is simplified
+// Emulate 'print' for ctype userdata, but simplified
+// Used for print struct field, cfn arguments, etc...
 pub fn pretty_format(lua: &Lua, userdata: &LuaAnyUserData) -> LuaResult<String> {
     if ctype_helper::is_ctype(userdata) {
         stringify(lua, userdata)

crates/lune-std-ffi/src/c/mod.rs

@@ -36,6 +36,7 @@ mod association_names {
     pub const CLOSURE_CFN: &str = "__closure_cfn";
 }
 
+// Export c namespace
 pub fn export_c(lua: &Lua) -> LuaResult<LuaTable> {
     TableBuilder::new(lua)?
         .with_value("void", CVoidInfo::new())?

crates/lune-std-ffi/src/c/ptr_info.rs

@@ -11,12 +11,9 @@ use crate::{
     },
 };
 
-const READ_CPTR_REF_FLAGS: u8 =
-    RefFlag::Dereferenceable.value() | RefFlag::Offsetable.value() | RefFlag::Leaked.value();
-const READ_REF_FLAGS: u8 = RefFlag::Offsetable.value()
-    | RefFlag::Leaked.value()
-    | RefFlag::Readable.value()
-    | RefFlag::Writable.value();
+const READ_CPTR_REF_FLAGS: u8 = RefFlag::Dereferenceable.value() | RefFlag::Offsetable.value();
+const READ_REF_FLAGS: u8 =
+    RefFlag::Offsetable.value() | RefFlag::Readable.value() | RefFlag::Writable.value();
 
 pub struct CPtrInfo {
     inner_size: usize,
@@ -42,9 +39,12 @@ impl FfiConvert for CPtrInfo {
         data_handle: &Ref<dyn FfiData>,
         value: LuaValue<'lua>,
     ) -> LuaResult<()> {
-        let value_userdata = value
-            .as_userdata()
-            .ok_or_else(|| LuaError::external("CPtrInfo:writeRef only allows data"))?;
+        let value_userdata = value.as_userdata().ok_or_else(|| {
+            LuaError::external(format!(
+                "Value must be a RefData, BoxData or ClosureData, got {}",
+                value.type_name()
+            ))
+        })?;
         *data_handle
             .get_inner_pointer()
             .byte_offset(offset)
@@ -112,7 +112,7 @@ impl CPtrInfo {
                 format!(" {pretty_formatted} ")
             })
         } else {
-            Err(LuaError::external("failed to get inner type userdata."))
+            Err(LuaError::external("Failed to retrieve inner type"))
         }
     }
 
@@ -124,11 +124,10 @@ impl CPtrInfo {
 
 impl LuaUserData for CPtrInfo {
     fn add_fields<'lua, F: LuaUserDataFields<'lua, Self>>(fields: &mut F) {
-        fields.add_field_method_get("size", |_, _| Ok(SIZE_OF_POINTER));
+        fields.add_field_method_get("size", |_lua, _this| Ok(SIZE_OF_POINTER));
         fields.add_field_function_get("inner", |lua, this| {
-            let inner = association::get(lua, CPTR_INNER, this)?
-                .ok_or_else(|| LuaError::external("inner type not found"))?;
-            Ok(inner)
+            association::get(lua, CPTR_INNER, this)?
+                .ok_or_else(|| LuaError::external("Failed to retrieve inner type"))
         });
     }
 

crates/lune-std-ffi/src/c/string_info.rs

@@ -1,3 +1,5 @@
+// TODO:
+
 use mlua::prelude::*;
 
 pub struct CStringInfo();