Merge pull request #741 from lilizoey/cleanup/script-instance-2

Cleanup some stuff mainly related to script instance

Author: Bromeon

godot-core/src/builtin/callable.rs

@@ -432,7 +432,7 @@ mod custom_callable {
         let a: &T = CallableUserdata::inner_from_raw(callable_userdata_a);
         let b: &T = CallableUserdata::inner_from_raw(callable_userdata_b);
 
-        (a == b) as sys::GDExtensionBool
+        sys::conv::bool_to_sys(a == b)
     }
 
     pub unsafe extern "C" fn rust_callable_to_string_display<T: fmt::Display>(
@@ -444,7 +444,7 @@ mod custom_callable {
         let s = crate::builtin::GString::from(c.to_string());
 
         s.move_into_string_ptr(r_out);
-        *r_is_valid = true as sys::GDExtensionBool;
+        *r_is_valid = sys::conv::SYS_TRUE;
     }
 
     pub unsafe extern "C" fn rust_callable_to_string_named<F>(
@@ -455,6 +455,6 @@ mod custom_callable {
         let w: &mut FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
 
         w.name.clone().move_into_string_ptr(r_out);
-        *r_is_valid = true as sys::GDExtensionBool;
+        *r_is_valid = sys::conv::SYS_TRUE;
     }
 }

godot-core/src/builtin/string/string_name.rs

@@ -317,7 +317,7 @@ impl From<&'static std::ffi::CStr> for StringName {
                 sys::interface_fn!(string_name_new_with_latin1_chars)(
                     ptr,
                     c_str.as_ptr(),
-                    true as sys::GDExtensionBool, // p_is_static
+                    sys::conv::SYS_TRUE, // p_is_static
                 )
             })
         };

godot-core/src/builtin/variant/mod.rs

@@ -374,6 +374,34 @@ impl Variant {
         // SAFETY: `variant_array` isn't null so it is safe to call `from_raw_parts_mut` on the pointer cast to `*mut Variant`.
         unsafe { std::slice::from_raw_parts_mut(variant_array, length) }
     }
+
+    /// Consumes self and turns it into a sys-ptr, should be used together with [`from_owned_var_sys`](Self::from_owned_var_sys).
+    ///
+    /// This will leak memory unless `from_owned_var_sys` is called on the returned pointer.
+    pub(crate) fn into_owned_var_sys(self) -> sys::GDExtensionVariantPtr {
+        sys::static_assert_eq_size_align!(Variant, sys::types::OpaqueVariant);
+
+        let leaked = Box::into_raw(Box::new(self));
+        leaked.cast()
+    }
+
+    /// Creates a `Variant` from a sys-ptr without incrementing the refcount.
+    ///
+    /// # Safety
+    ///
+    /// * Must only be used on a pointer returned from a call to [`into_owned_var_sys`](Self::into_owned_var_sys).
+    /// * Must not be called more than once on the same pointer.
+    #[deny(unsafe_op_in_unsafe_fn)]
+    pub(crate) unsafe fn from_owned_var_sys(ptr: sys::GDExtensionVariantPtr) -> Self {
+        sys::static_assert_eq_size_align!(Variant, sys::types::OpaqueVariant);
+
+        let ptr = ptr.cast::<Self>();
+
+        // SAFETY: `ptr` was returned from a call to `into_owned_var_sys`, which means it was created by a call to
+        // `Box::into_raw`, thus we can use `Box::from_raw` here. Additionally this is only called once on this pointer.
+        let boxed = unsafe { Box::from_raw(ptr) };
+        *boxed
+    }
 }
 
 impl ArrayElement for Variant {}

godot-core/src/meta/mod.rs

@@ -17,6 +17,7 @@ mod traits;
 pub mod error;
 pub use class_name::ClassName;
 pub use godot_convert::{FromGodot, GodotConvert, ToGodot};
+use sys::conv::u32_to_usize;
 pub use traits::{ArrayElement, GodotType};
 
 pub(crate) use crate::impl_godot_as_self;
@@ -241,50 +242,117 @@ pub struct MethodInfo {
 }
 
 impl MethodInfo {
-    /// Converts to the FFI type. Keep this object allocated while using that!
-    ///
-    /// The struct returned by this function contains pointers into the fields of `self`. `self` should therefore not be dropped while the
-    /// `sys::GDExtensionMethodInfo` is still in use.
+    /// Consumes self and turns it into a `sys::GDExtensionMethodInfo`, should be used together with
+    /// [`free_owned_method_sys`](Self::free_owned_method_sys).
     ///
-    /// This function also leaks memory that has to be cleaned up by the caller once it is no longer used. Specifically the `arguments` and
-    /// `default_arguments` vectors have to be reconstructed from the pointer and length and then dropped/freed.
-    ///
-    /// Each vector can be reconstructed with `Vec::from_raw_parts` since the pointers were created with `Vec::into_boxed_slice`, which
-    /// guarantees that the vector capacity and length are equal.
-    pub fn method_sys(&self) -> sys::GDExtensionMethodInfo {
+    /// This will leak memory unless used together with `free_owned_method_sys`.
+    pub fn into_owned_method_sys(self) -> sys::GDExtensionMethodInfo {
         use crate::obj::EngineBitfield as _;
 
-        let argument_count = self.arguments.len() as u32;
-        let argument_vec = self
-            .arguments
-            .iter()
-            .map(|arg| arg.property_sys())
-            .collect::<Vec<_>>()
-            .into_boxed_slice();
-
-        // SAFETY: dereferencing the new box pointer is fine as it is guaranteed to not be null
-        let arguments = unsafe { (*Box::into_raw(argument_vec)).as_mut_ptr() };
-
-        let default_argument_count = self.default_arguments.len() as u32;
-        let default_argument_vec = self
-            .default_arguments
-            .iter()
-            .map(|arg| sys::SysPtr::force_mut(arg.var_sys()))
-            .collect::<Vec<_>>()
-            .into_boxed_slice();
-
-        // SAFETY: dereferencing the new box pointer is fine as it is guaranteed to not be null
-        let default_arguments = unsafe { (*Box::into_raw(default_argument_vec)).as_mut_ptr() };
+        // Destructure self to ensure all fields are used.
+        let Self {
+            id,
+            method_name,
+            // TODO: Do we need this?
+            class_name: _class_name,
+            return_type,
+            arguments,
+            default_arguments,
+            flags,
+        } = self;
+
+        let argument_count: u32 = arguments
+            .len()
+            .try_into()
+            .expect("cannot have more than `u32::MAX` arguments");
+        let arguments = arguments
+            .into_iter()
+            .map(|arg| arg.into_owned_property_sys())
+            .collect::<Box<[_]>>();
+        let arguments = Box::leak(arguments).as_mut_ptr();
+
+        let default_argument_count: u32 = default_arguments
+            .len()
+            .try_into()
+            .expect("cannot have more than `u32::MAX` default arguments");
+        let default_argument = default_arguments
+            .into_iter()
+            .map(|arg| arg.into_owned_var_sys())
+            .collect::<Box<[_]>>();
+        let default_arguments = Box::leak(default_argument).as_mut_ptr();
 
         sys::GDExtensionMethodInfo {
-            id: self.id,
-            name: sys::SysPtr::force_mut(self.method_name.string_sys()),
-            return_value: self.return_type.property_sys(),
+            id,
+            name: method_name.into_owned_string_sys(),
+            return_value: return_type.into_owned_property_sys(),
             argument_count,
             arguments,
             default_argument_count,
             default_arguments,
-            flags: u32::try_from(self.flags.ord()).expect("flags should be valid"),
+            flags: flags.ord().try_into().expect("flags should be valid"),
+        }
+    }
+
+    /// Properly frees a `sys::GDExtensionMethodInfo` created by [`into_owned_method_sys`](Self::into_owned_method_sys).
+    ///
+    /// # Safety
+    ///
+    /// * Must only be used on a struct returned from a call to `into_owned_method_sys`, without modification.
+    /// * Must not be called more than once on a `sys::GDExtensionMethodInfo` struct.
+    #[deny(unsafe_op_in_unsafe_fn)]
+    pub unsafe fn free_owned_method_sys(info: sys::GDExtensionMethodInfo) {
+        // Destructure info to ensure all fields are used.
+        let sys::GDExtensionMethodInfo {
+            name,
+            return_value,
+            flags: _flags,
+            id: _id,
+            argument_count,
+            arguments,
+            default_argument_count,
+            default_arguments,
+        } = info;
+
+        // SAFETY: `name` is a pointer that was returned from `StringName::into_owned_string_sys`, and has not been freed before this.
+        let _name = unsafe { StringName::from_owned_string_sys(name) };
+
+        // SAFETY: `return_value` is a pointer that was returned from `PropertyInfo::into_owned_property_sys`, and has not been freed before
+        // this.
+        unsafe { PropertyInfo::free_owned_property_sys(return_value) };
+
+        // SAFETY:
+        // - `from_raw_parts_mut`: `arguments` comes from `as_mut_ptr()` on a mutable slice of length `argument_count`, and no other
+        //    accesses to the pointer happens for the lifetime of the slice.
+        // - `Box::from_raw`: The slice was returned from a call to `Box::leak`, and we have ownership of the value behind this pointer.
+        let arguments = unsafe {
+            let slice = std::slice::from_raw_parts_mut(arguments, u32_to_usize(argument_count));
+
+            Box::from_raw(slice)
+        };
+
+        for info in arguments.iter() {
+            // SAFETY: These infos were originally created from a call to `PropertyInfo::into_owned_property_sys`, and this method
+            // will not be called again on this pointer.
+            unsafe { PropertyInfo::free_owned_property_sys(*info) }
+        }
+
+        // SAFETY:
+        // - `from_raw_parts_mut`: `default_arguments` comes from `as_mut_ptr()` on a mutable slice of length `default_argument_count`, and no
+        //    other accesses to the pointer happens for the lifetime of the slice.
+        // - `Box::from_raw`: The slice was returned from a call to `Box::leak`, and we have ownership of the value behind this pointer.
+        let default_arguments = unsafe {
+            let slice = std::slice::from_raw_parts_mut(
+                default_arguments,
+                u32_to_usize(default_argument_count),
+            );
+
+            Box::from_raw(slice)
+        };
+
+        for variant in default_arguments.iter() {
+            // SAFETY: These pointers were originally created from a call to `Variant::into_owned_var_sys`, and this method will not be
+            // called again on this pointer.
+            let _variant = unsafe { Variant::from_owned_var_sys(*variant) };
         }
     }
 }