From 686b59dedb23c0a5f7d766243e666922205400ce Mon Sep 17 00:00:00 2001 From: Lili Zoey Zyli Date: Sun, 2 Jun 2024 18:32:21 +0200 Subject: [PATCH] Add `sys::conv` Add `#[deny(unsafe_op_in_unsafe_fn)]` to `script_instance_info` Use `sys::conv` bools Final touches --- godot-core/src/builtin/callable.rs | 6 +- godot-core/src/builtin/string/string_name.rs | 2 +- godot-core/src/meta/mod.rs | 68 ++-- godot-core/src/obj/script.rs | 364 +++++++++++------- godot-core/src/registry/callbacks.rs | 17 +- godot-core/src/registry/class.rs | 12 +- godot-core/src/registry/constant.rs | 2 +- godot-ffi/src/conv.rs | 65 ++++ godot-ffi/src/lib.rs | 2 + itest/godot/ManualFfiTests.gd | 15 +- .../script/script_instance_tests.rs | 38 +- 11 files changed, 359 insertions(+), 232 deletions(-) create mode 100644 godot-ffi/src/conv.rs diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index ddfb5c930..277024a09 100644 --- a/godot-core/src/builtin/callable.rs +++ b/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( @@ -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( @@ -455,6 +455,6 @@ mod custom_callable { let w: &mut FnWrapper = 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; } } diff --git a/godot-core/src/builtin/string/string_name.rs b/godot-core/src/builtin/string/string_name.rs index d33f83e6e..f49614051 100644 --- a/godot-core/src/builtin/string/string_name.rs +++ b/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 ) }) }; diff --git a/godot-core/src/meta/mod.rs b/godot-core/src/meta/mod.rs index 8a2c063a0..641688c0a 100644 --- a/godot-core/src/meta/mod.rs +++ b/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; @@ -312,43 +313,46 @@ impl MethodInfo { default_arguments, } = info; - // SAFETY: `name` and `return_value` were created from the appropriate method calls, and have not been freed before this. - unsafe { - let _name = StringName::from_owned_string_sys(name); - PropertyInfo::free_owned_property_sys(return_value); - } + // 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)); - // SAFETY: These pointers were both created from a call to `as_mut_ptr` on a slice. Additionally these pointer will not be accessed - // again after this function call. - let (arguments_slice, default_arguments_slice) = unsafe { - ( - std::slice::from_raw_parts_mut( - arguments, - argument_count - .try_into() - .expect("gdext only supports targets where u32 <= usize"), - ), - std::slice::from_raw_parts_mut( - default_arguments, - default_argument_count - .try_into() - .expect("gdext only supports targets where u32 <= usize"), - ), - ) + Box::from_raw(slice) }; - // SAFETY: We have exclusive ownership of these slices, and they were originally created from a call to `Box::leak`. - let (_arguments, default_arguments) = unsafe { - ( - Box::from_raw(arguments_slice), - Box::from_raw(default_arguments_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) }; - default_arguments.iter().for_each(|ptr| { - // SAFETY: These pointers were originally created from a call to `Variant::into_owner_var_sys`, and this method will not be + 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(*ptr) }; - }); + let _variant = unsafe { Variant::from_owned_var_sys(*variant) }; + } } } diff --git a/godot-core/src/obj/script.rs b/godot-core/src/obj/script.rs index 75e48dda4..fb54459f2 100644 --- a/godot-core/src/obj/script.rs +++ b/godot-core/src/obj/script.rs @@ -30,7 +30,7 @@ use crate::meta::{MethodInfo, PropertyInfo}; use crate::obj::{Base, Gd, GodotClass}; use crate::sys; -use self::ptrlist_container::PtrlistContainer; +use self::bounded_ptr_list::BoundedPtrList; /// Implement custom scripts that can be attached to objects in Godot. /// @@ -131,7 +131,7 @@ pub trait ScriptInstance: Sized { /// Lets the engine get a reference to the script this instance was created for. /// - /// This function has to return a reference, because Scripts are reference counted in Godot and it has to be guaranteed that the object is + /// This function has to return a reference, because Scripts are reference counted in Godot and it must be guaranteed that the object is /// not freed before the engine increased the reference count. (every time a `Gd` which contains a reference counted object is dropped the /// reference count is decremented.) fn get_script(&self) -> &Gd