From 7a4161cfbd1b26398af26f539e0177b2a534e612 Mon Sep 17 00:00:00 2001 From: Lili Zoey Zyli Date: Sat, 24 Feb 2024 20:08:47 +0100 Subject: [PATCH] Refactor `GodotFfi` a bit to use const vs mut pointers better and have more clear names --- godot-codegen/src/generator/builtins.rs | 4 +- godot-core/src/builtin/aabb.rs | 6 + godot-core/src/builtin/array.rs | 70 ++++-- godot-core/src/builtin/basis.rs | 6 + godot-core/src/builtin/callable.rs | 40 ++-- godot-core/src/builtin/color.rs | 6 + godot-core/src/builtin/dictionary.rs | 42 ++-- godot-core/src/builtin/macros.rs | 10 +- godot-core/src/builtin/meta/class_name.rs | 2 +- godot-core/src/builtin/meta/mod.rs | 10 +- .../src/builtin/meta/registration/method.rs | 9 +- godot-core/src/builtin/meta/return_marshal.rs | 8 +- godot-core/src/builtin/meta/signature.rs | 14 +- godot-core/src/builtin/packed_array.rs | 47 +++- godot-core/src/builtin/plane.rs | 25 +- godot-core/src/builtin/projection.rs | 6 + godot-core/src/builtin/quaternion.rs | 6 + godot-core/src/builtin/rect2.rs | 6 + godot-core/src/builtin/rect2i.rs | 6 + godot-core/src/builtin/rid.rs | 25 +- godot-core/src/builtin/signal.rs | 30 ++- godot-core/src/builtin/string/gstring.rs | 36 +-- godot-core/src/builtin/string/node_path.rs | 30 ++- godot-core/src/builtin/string/string_name.rs | 45 ++-- godot-core/src/builtin/transform2d.rs | 6 + godot-core/src/builtin/transform3d.rs | 6 + godot-core/src/builtin/variant/impls.rs | 7 +- godot-core/src/builtin/variant/mod.rs | 55 +++-- godot-core/src/engine/script_instance.rs | 32 +-- godot-core/src/init/mod.rs | 4 +- godot-core/src/log.rs | 6 +- godot-core/src/obj/raw.rs | 29 ++- godot-core/src/registry/callbacks.rs | 25 +- godot-ffi/src/extras.rs | 71 ++++++ godot-ffi/src/godot_ffi.rs | 217 ++++++++++-------- godot-ffi/src/lib.rs | 4 +- .../builtin_tests/containers/variant_test.rs | 4 +- itest/rust/src/object_tests/object_test.rs | 6 +- .../src/register_tests/option_ffi_test.rs | 4 +- 39 files changed, 625 insertions(+), 340 deletions(-) diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs index 9fc67c6a6..ea5c7cd80 100644 --- a/godot-codegen/src/generator/builtins.rs +++ b/godot-codegen/src/generator/builtins.rs @@ -117,7 +117,7 @@ fn make_builtin_class(class: &BuiltinClass, ctx: &mut Context) -> GeneratedBuilt pub fn from_outer(outer: &#outer_class) -> Self { Self { _outer_lifetime: std::marker::PhantomData, - sys_ptr: outer.sys(), + sys_ptr: sys::AsConst::force_mut(outer.sys()), } } #special_constructors @@ -154,7 +154,7 @@ fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStr { Self { _outer_lifetime: std::marker::PhantomData, - sys_ptr: outer.sys(), + sys_ptr: sys::AsConst::force_mut(outer.sys()), } } } diff --git a/godot-core/src/builtin/aabb.rs b/godot-core/src/builtin/aabb.rs index e3bdff770..d8e8a32b1 100644 --- a/godot-core/src/builtin/aabb.rs +++ b/godot-core/src/builtin/aabb.rs @@ -406,6 +406,12 @@ unsafe impl GodotFfi for Aabb { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Aabb { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Aabb); impl ApproxEq for Aabb { diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index 05291f317..7a066ac21 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -11,7 +11,7 @@ use crate::builtin::*; use crate::property::{Export, PropertyHintInfo, TypeStringHint, Var}; use std::fmt; use std::marker::PhantomData; -use sys::{ffi_methods, interface_fn, GodotFfi}; +use sys::{ffi_methods, interface_fn, GodotFfi, GodotFfiBorrowable}; use super::meta::{ ConvertError, FromGodot, FromGodotError, FromVariantError, GodotConvert, GodotFfiVariant, @@ -54,7 +54,7 @@ use super::meta::{ // for `T: GodotType` because `drop()` requires `sys_mut()`, which is on the `GodotFfi` // trait, whose `from_sys_init()` requires `Default`, which is only implemented for `T: // GodotType`. Whew. This could be fixed by splitting up `GodotFfi` if desired. -#[repr(C)] +#[repr(transparent)] pub struct Array { // Safety Invariant: The type of all values in `opaque` matches the type `T`. opaque: sys::types::OpaqueArray, @@ -253,7 +253,7 @@ impl Array { /// # Panics /// /// If `index` is out of bounds. - fn ptr_mut(&self, index: usize) -> *mut Variant { + fn ptr_mut(&mut self, index: usize) -> *mut Variant { let ptr = self.ptr_mut_or_null(index); assert!( !ptr.is_null(), @@ -264,11 +264,11 @@ impl Array { } /// Returns a pointer to the element at the given index, or null if out of bounds. - fn ptr_mut_or_null(&self, index: usize) -> *mut Variant { + fn ptr_mut_or_null(&mut self, index: usize) -> *mut Variant { // SAFETY: array_operator_index returns null for invalid indexes. let variant_ptr = unsafe { let index = to_i64(index); - interface_fn!(array_operator_index)(self.sys(), index) + interface_fn!(array_operator_index)(self.sys_mut(), index) }; Variant::ptr_from_sys_mut(variant_ptr) @@ -456,7 +456,7 @@ impl Array { // SAFETY: The array is a newly created empty untyped array. unsafe { interface_fn!(array_set_typed)( - self.sys(), + self.sys_mut(), type_info.variant_type.sys(), type_info.class_name.string_sys(), script.var_sys(), @@ -745,22 +745,46 @@ unsafe impl GodotFfi for Array { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; fn sys; - fn from_sys_init; + fn sys_mut; + fn new_with_uninit; + fn new_with_init; fn move_return_ptr; } - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { + let array = Self::borrow_sys(ptr); + array.clone() } +} - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - let array = Self::from_sys(ptr); - std::mem::forget(array.clone()); - array +impl sys::HasOpaque for Array { + type Opaque = sys::types::OpaqueArray; + + unsafe fn borrow_opaque<'a>(ptr: *const Self::Opaque) -> &'a Self { + &*(ptr as *mut Self) + } + + fn from_opaque(opaque: Self::Opaque) -> Self { + Self { + opaque, + _phantom: PhantomData, + } + } + + fn opaque(&self) -> &Self::Opaque { + &self.opaque + } + + fn opaque_mut(&mut self) -> &mut Self::Opaque { + &mut self.opaque + } +} + +impl GodotFfiBorrowable for Array { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const sys::types::OpaqueArray) } } @@ -825,9 +849,9 @@ impl Clone for Array { fn clone(&self) -> Self { // SAFETY: `self` is a valid array, since we have a reference that keeps it alive. let array = unsafe { - Self::from_sys_init(|self_ptr| { + Self::new_with_uninit(|self_ptr| { let ctor = ::godot_ffi::builtin_fn!(array_construct_copy); - let args = [self.sys_const()]; + let args = [self.sys()]; ctor(self_ptr, args.as_ptr()); }) }; @@ -891,7 +915,7 @@ impl Default for Array { #[inline] fn default() -> Self { let mut array = unsafe { - Self::from_sys_init(|self_ptr| { + Self::new_with_uninit(|self_ptr| { let ctor = sys::builtin_fn!(array_construct_default); ctor(self_ptr, std::ptr::null_mut()) }) @@ -937,9 +961,9 @@ impl GodotType for Array { impl GodotFfiVariant for Array { fn ffi_to_variant(&self) -> Variant { unsafe { - Variant::from_var_sys_init(|variant_ptr| { + Variant::new_with_uninit_var_sys(|variant_ptr| { let array_to_variant = sys::builtin_fn!(array_to_variant); - array_to_variant(variant_ptr, self.sys()); + array_to_variant(variant_ptr, sys::AsConst::force_mut(self.sys())); }) } } @@ -956,7 +980,7 @@ impl GodotFfiVariant for Array { let array = unsafe { sys::from_sys_init_or_init_default::(|self_ptr| { let array_from_variant = sys::builtin_fn!(array_from_variant); - array_from_variant(self_ptr, variant.var_sys()); + array_from_variant(self_ptr, sys::AsConst::force_mut(variant.var_sys())); }) }; @@ -977,7 +1001,7 @@ impl From<&[T; N]> for Array { /// Creates a `Array` from the given slice. impl From<&[T]> for Array { fn from(slice: &[T]) -> Self { - let array = Self::new(); + let mut array = Self::new(); let len = slice.len(); if len == 0 { return array; diff --git a/godot-core/src/builtin/basis.rs b/godot-core/src/builtin/basis.rs index 63786e04c..087d6c4a4 100644 --- a/godot-core/src/builtin/basis.rs +++ b/godot-core/src/builtin/basis.rs @@ -604,6 +604,12 @@ unsafe impl GodotFfi for Basis { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Basis { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Basis); /// The ordering used to interpret a set of euler angles as extrinsic diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index 71a802a1f..cfcfe47f7 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -14,7 +14,7 @@ use crate::obj::bounds::DynMemory; use crate::obj::Bounds; use crate::obj::{Gd, GodotClass, InstanceId}; use std::{fmt, ptr}; -use sys::{ffi_methods, GodotFfi}; +use sys::{ffi_methods, GodotFfi, GodotFfiBorrowable}; /// A `Callable` represents a function in Godot. /// @@ -25,7 +25,7 @@ use sys::{ffi_methods, GodotFfi}; /// Currently it is impossible to use `bind` and `unbind` in GDExtension, see [godot-cpp#802]. /// /// [godot-cpp#802]: https://github.com/godotengine/godot-cpp/issues/802 -#[repr(C, align(8))] +#[repr(transparent)] pub struct Callable { opaque: sys::types::OpaqueCallable, } @@ -51,7 +51,7 @@ impl Callable { sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(callable_from_object_method); let raw = object.to_ffi(); - let args = [raw.as_arg_ptr(), method.sys_const()]; + let args = [raw.as_arg_ptr(), method.sys()]; ctor(self_ptr, args.as_ptr()); }) } @@ -140,7 +140,7 @@ impl Callable { fn from_custom_info(mut info: sys::GDExtensionCallableCustomInfo) -> Callable { // SAFETY: callable_custom_create() is a valid way of creating callables. unsafe { - Callable::from_sys_init(|type_ptr| { + Callable::new_with_uninit(|type_ptr| { sys::interface_fn!(callable_custom_create)(type_ptr, ptr::addr_of_mut!(info)) }) } @@ -151,7 +151,7 @@ impl Callable { /// _Godot equivalent: `Callable()`_ pub fn invalid() -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::new_with_uninit(|self_ptr| { let ctor = sys::builtin_fn!(callable_construct_default); ctor(self_ptr, ptr::null_mut()) }) @@ -293,22 +293,36 @@ unsafe impl GodotFfi for Callable { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; fn sys; - fn from_sys_init; + fn sys_mut; + fn new_with_uninit; fn move_return_ptr; } unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - let callable = Self::from_sys(ptr); - callable.inc_ref(); + let callable = Self::borrow_sys(ptr); + callable.clone() + } + + fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self + where + Self: Sized, + { + let mut callable = Callable::invalid(); + init_fn(&mut callable); + callable } +} + +sys::unsafe_impl_has_opaque!(Callable { + opaque: sys::types::OpaqueCallable +}); - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::invalid(); - init_fn(result.sys_mut()); - result +impl GodotFfiBorrowable for Callable { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const sys::types::OpaqueCallable) } } diff --git a/godot-core/src/builtin/color.rs b/godot-core/src/builtin/color.rs index 3a515b1e5..89169531d 100644 --- a/godot-core/src/builtin/color.rs +++ b/godot-core/src/builtin/color.rs @@ -356,6 +356,12 @@ unsafe impl GodotFfi for Color { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Color { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Color); impl ApproxEq for Color { diff --git a/godot-core/src/builtin/dictionary.rs b/godot-core/src/builtin/dictionary.rs index a5b54b821..862e81e71 100644 --- a/godot-core/src/builtin/dictionary.rs +++ b/godot-core/src/builtin/dictionary.rs @@ -13,7 +13,7 @@ use crate::property::{Export, PropertyHintInfo, TypeStringHint, Var}; use std::marker::PhantomData; use std::{fmt, ptr}; use sys::types::OpaqueDictionary; -use sys::{ffi_methods, interface_fn, AsUninit, GodotFfi}; +use sys::{ffi_methods, interface_fn, AsUninit, GodotFfi, GodotFfiBorrowable}; use super::meta::impl_godot_as_self; @@ -25,7 +25,7 @@ use super::meta::impl_godot_as_self; /// # Thread safety /// /// The same principles apply as for [`VariantArray`]. Consult its documentation for details. -#[repr(C)] +#[repr(transparent)] pub struct Dictionary { opaque: OpaqueDictionary, } @@ -244,9 +244,8 @@ impl Dictionary { let key = key.to_variant(); // SAFETY: accessing an unknown key _mutably_ creates that entry in the dictionary, with value `NIL`. - let ptr = unsafe { - interface_fn!(dictionary_operator_index)(self.sys_mut(), key.var_sys_const()) - }; + let ptr = + unsafe { interface_fn!(dictionary_operator_index)(self.sys_mut(), key.var_sys()) }; // Never a null pointer, since entry either existed already or was inserted above. Variant::ptr_from_sys_mut(ptr) @@ -271,22 +270,27 @@ unsafe impl GodotFfi for Dictionary { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn from_sys_init; + fn new_from_sys; fn sys; + fn sys_mut; + fn new_with_uninit; + fn new_with_init; fn move_return_ptr; } - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result + unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { + let dictionary = Self::borrow_sys(ptr); + dictionary.clone() } +} - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - let dictionary = Self::from_sys(ptr); - std::mem::forget(dictionary.clone()); - dictionary +sys::unsafe_impl_has_opaque!(Dictionary { + opaque: OpaqueDictionary +}); + +impl GodotFfiBorrowable for Dictionary { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const OpaqueDictionary) } } @@ -331,9 +335,9 @@ impl Clone for Dictionary { fn clone(&self) -> Self { // SAFETY: `self` is a valid dictionary, since we have a reference that keeps it alive. unsafe { - Self::from_sys_init(|self_ptr| { + Self::new_with_uninit(|self_ptr| { let ctor = sys::builtin_fn!(dictionary_construct_copy); - let args = [self.sys_const()]; + let args = [self.sys()]; ctor(self_ptr, args.as_ptr()); }) } @@ -484,7 +488,7 @@ impl<'a> DictionaryIter<'a> { *mut sys::GDExtensionBool, ) -> sys::GDExtensionBool, dictionary: &Dictionary, - next_value: Variant, + mut next_value: Variant, ) -> Option { let dictionary = dictionary.to_variant(); let mut valid_u8: u8 = 0; @@ -496,7 +500,7 @@ impl<'a> DictionaryIter<'a> { let has_next = unsafe { iter_fn( dictionary.var_sys(), - next_value.var_sys(), + next_value.var_sys_mut(), ptr::addr_of_mut!(valid_u8), ) }; diff --git a/godot-core/src/builtin/macros.rs b/godot-core/src/builtin/macros.rs index 432936667..a55e7d9ff 100644 --- a/godot-core/src/builtin/macros.rs +++ b/godot-core/src/builtin/macros.rs @@ -13,7 +13,7 @@ macro_rules! impl_builtin_traits_inner { #[inline] fn default() -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::new_with_uninit(|self_ptr| { let ctor = ::godot_ffi::builtin_fn!($gd_method); ctor(self_ptr, std::ptr::null_mut()) }) @@ -27,9 +27,9 @@ macro_rules! impl_builtin_traits_inner { #[inline] fn clone(&self) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::new_with_uninit(|self_ptr| { let ctor = ::godot_ffi::builtin_fn!($gd_method); - let args = [self.sys_const()]; + let args = [self.sys()]; ctor(self_ptr, args.as_ptr()); }) } @@ -129,8 +129,8 @@ macro_rules! impl_builtin_froms { fn from(other: &$From) -> Self { unsafe { // TODO should this be from_sys_init_default()? - Self::from_sys_init(|ptr| { - let args = [other.sys_const()]; + Self::new_with_uninit(|ptr| { + let args = [other.sys()]; ::godot_ffi::builtin_call! { $from_fn(ptr, args.as_ptr()) } diff --git a/godot-core/src/builtin/meta/class_name.rs b/godot-core/src/builtin/meta/class_name.rs index cd71ff669..dcf1da534 100644 --- a/godot-core/src/builtin/meta/class_name.rs +++ b/godot-core/src/builtin/meta/class_name.rs @@ -82,7 +82,7 @@ impl ClassName { /// The returned pointer is valid indefinitely, as entries are never deleted from the cache. /// Since we use `Box`, `HashMap` reallocations don't affect the validity of the StringName. #[doc(hidden)] - pub fn string_sys(&self) -> sys::GDExtensionStringNamePtr { + pub fn string_sys(&self) -> sys::GDExtensionConstStringNamePtr { self.with_string_name(|s| s.string_sys()) } diff --git a/godot-core/src/builtin/meta/mod.rs b/godot-core/src/builtin/meta/mod.rs index d56dffef1..85af37b8a 100644 --- a/godot-core/src/builtin/meta/mod.rs +++ b/godot-core/src/builtin/meta/mod.rs @@ -263,10 +263,10 @@ impl PropertyInfo { sys::GDExtensionPropertyInfo { type_: self.variant_type.sys(), - name: self.property_name.string_sys(), - class_name: self.class_name.string_sys(), + name: sys::AsConst::force_mut(self.property_name.string_sys()), + class_name: sys::AsConst::force_mut(self.class_name.string_sys()), hint: u32::try_from(self.hint.ord()).expect("hint.ord()"), - hint_string: self.hint_string.string_sys(), + hint_string: sys::AsConst::force_mut(self.hint_string.string_sys()), usage: u32::try_from(self.usage.ord()).expect("usage.ord()"), } } @@ -326,7 +326,7 @@ impl MethodInfo { let default_argument_vec = self .default_arguments .iter() - .map(|arg| arg.var_sys()) + .map(|arg| sys::AsConst::force_mut(arg.var_sys())) .collect::>() .into_boxed_slice(); @@ -335,7 +335,7 @@ impl MethodInfo { sys::GDExtensionMethodInfo { id: self.id, - name: self.method_name.string_sys(), + name: sys::AsConst::force_mut(self.method_name.string_sys()), return_value: self.return_type.property_sys(), argument_count, arguments, diff --git a/godot-core/src/builtin/meta/registration/method.rs b/godot-core/src/builtin/meta/registration/method.rs index b728b84bf..43ce36517 100644 --- a/godot-core/src/builtin/meta/registration/method.rs +++ b/godot-core/src/builtin/meta/registration/method.rs @@ -118,11 +118,14 @@ impl ClassMethodInfo { let mut arguments_metadata: Vec = self.arguments.iter().map(|info| info.metadata).collect(); - let mut default_arguments_sys: Vec = - self.default_arguments.iter().map(|v| v.var_sys()).collect(); + let mut default_arguments_sys: Vec = self + .default_arguments + .iter() + .map(|v| sys::AsConst::force_mut(v.var_sys())) + .collect(); let method_info_sys = sys::GDExtensionClassMethodInfo { - name: self.method_name.string_sys(), + name: sys::AsConst::force_mut(self.method_name.string_sys()), method_userdata: std::ptr::null_mut(), call_func: self.call_func, ptrcall_func: self.ptrcall_func, diff --git a/godot-core/src/builtin/meta/return_marshal.rs b/godot-core/src/builtin/meta/return_marshal.rs index 53f21f3d4..98aeb7c81 100644 --- a/godot-core/src/builtin/meta/return_marshal.rs +++ b/godot-core/src/builtin/meta/return_marshal.rs @@ -50,10 +50,10 @@ impl PtrcallReturn for PtrcallReturnT { unsafe fn call( mut process_return_ptr: impl FnMut(sys::GDExtensionTypePtr), ) -> Result { - let ffi = - <::Ffi as sys::GodotFfi>::from_sys_init_default(|return_ptr| { - process_return_ptr(return_ptr) - }); + let ffi = <::Ffi as sys::GodotFfi>::new_with_init(|return_ptr| { + let ptr = sys::GodotFfi::sys_mut(return_ptr); + process_return_ptr(ptr) + }); T::Via::try_from_ffi(ffi).and_then(T::try_from_godot) } diff --git a/godot-core/src/builtin/meta/signature.rs b/godot-core/src/builtin/meta/signature.rs index b01aa5c71..cfac3317e 100644 --- a/godot-core/src/builtin/meta/signature.rs +++ b/godot-core/src/builtin/meta/signature.rs @@ -221,10 +221,10 @@ macro_rules! impl_varcall_signature_for_tuple { ]; let mut variant_ptrs = Vec::with_capacity(explicit_args.len() + varargs.len()); - variant_ptrs.extend(explicit_args.iter().map(Variant::var_sys_const)); - variant_ptrs.extend(varargs.iter().map(Variant::var_sys_const)); + variant_ptrs.extend(explicit_args.iter().map(Variant::var_sys)); + variant_ptrs.extend(varargs.iter().map(Variant::var_sys)); - let variant = Variant::from_var_sys_init(|return_ptr| { + let variant = Variant::new_with_uninit_var_sys(|return_ptr| { let mut err = sys::default_call_error(); class_fn( method_bind.0, @@ -263,9 +263,9 @@ macro_rules! impl_varcall_signature_for_tuple { )* ]; - let variant_ptrs = explicit_args.iter().map(Variant::var_sys_const).collect::>(); + let variant_ptrs = explicit_args.iter().map(Variant::var_sys).collect::>(); - let variant = Variant::from_var_sys_init(|return_ptr| { + let variant = Variant::new_with_uninit_var_sys(|return_ptr| { let mut err = sys::default_call_error(); object_call_script_method( object_ptr, @@ -299,8 +299,8 @@ macro_rules! impl_varcall_signature_for_tuple { ]; let mut type_ptrs = Vec::with_capacity(explicit_args.len() + varargs.len()); - type_ptrs.extend(explicit_args.iter().map(sys::GodotFfi::sys_const)); - type_ptrs.extend(varargs.iter().map(sys::GodotFfi::sys_const)); + type_ptrs.extend(explicit_args.iter().map(sys::GodotFfi::sys)); + type_ptrs.extend(varargs.iter().map(sys::GodotFfi::sys)); // Important: this calls from_sys_init_default(). let result = PtrcallReturnT::<$R>::call(|return_ptr| { diff --git a/godot-core/src/builtin/packed_array.rs b/godot-core/src/builtin/packed_array.rs index 787986a99..66d089b12 100644 --- a/godot-core/src/builtin/packed_array.rs +++ b/godot-core/src/builtin/packed_array.rs @@ -71,7 +71,7 @@ macro_rules! impl_packed_array { /// but any writes must be externally synchronized. The Rust compiler will enforce this as /// long as you use only Rust threads, but it cannot protect against concurrent modification /// on other threads (e.g. created through GDScript). - #[repr(C)] + #[repr(transparent)] pub struct $PackedArray { opaque: $Opaque, } @@ -359,13 +359,13 @@ macro_rules! impl_packed_array { /// # Panics /// /// If `index` is out of bounds. - fn ptr_mut(&self, index: usize) -> *mut $Element { + fn ptr_mut(&mut self, index: usize) -> *mut $Element { self.check_bounds(index); // SAFETY: We just checked that the index is not out of bounds. let ptr = unsafe { let item_ptr: *mut $IndexRetType = - (interface_fn!($operator_index))(self.sys(), to_i64(index)); + (interface_fn!($operator_index))(self.sys_mut(), to_i64(index)); item_ptr as *mut $Element }; assert!(!ptr.is_null()); @@ -473,9 +473,11 @@ macro_rules! impl_packed_array { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; fn sys; - fn from_sys_init; + fn sys_mut; + fn new_with_uninit; + fn new_with_init; // SAFETY: // Nothing special needs to be done beyond a `std::mem::swap` when returning a packed array. fn move_return_ptr; @@ -487,15 +489,36 @@ macro_rules! impl_packed_array { // // Using `std::mem::forget(array.clone())` increments the ref count. unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - let array = Self::from_sys(ptr); - std::mem::forget(array.clone()); - array + let array: &Self = sys::GodotFfiBorrowable::borrow_sys(ptr); + array.clone() } + } + + impl sys::HasOpaque for $PackedArray { + type Opaque = $Opaque; + + unsafe fn borrow_opaque<'a>(ptr: *const Self::Opaque) -> &'a Self { + &*(ptr as *mut Self) + } + + fn from_opaque(opaque: Self::Opaque) -> Self { + $PackedArray { + opaque, + } + } + + fn opaque(&self) -> &Self::Opaque { + &self.opaque + } + + fn opaque_mut(&mut self) -> &mut Self::Opaque { + &mut self.opaque + } + } - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result + impl sys::GodotFfiBorrowable for $PackedArray { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const $Opaque) } } diff --git a/godot-core/src/builtin/plane.rs b/godot-core/src/builtin/plane.rs index 3630cdf6d..7ae03d668 100644 --- a/godot-core/src/builtin/plane.rs +++ b/godot-core/src/builtin/plane.rs @@ -267,7 +267,30 @@ unsafe impl GodotFfi for Plane { sys::VariantType::Plane } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; + fn new_from_sys; + fn sys; + fn sys_mut; + fn new_with_uninit; + fn from_arg_ptr; + fn move_return_ptr; + } + + fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self + where + Self: Sized, + { + let mut plane = Self::new(Vector3::UP, 0.0); + init_fn(&mut plane); + + plane + } +} + +impl sys::GodotFfiBorrowable for Plane { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } } impl_godot_as_self!(Plane); diff --git a/godot-core/src/builtin/projection.rs b/godot-core/src/builtin/projection.rs index 2489ae923..b157eaf3b 100644 --- a/godot-core/src/builtin/projection.rs +++ b/godot-core/src/builtin/projection.rs @@ -536,6 +536,12 @@ unsafe impl GodotFfi for Projection { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Projection { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Projection); /// A projection's clipping plane. diff --git a/godot-core/src/builtin/quaternion.rs b/godot-core/src/builtin/quaternion.rs index ac5826528..ee4032a34 100644 --- a/godot-core/src/builtin/quaternion.rs +++ b/godot-core/src/builtin/quaternion.rs @@ -287,6 +287,12 @@ unsafe impl GodotFfi for Quaternion { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Quaternion { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Quaternion); impl std::fmt::Display for Quaternion { diff --git a/godot-core/src/builtin/rect2.rs b/godot-core/src/builtin/rect2.rs index 04102d574..6dc06e36e 100644 --- a/godot-core/src/builtin/rect2.rs +++ b/godot-core/src/builtin/rect2.rs @@ -264,6 +264,12 @@ unsafe impl GodotFfi for Rect2 { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Rect2 { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Rect2); impl ApproxEq for Rect2 { diff --git a/godot-core/src/builtin/rect2i.rs b/godot-core/src/builtin/rect2i.rs index aa9fa8afa..ad71ad857 100644 --- a/godot-core/src/builtin/rect2i.rs +++ b/godot-core/src/builtin/rect2i.rs @@ -274,6 +274,12 @@ unsafe impl GodotFfi for Rect2i { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Rect2i { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Rect2i); impl std::fmt::Display for Rect2i { diff --git a/godot-core/src/builtin/rid.rs b/godot-core/src/builtin/rid.rs index cc3da542f..002b459da 100644 --- a/godot-core/src/builtin/rid.rs +++ b/godot-core/src/builtin/rid.rs @@ -123,7 +123,30 @@ unsafe impl GodotFfi for Rid { sys::VariantType::Rid } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; + fn new_from_sys; + fn sys; + fn sys_mut; + fn new_with_uninit; + fn from_arg_ptr; + fn move_return_ptr; + } + + fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self + where + Self: Sized, + { + let mut rid = Rid::Invalid; + init_fn(&mut rid); + + rid + } +} + +impl sys::GodotFfiBorrowable for Rid { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } } impl_godot_as_self!(Rid); diff --git a/godot-core/src/builtin/signal.rs b/godot-core/src/builtin/signal.rs index 906e18301..5a016e51c 100644 --- a/godot-core/src/builtin/signal.rs +++ b/godot-core/src/builtin/signal.rs @@ -9,6 +9,7 @@ use std::fmt; use std::ptr; use godot_ffi as sys; +use sys::GodotFfiBorrowable; use super::meta::{impl_godot_as_self, FromGodot, GodotType}; use crate::builtin::meta::ToGodot; @@ -21,7 +22,7 @@ use sys::{ffi_methods, GodotFfi}; /// A `Signal` represents a signal of an Object instance in Godot. /// /// Signals are composed of a reference to an `Object` and the name of the signal on this object. -#[repr(C, align(8))] +#[repr(transparent)] pub struct Signal { opaque: sys::types::OpaqueSignal, } @@ -44,7 +45,7 @@ impl Signal { sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(signal_from_object_signal); let raw = object.to_ffi(); - let args = [raw.as_arg_ptr(), signal_name.sys_const()]; + let args = [raw.as_arg_ptr(), signal_name.sys()]; ctor(self_ptr, args.as_ptr()); }) } @@ -55,7 +56,7 @@ impl Signal { /// _Godot equivalent: `Signal()`_ pub fn invalid() -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::new_with_uninit(|self_ptr| { let ctor = sys::builtin_fn!(signal_construct_default); ctor(self_ptr, ptr::null_mut()) }) @@ -163,24 +164,35 @@ unsafe impl GodotFfi for Signal { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; fn sys; - fn from_sys_init; + fn sys_mut; + fn new_with_uninit; fn move_return_ptr; } unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - Self::from_sys(ptr) + let signal = Self::borrow_sys(ptr); + signal.clone() } - #[cfg(before_api = "4.1")] - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { + fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self { let mut result = Self::invalid(); - init_fn(result.sys_mut()); + init_fn(&mut result); result } } +sys::unsafe_impl_has_opaque!(Signal { + opaque: sys::types::OpaqueSignal +}); + +impl GodotFfiBorrowable for Signal { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const sys::types::OpaqueSignal) + } +} + impl_builtin_traits! { for Signal { Clone => signal_construct_copy; diff --git a/godot-core/src/builtin/string/gstring.rs b/godot-core/src/builtin/string/gstring.rs index 435a76560..c4c2d30b3 100644 --- a/godot-core/src/builtin/string/gstring.rs +++ b/godot-core/src/builtin/string/gstring.rs @@ -9,7 +9,7 @@ use std::{convert::Infallible, ffi::c_char, fmt, str::FromStr}; use godot_ffi as sys; use sys::types::OpaqueString; -use sys::{ffi_methods, interface_fn, GodotFfi}; +use sys::{ffi_methods, interface_fn, AsUninit, GodotFfi, GodotFfiBorrowable}; use crate::builtin::inner; use crate::builtin::meta::impl_godot_as_self; @@ -125,9 +125,10 @@ impl GString { ffi_methods! { type sys::GDExtensionStringPtr = *mut Opaque; - fn from_string_sys = from_sys; - fn from_string_sys_init = from_sys_init; + fn new_from_string_sys = new_from_sys; + fn new_with_uninit_string_sys = new_with_uninit; fn string_sys = sys; + fn string_sys_mut = sys_mut; } /// Move `self` into a system pointer. This transfers ownership and thus does not call the destructor. @@ -159,22 +160,27 @@ unsafe impl GodotFfi for GString { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; fn sys; - fn from_sys_init; + fn sys_mut; + fn new_with_uninit; + fn new_with_init; fn move_return_ptr; } unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - let string = Self::from_sys(ptr); - std::mem::forget(string.clone()); - string + let string = Self::borrow_sys(ptr); + string.clone() } +} + +sys::unsafe_impl_has_opaque!(GString { + opaque: OpaqueString +}); - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result +impl GodotFfiBorrowable for GString { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const OpaqueString) } } @@ -217,7 +223,7 @@ where let bytes = s.as_ref().as_bytes(); unsafe { - Self::from_string_sys_init(|string_ptr| { + Self::new_with_uninit_string_sys(|string_ptr| { let ctor = interface_fn!(string_new_with_utf8_chars_and_len); ctor( string_ptr, @@ -275,7 +281,7 @@ impl From<&StringName> for GString { unsafe { sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(string_from_string_name); - let args = [string.sys_const()]; + let args = [string.sys()]; ctor(self_ptr, args.as_ptr()); }) } @@ -296,7 +302,7 @@ impl From<&NodePath> for GString { unsafe { sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(string_from_node_path); - let args = [path.sys_const()]; + let args = [path.sys()]; ctor(self_ptr, args.as_ptr()); }) } diff --git a/godot-core/src/builtin/string/node_path.rs b/godot-core/src/builtin/string/node_path.rs index da3ef2068..de3a9939a 100644 --- a/godot-core/src/builtin/string/node_path.rs +++ b/godot-core/src/builtin/string/node_path.rs @@ -8,7 +8,8 @@ use std::fmt; use godot_ffi as sys; -use godot_ffi::{ffi_methods, GDExtensionTypePtr, GodotFfi}; +use godot_ffi::{ffi_methods, GodotFfi}; +use sys::GodotFfiBorrowable; use crate::builtin::inner; use crate::builtin::meta::impl_godot_as_self; @@ -16,7 +17,7 @@ use crate::builtin::meta::impl_godot_as_self; use super::{GString, StringName}; /// A pre-parsed scene tree path. -#[repr(C)] +#[repr(transparent)] pub struct NodePath { opaque: sys::types::OpaqueNodePath, } @@ -59,22 +60,27 @@ unsafe impl GodotFfi for NodePath { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; fn sys; - fn from_sys_init; + fn sys_mut; + fn new_with_uninit; + fn new_with_init; fn move_return_ptr; } unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - let node_path = Self::from_sys(ptr); - std::mem::forget(node_path.clone()); - node_path + let node_path = Self::borrow_sys(ptr); + node_path.clone() } +} + +sys::unsafe_impl_has_opaque!(NodePath { + opaque: sys::types::OpaqueNodePath +}); - unsafe fn from_sys_init_default(init_fn: impl FnOnce(GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result +impl GodotFfiBorrowable for NodePath { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const sys::types::OpaqueNodePath) } } @@ -126,7 +132,7 @@ impl From<&GString> for NodePath { unsafe { sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(node_path_from_string); - let args = [string.sys_const()]; + let args = [string.sys()]; ctor(self_ptr, args.as_ptr()); }) } diff --git a/godot-core/src/builtin/string/string_name.rs b/godot-core/src/builtin/string/string_name.rs index 4ce8508b4..672a3ec88 100644 --- a/godot-core/src/builtin/string/string_name.rs +++ b/godot-core/src/builtin/string/string_name.rs @@ -8,7 +8,7 @@ use std::fmt; use godot_ffi as sys; -use sys::{ffi_methods, GodotFfi}; +use sys::{ffi_methods, GodotFfi, GodotFfiBorrowable}; use crate::builtin::inner; use crate::builtin::meta::impl_godot_as_self; @@ -59,7 +59,7 @@ impl StringName { // SAFETY: latin1_c_str is nul-terminated and remains valid for entire program duration. let result = unsafe { - Self::from_string_sys_init(|ptr| { + Self::new_with_uninit_string_sys(|ptr| { sys::interface_fn!(string_name_new_with_latin1_chars)( ptr, c_str.as_ptr(), @@ -114,15 +114,21 @@ impl StringName { ffi_methods! { type sys::GDExtensionStringNamePtr = *mut Opaque; - // Note: unlike from_sys, from_string_sys does not default-construct instance first. Typical usage in C++ is placement new. - fn from_string_sys = from_sys; - fn from_string_sys_init = from_sys_init; + fn new_from_string_sys = new_from_sys; + fn new_with_uninit_string_sys = new_with_uninit; fn string_sys = sys; + fn string_sys_mut = sys_mut; + } + + #[doc(hidden)] + pub unsafe fn borrow_string_sys<'a>(ptr: sys::GDExtensionConstStringNamePtr) -> &'a Self { + let ptr = ptr as sys::GDExtensionConstTypePtr; + Self::borrow_sys(ptr) } #[doc(hidden)] pub fn string_sys_const(&self) -> sys::GDExtensionConstStringNamePtr { - sys::to_const_ptr(self.string_sys()) + self.string_sys() } #[doc(hidden)] @@ -151,22 +157,27 @@ unsafe impl GodotFfi for StringName { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; fn sys; - fn from_sys_init; + fn sys_mut; + fn new_with_uninit; + fn new_with_init; fn move_return_ptr; } unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - let string_name = Self::from_sys(ptr); - string_name.inc_ref(); - string_name + let string_name = Self::borrow_sys(ptr); + string_name.clone() } +} - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result +sys::unsafe_impl_has_opaque!(StringName { + opaque: sys::types::OpaqueStringName +}); + +impl GodotFfiBorrowable for StringName { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const sys::types::OpaqueStringName) } } @@ -227,7 +238,7 @@ where // SAFETY: Rust guarantees validity and range of string. unsafe { - Self::from_string_sys_init(|ptr| { + Self::new_with_uninit_string_sys(|ptr| { sys::interface_fn!(string_name_new_with_utf8_chars_and_len)( ptr, utf8.as_ptr() as *const std::ffi::c_char, @@ -243,7 +254,7 @@ impl From<&GString> for StringName { unsafe { sys::from_sys_init_or_init_default::(|self_ptr| { let ctor = sys::builtin_fn!(string_name_from_string); - let args = [string.sys_const()]; + let args = [string.sys()]; ctor(self_ptr, args.as_ptr()); }) } diff --git a/godot-core/src/builtin/transform2d.rs b/godot-core/src/builtin/transform2d.rs index a5f13c61a..060b02d99 100644 --- a/godot-core/src/builtin/transform2d.rs +++ b/godot-core/src/builtin/transform2d.rs @@ -385,6 +385,12 @@ unsafe impl GodotFfi for Transform2D { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Transform2D { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Transform2D); /// A 2x2 matrix, typically used as an orthogonal basis for [`Transform2D`]. diff --git a/godot-core/src/builtin/transform3d.rs b/godot-core/src/builtin/transform3d.rs index 465913e16..ae1f7522e 100644 --- a/godot-core/src/builtin/transform3d.rs +++ b/godot-core/src/builtin/transform3d.rs @@ -392,6 +392,12 @@ unsafe impl GodotFfi for Transform3D { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } +impl sys::GodotFfiBorrowable for Transform3D { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *const Self) + } +} + impl_godot_as_self!(Transform3D); #[cfg(test)] diff --git a/godot-core/src/builtin/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index a022b3a91..b4b8f79e3 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -10,7 +10,6 @@ use crate::builtin::meta::{FromVariantError, GodotFfiVariant, GodotType, Propert use crate::builtin::*; use crate::engine::global; use godot_ffi as sys; -use sys::GodotFfi; // ---------------------------------------------------------------------------------------------------------------------------------------------- // Macro definitions @@ -26,9 +25,9 @@ macro_rules! impl_ffi_variant { impl GodotFfiVariant for $T { fn ffi_to_variant(&self) -> Variant { let variant = unsafe { - Variant::from_var_sys_init(|variant_ptr| { + Variant::new_with_uninit_var_sys(|variant_ptr| { let converter = sys::builtin_fn!($from_fn); - converter(variant_ptr, self.sys()); + converter(variant_ptr, godot_ffi::AsConst::force_mut(self.sys())); }) }; @@ -56,7 +55,7 @@ macro_rules! impl_ffi_variant { let result = unsafe { sys::from_sys_init_or_init_default(|self_ptr| { let converter = sys::builtin_fn!($to_fn); - converter(self_ptr, variant.var_sys()); + converter(self_ptr, godot_ffi::AsConst::force_mut(variant.var_sys())); }) }; diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index a12a472f4..0ed645391 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -9,7 +9,7 @@ use crate::builtin::{GString, StringName}; use godot_ffi as sys; use std::{fmt, ptr}; use sys::types::OpaqueVariant; -use sys::{ffi_methods, interface_fn, GodotFfi}; +use sys::{ffi_methods, interface_fn, GodotFfi, GodotFfiBorrowable}; mod impls; @@ -24,7 +24,7 @@ use super::meta::{impl_godot_as_self, ConvertError, FromGodot, ToGodot}; /// value. /// /// See also [Godot documentation for `Variant`](https://docs.godotengine.org/en/stable/classes/class_variant.html). -#[repr(C, align(8))] +#[repr(transparent)] pub struct Variant { opaque: OpaqueVariant, } @@ -81,7 +81,7 @@ impl Variant { let object_ptr = unsafe { crate::obj::raw_object_init(|type_ptr| { let converter = sys::builtin_fn!(object_from_variant); - converter(type_ptr, self.var_sys()); + converter(type_ptr, sys::AsConst::force_mut(self.var_sys())); }) }; @@ -111,13 +111,14 @@ impl Variant { } fn call_inner(&self, method: StringName, args: &[Variant]) -> Variant { - let args_sys: Vec<_> = args.iter().map(|v| v.var_sys_const()).collect(); + let args_sys: Vec<_> = args.iter().map(|v| v.var_sys()).collect(); let mut error = sys::default_call_error(); let result = unsafe { Variant::from_var_sys_init_or_init_default(|variant_ptr| { interface_fn!(variant_call)( - self.var_sys(), + // TODO: This method should strictly speaking take a `&mut self`. + sys::AsConst::force_mut(self.var_sys()), method.string_sys(), args_sys.as_ptr(), args_sys.len() as i64, @@ -177,7 +178,7 @@ impl Variant { pub fn stringify(&self) -> GString { let mut result = GString::new(); unsafe { - interface_fn!(variant_stringify)(self.var_sys(), result.string_sys()); + interface_fn!(variant_stringify)(self.var_sys(), result.string_sys_mut()); } result } @@ -212,19 +213,16 @@ impl Variant { ffi_methods! { type sys::GDExtensionVariantPtr = *mut Opaque; - fn from_var_sys = from_sys; - fn from_var_sys_init = from_sys_init; + fn new_from_var_sys = new_from_sys; + fn new_with_uninit_var_sys = new_with_uninit; fn var_sys = sys; + fn var_sys_mut = sys_mut; } #[doc(hidden)] - pub unsafe fn from_var_sys_init_default( - init_fn: impl FnOnce(sys::GDExtensionVariantPtr), - ) -> Self { - #[allow(unused_mut)] - let mut variant = Variant::nil(); - init_fn(variant.var_sys()); - variant + pub unsafe fn borrow_var_sys<'a>(ptr: sys::GDExtensionConstVariantPtr) -> &'a Self { + let ptr = ptr as sys::GDExtensionConstTypePtr; + Self::borrow_sys(ptr) } /// # Safety @@ -234,7 +232,7 @@ impl Variant { pub unsafe fn from_var_sys_init_or_init_default( init_fn: impl FnOnce(sys::GDExtensionVariantPtr), ) -> Self { - Self::from_var_sys_init_default(init_fn) + Self::new_with_init(|var| init_fn(var.var_sys())) } /// # Safety @@ -245,12 +243,7 @@ impl Variant { pub unsafe fn from_var_sys_init_or_init_default( init_fn: impl FnOnce(sys::GDExtensionUninitializedVariantPtr), ) -> Self { - Self::from_var_sys_init(init_fn) - } - - #[doc(hidden)] - pub fn var_sys_const(&self) -> sys::GDExtensionConstVariantPtr { - sys::to_const_ptr(self.var_sys()) + Self::new_with_uninit_var_sys(init_fn) } /// Converts to variant pointer; can be a null pointer. @@ -305,11 +298,15 @@ unsafe impl GodotFfi for Variant { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. } +} - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result +sys::unsafe_impl_has_opaque!(Variant { + opaque: OpaqueVariant +}); + +impl GodotFfiBorrowable for Variant { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + sys::HasOpaque::borrow_opaque(ptr as *const OpaqueVariant) } } @@ -318,7 +315,7 @@ impl_godot_as_self!(Variant); impl Default for Variant { fn default() -> Self { unsafe { - Self::from_var_sys_init(|variant_ptr| { + Self::new_with_uninit_var_sys(|variant_ptr| { interface_fn!(variant_new_nil)(variant_ptr); }) } @@ -328,7 +325,7 @@ impl Default for Variant { impl Clone for Variant { fn clone(&self) -> Self { unsafe { - Self::from_var_sys_init(|variant_ptr| { + Self::new_with_uninit_var_sys(|variant_ptr| { interface_fn!(variant_new_copy)(variant_ptr, self.var_sys()); }) } @@ -338,7 +335,7 @@ impl Clone for Variant { impl Drop for Variant { fn drop(&mut self) { unsafe { - interface_fn!(variant_destroy)(self.var_sys()); + interface_fn!(variant_destroy)(self.var_sys_mut()); } } } diff --git a/godot-core/src/engine/script_instance.rs b/godot-core/src/engine/script_instance.rs index 8bbd38408..a0f582ba2 100644 --- a/godot-core/src/engine/script_instance.rs +++ b/godot-core/src/engine/script_instance.rs @@ -317,7 +317,6 @@ mod script_instance_info { use std::cell::{BorrowError, Ref, RefMut}; use std::ffi::c_void; use std::mem::ManuallyDrop; - use std::ops::Deref; use crate::builtin::{GString, StringName, Variant}; use crate::engine::ScriptLanguage; @@ -351,23 +350,6 @@ mod script_instance_info { &*(p_instance as *mut ScriptInstanceData) } - /// # Safety - /// - /// - We expect the engine to provide a valid const string name pointer. - unsafe fn transfer_string_name_from_godot( - p_name: sys::GDExtensionConstStringNamePtr, - ) -> StringName { - // This `StringName` is not ours and the engine will decrement the reference count later. To own our own `StringName` reference we - // cannot call the destructor on the "original" `StringName`, but have to clone it to increase the reference count. This new instance can - // then be passed around and eventually droped which will decrement the reference count again. - // - // By wrapping the initial `StringName` in a `ManuallyDrop` the destructor is prevented from being executed, which would decrement the - // reference count. - ManuallyDrop::new(StringName::from_string_sys(sys::force_mut_ptr(p_name))) - .deref() - .clone() - } - fn transfer_bool_to_godot(value: bool) -> sys::GDExtensionBool { value as sys::GDExtensionBool } @@ -452,7 +434,7 @@ mod script_instance_info { p_name: sys::GDExtensionConstStringNamePtr, p_value: sys::GDExtensionConstVariantPtr, ) -> sys::GDExtensionBool { - let name = transfer_string_name_from_godot(p_name); + let name = StringName::new_from_string_sys(p_name); let value = &*Variant::ptr_from_sys(p_value); let ctx = || format!("error when calling {}::set", type_name::()); @@ -476,7 +458,7 @@ mod script_instance_info { p_name: sys::GDExtensionConstStringNamePtr, r_ret: sys::GDExtensionVariantPtr, ) -> sys::GDExtensionBool { - let name = transfer_string_name_from_godot(p_name); + let name = StringName::new_from_string_sys(p_name); let ctx = || format!("error when calling {}::get", type_name::()); let return_value = handle_panic(ctx, || { @@ -596,7 +578,7 @@ mod script_instance_info { r_return: sys::GDExtensionVariantPtr, r_error: *mut sys::GDExtensionCallError, ) { - let method = transfer_string_name_from_godot(p_method); + let method = StringName::new_from_string_sys(p_method); let args = Variant::unbounded_refs_from_sys(p_args, p_argument_count as usize); let ctx = || format!("error when calling {}::call", type_name::()); @@ -667,7 +649,7 @@ mod script_instance_info { p_instance: sys::GDExtensionScriptInstanceDataPtr, p_method: sys::GDExtensionConstStringNamePtr, ) -> sys::GDExtensionBool { - let method = transfer_string_name_from_godot(p_method); + let method = StringName::new_from_string_sys(p_method); let ctx = || format!("error when calling {}::has_method", type_name::()); let has_method = handle_panic(ctx, || { @@ -729,7 +711,7 @@ mod script_instance_info { type_name::() ) }; - let name = transfer_string_name_from_godot(p_name); + let name = StringName::new_from_string_sys(p_name); let result = handle_panic(ctx, || { let instance = instance_data_as_script_instance::(p_instance); @@ -900,7 +882,7 @@ mod script_instance_info { p_name: sys::GDExtensionConstStringNamePtr, r_ret: sys::GDExtensionVariantPtr, ) -> sys::GDExtensionBool { - let name = transfer_string_name_from_godot(p_name); + let name = StringName::new_from_string_sys(p_name); let ctx = || { format!( @@ -934,7 +916,7 @@ mod script_instance_info { p_name: sys::GDExtensionConstStringNamePtr, p_value: sys::GDExtensionConstVariantPtr, ) -> sys::GDExtensionBool { - let name = transfer_string_name_from_godot(p_name); + let name = StringName::new_from_string_sys(p_name); let value = &*Variant::ptr_from_sys(p_value); let ctx = || { diff --git a/godot-core/src/init/mod.rs b/godot-core/src/init/mod.rs index 6403c5387..e3b13251a 100644 --- a/godot-core/src/init/mod.rs +++ b/godot-core/src/init/mod.rs @@ -261,8 +261,8 @@ fn ensure_godot_features_compatible() { // SAFETY: main thread, after initialize(), valid string pointers. let gdext_is_double = cfg!(feature = "double-precision"); let godot_is_double = unsafe { - let is_single = sys::godot_has_feature(os_class.string_sys(), single.sys_const()); - let is_double = sys::godot_has_feature(os_class.string_sys(), double.sys_const()); + let is_single = sys::godot_has_feature(os_class.string_sys(), single.sys()); + let is_double = sys::godot_has_feature(os_class.string_sys(), double.sys()); assert_ne!( is_single, is_double, diff --git a/godot-core/src/log.rs b/godot-core/src/log.rs index f7c450bea..6f1713b1c 100644 --- a/godot-core/src/log.rs +++ b/godot-core/src/log.rs @@ -93,11 +93,11 @@ pub fn print(varargs: &[Variant]) { let call_fn = call_fn.unwrap_unchecked(); let mut args = Vec::new(); - args.extend(varargs.iter().map(Variant::sys_const)); + args.extend(varargs.iter().map(Variant::sys)); let args_ptr = args.as_ptr(); - let _variant = Variant::from_sys_init_default(|return_ptr| { - call_fn(return_ptr, args_ptr, args.len() as i32); + let _variant = Variant::new_with_init(|return_ptr| { + call_fn(return_ptr.sys_mut(), args_ptr, args.len() as i32); }); } diff --git a/godot-core/src/obj/raw.rs b/godot-core/src/obj/raw.rs index 43ffb0cc5..7be225e90 100644 --- a/godot-core/src/obj/raw.rs +++ b/godot-core/src/obj/raw.rs @@ -444,16 +444,33 @@ where sys::VariantType::Object } - unsafe fn from_sys(ptr: sys::GDExtensionTypePtr) -> Self { + unsafe fn new_from_sys(ptr: sys::GDExtensionConstTypePtr) -> Self { Self::from_obj_sys_weak(ptr as sys::GDExtensionObjectPtr) } - unsafe fn from_sys_init(init: impl FnOnce(sys::GDExtensionUninitializedTypePtr)) -> Self { + unsafe fn new_with_uninit(init: impl FnOnce(sys::GDExtensionUninitializedTypePtr)) -> Self { let obj = raw_object_init(init); Self::from_obj_sys_weak(obj) } - fn sys(&self) -> sys::GDExtensionTypePtr { + fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self + where + Self: Sized, + { + let mut obj = Self { + obj: std::ptr::null_mut(), + // We allow the `init_fn` to change this if need be, since this object isn't _really_ fully initialized yet. + cached_rtti: None, + }; + init_fn(&mut obj); + obj + } + + fn sys(&self) -> sys::GDExtensionConstTypePtr { + self.obj as sys::GDExtensionConstTypePtr + } + + fn sys_mut(&mut self) -> sys::GDExtensionTypePtr { self.obj as sys::GDExtensionTypePtr } @@ -647,7 +664,7 @@ impl GodotFfiVariant for RawGd { // (This behaves differently in the opposite direction `variant_to_object`.) unsafe { - Variant::from_var_sys_init(|variant_ptr| { + Variant::new_with_uninit_var_sys(|variant_ptr| { let converter = sys::builtin_fn!(object_to_variant); // Note: this is a special case because of an inconsistency in Godot, where sometimes the equivalency is @@ -670,9 +687,9 @@ impl GodotFfiVariant for RawGd { // TODO(uninit) - see if we can use from_sys_init() // raw_object_init? - RawGd::::from_sys_init(|self_ptr| { + RawGd::::new_with_uninit(|self_ptr| { let converter = sys::builtin_fn!(object_from_variant); - converter(self_ptr, variant.var_sys()); + converter(self_ptr, sys::AsConst::force_mut(variant.var_sys())); }) }; diff --git a/godot-core/src/registry/callbacks.rs b/godot-core/src/registry/callbacks.rs index 5ed7a2333..5341010e8 100644 --- a/godot-core/src/registry/callbacks.rs +++ b/godot-core/src/registry/callbacks.rs @@ -16,7 +16,7 @@ use crate::obj::{cap, Base, GodotClass, UserClass}; use crate::storage::{as_storage, InstanceStorage, Storage, StorageRefCounted}; use godot_ffi as sys; use std::any::Any; -use sys::interface_fn; +use sys::{interface_fn, GodotFfiBorrowable}; pub unsafe extern "C" fn create( _class_userdata: *mut std::ffi::c_void, @@ -102,10 +102,8 @@ pub unsafe extern "C" fn get_virtual( _class_user_data: *mut std::ffi::c_void, name: sys::GDExtensionConstStringNamePtr, ) -> sys::GDExtensionClassCallVirtual { - // This string is not ours, so we cannot call the destructor on it. - let borrowed_string = StringName::from_string_sys(sys::force_mut_ptr(name)); + let borrowed_string = StringName::borrow_string_sys(name); let method_name = borrowed_string.to_string(); - std::mem::forget(borrowed_string); T::__virtual_call(method_name.as_str()) } @@ -114,10 +112,8 @@ pub unsafe extern "C" fn default_get_virtual( _class_user_data: *mut std::ffi::c_void, name: sys::GDExtensionConstStringNamePtr, ) -> sys::GDExtensionClassCallVirtual { - // This string is not ours, so we cannot call the destructor on it. - let borrowed_string = StringName::from_string_sys(sys::force_mut_ptr(name)); + let borrowed_string = StringName::borrow_string_sys(name); let method_name = borrowed_string.to_string(); - std::mem::forget(borrowed_string); T::__default_virtual_call(method_name.as_str()) } @@ -168,11 +164,9 @@ pub unsafe extern "C" fn get_property( ) -> sys::GDExtensionBool { let storage = as_storage::(instance); let instance = storage.get(); - let property = StringName::from_string_sys(sys::force_mut_ptr(name)); + let property = StringName::borrow_string_sys(name); - std::mem::forget(property.clone()); - - match T::__godot_get_property(&*instance, property) { + match T::__godot_get_property(&*instance, property.clone()) { Some(value) => { value.move_var_ptr(ret); true as sys::GDExtensionBool @@ -189,13 +183,10 @@ pub unsafe extern "C" fn set_property( let storage = as_storage::(instance); let mut instance = storage.get_mut(); - let property = StringName::from_string_sys(sys::force_mut_ptr(name)); - let value = Variant::from_var_sys(sys::force_mut_ptr(value)); - - std::mem::forget(property.clone()); - std::mem::forget(value.clone()); + let property = StringName::borrow_string_sys(name); + let value = Variant::borrow_var_sys(value); - T::__godot_set_property(&mut *instance, property, value) as sys::GDExtensionBool + T::__godot_set_property(&mut *instance, property.clone(), value.clone()) as sys::GDExtensionBool } pub unsafe extern "C" fn reference(instance: sys::GDExtensionClassInstancePtr) { diff --git a/godot-ffi/src/extras.rs b/godot-ffi/src/extras.rs index 78712fe0c..85b647df3 100644 --- a/godot-ffi/src/extras.rs +++ b/godot-ffi/src/extras.rs @@ -55,6 +55,77 @@ impl_as_uninit!(GDExtensionStringPtr, GDExtensionUninitializedStringPtr); impl_as_uninit!(GDExtensionObjectPtr, GDExtensionUninitializedObjectPtr); impl_as_uninit!(GDExtensionTypePtr, GDExtensionUninitializedTypePtr); +pub trait AsConst { + type Ptr; + + fn as_const(self) -> Self::Ptr; + + fn force_mut(uninit: Self::Ptr) -> Self; +} + +macro_rules! impl_as_const { + ($Ptr:ty, $Const:ty) => { + impl AsConst for $Ptr { + type Ptr = $Const; + + fn as_const(self) -> $Const { + self as $Const + } + + fn force_mut(const_: Self::Ptr) -> Self { + const_ as Self + } + } + }; +} + +#[rustfmt::skip] +impl_as_const!(GDExtensionStringNamePtr, GDExtensionConstStringNamePtr); +impl_as_const!(GDExtensionVariantPtr, GDExtensionConstVariantPtr); +impl_as_const!(GDExtensionStringPtr, GDExtensionConstStringPtr); +impl_as_const!(GDExtensionObjectPtr, GDExtensionConstObjectPtr); +impl_as_const!(GDExtensionTypePtr, GDExtensionConstTypePtr); + +#[doc(hidden)] +pub trait HasOpaque { + type Opaque; + + unsafe fn borrow_opaque<'a>(ptr: *const Self::Opaque) -> &'a Self; + fn from_opaque(opaque: Self::Opaque) -> Self; + fn opaque(&self) -> &Self::Opaque; + fn opaque_mut(&mut self) -> &mut Self::Opaque; +} + +/// Implement [`HasOpaque`] for a newtype around an opaque field. +/// +/// # Safety +/// +/// `Ty` must be a `#[repr(transparent)]` type with a single field named `$field` with type `$OpaqueTy`. +#[macro_export] +macro_rules! unsafe_impl_has_opaque { + ($Ty:ty { $field:ident: $OpaqueTy:ty }) => { + impl $crate::HasOpaque for $Ty { + type Opaque = $OpaqueTy; + + unsafe fn borrow_opaque<'a>(ptr: *const Self::Opaque) -> &'a Self { + &*(ptr as *const Self) + } + + fn from_opaque(opaque: Self::Opaque) -> Self { + Self { $field: opaque } + } + + fn opaque(&self) -> &Self::Opaque { + &self.$field + } + + fn opaque_mut(&mut self) -> &mut Self::Opaque { + &mut self.$field + } + } + }; +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Helper functions diff --git a/godot-ffi/src/godot_ffi.rs b/godot-ffi/src/godot_ffi.rs index 3f9fba75f..726a36d64 100644 --- a/godot-ffi/src/godot_ffi.rs +++ b/godot-ffi/src/godot_ffi.rs @@ -17,6 +17,8 @@ use std::marker::PhantomData; /// /// [`from_arg_ptr`](GodotFfi::from_arg_ptr) and [`move_return_ptr`](GodotFfi::move_return_ptr) /// must properly initialize and clean up values given the [`PtrcallType`] provided by the caller. +/// +/// It must be safe to mutate `self` from the pointer returned from [`sys_mut`](GodotFfi::sys_mut). #[doc(hidden)] // shows up in implementors otherwise pub unsafe trait GodotFfi { fn variant_type() -> sys::VariantType; @@ -29,64 +31,42 @@ pub unsafe trait GodotFfi { /// # Safety /// `ptr` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`, /// which is different depending on the type. - /// The type in `ptr` must not require any special consideration upon referencing. Such as - /// incrementing a refcount. - unsafe fn from_sys(ptr: sys::GDExtensionTypePtr) -> Self; + unsafe fn new_from_sys(ptr: sys::GDExtensionConstTypePtr) -> Self; /// Construct uninitialized opaque data, then initialize it with `init_fn` function. /// /// # Safety /// `init_fn` must be a function that correctly handles a (possibly-uninitialized) _type ptr_. - unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionUninitializedTypePtr)) -> Self; + unsafe fn new_with_uninit(init_fn: impl FnOnce(sys::GDExtensionUninitializedTypePtr)) -> Self; - /// Like [`Self::from_sys_init`], but pre-initializes the sys pointer to a `Default::default()` instance + /// Like [`Self::from_sys_init`], but pre-initializes the sys pointer to an initialized instance /// before calling `init_fn`. /// + /// This means that this function is entirely safe to call, since we call the `init_fn` with a known validly initialized value. + /// This doesn't mean the value is useful, it may for instance be a null object, but it is at least a valid instance. + /// /// Some FFI functions in Godot expect a pre-existing instance at the destination pointer, e.g. CoW/ref-counted /// builtin types like `Array`, `Dictionary`, `String`, `StringName`. - /// - /// If not overridden, this just calls [`Self::from_sys_init`]. - /// - /// # Safety - /// `init_fn` must be a function that correctly handles a (possibly-uninitialized) _type ptr_. - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self + fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self where - Self: Sized, // + Default - { - // SAFETY: this default implementation is potentially incorrect. - // By implementing the GodotFfi trait, you acknowledge that these may need to be overridden. - Self::from_sys_init(|ptr| init_fn(sys::AsUninit::force_init(ptr))) - - // TODO consider using this, if all the implementors support it - // let mut result = Self::default(); - // init_fn(result.sys_mut().as_uninit()); - // result - } + Self: Sized; /// Return Godot opaque pointer, for an immutable operation. /// /// Note that this is a `*mut` pointer despite taking `&self` by shared-ref. /// This is because most of Godot's Rust API is not const-correct. This can still /// enhance user code (calling `sys_mut` ensures no aliasing at the time of the call). - fn sys(&self) -> sys::GDExtensionTypePtr; + fn sys(&self) -> sys::GDExtensionConstTypePtr; /// Return Godot opaque pointer, for a mutable operation. /// - /// Should usually not be overridden; behaves like `sys()` but ensures no aliasing - /// at the time of the call (not necessarily during any subsequent modifications though). - fn sys_mut(&mut self) -> sys::GDExtensionTypePtr { - self.sys() - } - - // TODO check if sys() can take over this - // also, from_sys() might take *const T - // possibly separate 2 pointer types - fn sys_const(&self) -> sys::GDExtensionConstTypePtr { - self.sys() - } + /// This method ensures no aliasing, as well as allowing mutation of the original value through the pointer this method returns. + // Cannot implement this as `self.sys()` since that would make the returned pointer derived from an immutable reference, meaning + // that modifying the value through the returned pointer would be UB. + fn sys_mut(&mut self) -> sys::GDExtensionTypePtr; fn as_arg_ptr(&self) -> sys::GDExtensionConstTypePtr { - self.sys_const() + self.sys() } /// Construct from a pointer to an argument in a call. @@ -109,6 +89,17 @@ pub unsafe trait GodotFfi { unsafe fn move_return_ptr(self, dst: sys::GDExtensionTypePtr, call_type: PtrcallType); } +#[doc(hidden)] +pub trait GodotFfiBorrowable: GodotFfi { + /// Construct a reference from Godot opaque pointer. + /// + /// # Safety + /// `ptr` must be a valid _type ptr_: it must follow Godot's convention to encode `Self`, + /// which is different depending on the type. + /// `ptr` must be a valid reference to `Self` for the lifetime of `'a`. + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self; +} + // In Godot 4.0.x, a lot of that are "constructed into" require a default-initialized value. // In Godot 4.1+, placement new is used, requiring no prior value. // This method abstracts over that. Outside of GodotFfi because it should not be overridden. @@ -120,7 +111,7 @@ pub unsafe trait GodotFfi { pub unsafe fn from_sys_init_or_init_default( init_fn: impl FnOnce(sys::GDExtensionTypePtr), ) -> T { - T::from_sys_init_default(init_fn) + T::new_with_init(init_fn) } /// # Safety @@ -130,7 +121,7 @@ pub unsafe fn from_sys_init_or_init_default( pub unsafe fn from_sys_init_or_init_default( init_fn: impl FnOnce(sys::GDExtensionUninitializedTypePtr), ) -> T { - T::from_sys_init(init_fn) + T::new_with_uninit(init_fn) } // ---------------------------------------------------------------------------------------------------------------------------------------------- @@ -183,96 +174,92 @@ pub enum PtrcallType { #[doc(hidden)] macro_rules! ffi_methods_one { // type $Ptr = *mut Opaque - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $new_from_sys:ident = new_from_sys) => { $( #[$attr] )? $vis - unsafe fn $from_sys(ptr: $Ptr) -> Self { - let opaque = std::ptr::read(ptr as *mut _); - Self::from_opaque(opaque) + unsafe fn $new_from_sys(ptr: <$Ptr as $crate::AsConst>::Ptr) -> Self { + ::borrow_sys(ptr as sys::GDExtensionConstTypePtr).clone() } }; - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $new_with_uninit:ident = new_with_uninit) => { $( #[$attr] )? $vis - unsafe fn $from_sys_init(init: impl FnOnce(<$Ptr as $crate::AsUninit>::Ptr)) -> Self { - let mut raw = std::mem::MaybeUninit::uninit(); + unsafe fn $new_with_uninit(init: impl FnOnce(<$Ptr as $crate::AsUninit>::Ptr)) -> Self { + let mut raw: std::mem::MaybeUninit<::Opaque> = std::mem::MaybeUninit::uninit(); init(raw.as_mut_ptr() as <$Ptr as $crate::AsUninit>::Ptr); - Self::from_opaque(raw.assume_init()) - } - }; - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { - $( #[$attr] )? $vis - fn $sys(&self) -> $Ptr { - &self.opaque as *const _ as $Ptr + ::from_opaque(raw.assume_init()) } }; - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_arg_ptr:ident = from_arg_ptr) => { - $( #[$attr] )? $vis - unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::PtrcallType) -> Self { - Self::from_sys(ptr as *mut _) - } - }; - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $move_return_ptr:ident = move_return_ptr) => { + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $new_with_init:ident = new_with_init) => { $( #[$attr] )? $vis - unsafe fn $move_return_ptr(mut self, dst: $Ptr, _call_type: $crate::PtrcallType) { - std::ptr::swap(dst as *mut _, std::ptr::addr_of_mut!(self.opaque)) - } - }; + fn $new_with_init(init: impl FnOnce(&mut Self)) -> Self { + let mut default: Self = Default::default(); + init(&mut default); - // type $Ptr = Opaque - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { - $( #[$attr] )? $vis - unsafe fn $from_sys(ptr: $Ptr) -> Self { - let opaque = std::mem::transmute(ptr); - Self::from_opaque(opaque) + default } }; - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { $( #[$attr] )? $vis - unsafe fn $from_sys_init(init: impl FnOnce(<$Ptr as $crate::AsUninit>::Ptr)) -> Self { - let mut raw = std::mem::MaybeUninit::uninit(); - init(std::mem::transmute(raw.as_mut_ptr())); - Self::from_opaque(raw.assume_init()) + fn $sys(&self) -> <$Ptr as $crate::AsConst>::Ptr { + let ptr = ::opaque(self) as *const ::Opaque as $Ptr; + $crate::AsConst::as_const(ptr) } }; - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys_mut:ident = sys_mut) => { $( #[$attr] )? $vis - fn $sys(&self) -> $Ptr { - unsafe { std::mem::transmute(self.opaque) } + fn $sys_mut(&mut self) -> $Ptr { + ::opaque_mut(self) as *mut ::Opaque as $Ptr } }; - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_arg_ptr:ident = from_arg_ptr) => { + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_arg_ptr:ident = from_arg_ptr) => { $( #[$attr] )? $vis unsafe fn $from_arg_ptr(ptr: $Ptr, _call_type: $crate::PtrcallType) -> Self { - Self::from_sys(ptr as *mut _) + Self::new_from_sys(ptr as *mut _) } }; - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $move_return_ptr:ident = move_return_ptr) => { + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $move_return_ptr:ident = move_return_ptr) => { $( #[$attr] )? $vis unsafe fn $move_return_ptr(mut self, dst: $Ptr, _call_type: $crate::PtrcallType) { - std::ptr::swap(dst, std::mem::transmute::<_, $Ptr>(self.opaque)) + let opaque_ptr = ::opaque_mut(&mut self) as *mut ::Opaque; + std::ptr::swap(dst as *mut ::Opaque, opaque_ptr) } }; // type $Ptr = *mut Self - (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $new_from_sys:ident = new_from_sys) => { $( #[$attr] )? $vis - unsafe fn $from_sys(ptr: $Ptr) -> Self { - *(ptr as *mut Self) + unsafe fn $new_from_sys(ptr: <$Ptr as $crate::AsConst>::Ptr) -> Self { + *(ptr as *const Self) } }; - (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $new_with_uninit:ident = new_with_uninit) => { $( #[$attr] )? $vis - unsafe fn $from_sys_init(init: impl FnOnce(<$Ptr as $crate::AsUninit>::Ptr)) -> Self { + unsafe fn $new_with_uninit(init: impl FnOnce(<$Ptr as $crate::AsUninit>::Ptr)) -> Self { let mut raw = std::mem::MaybeUninit::::uninit(); init(raw.as_mut_ptr() as <$Ptr as $crate::AsUninit>::Ptr); raw.assume_init() } }; + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $new_with_init:ident = new_with_init) => { + $( #[$attr] )? $vis + fn $new_with_init(init: impl FnOnce(&mut Self)) -> Self { + let mut default: Self = Default::default(); + init(&mut default); + + default + } + }; (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { $( #[$attr] )? $vis - fn $sys(&self) -> $Ptr { - self as *const Self as $Ptr + fn $sys(&self) -> <$Ptr as $crate::AsConst>::Ptr { + $crate::AsConst::as_const(self as *const Self as $Ptr) + } + }; + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys_mut:ident = sys_mut) => { + $( #[$attr] )? $vis + fn $sys_mut(&mut self) -> $Ptr { + self as *mut Self as $Ptr } }; (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_arg_ptr:ident = from_arg_ptr) => { @@ -307,9 +294,11 @@ macro_rules! ffi_methods_rest { ( // impl GodotFfi for T (default all 5) $Impl:ident $Ptr:ty; .. ) => { - $crate::ffi_methods_one!($Impl $Ptr; from_sys = from_sys); - $crate::ffi_methods_one!($Impl $Ptr; from_sys_init = from_sys_init); + $crate::ffi_methods_one!($Impl $Ptr; new_from_sys = new_from_sys); + $crate::ffi_methods_one!($Impl $Ptr; new_with_uninit = new_with_uninit); + $crate::ffi_methods_one!($Impl $Ptr; new_with_init = new_with_init); $crate::ffi_methods_one!($Impl $Ptr; sys = sys); + $crate::ffi_methods_one!($Impl $Ptr; sys_mut = sys_mut); $crate::ffi_methods_one!($Impl $Ptr; from_arg_ptr = from_arg_ptr); $crate::ffi_methods_one!($Impl $Ptr; move_return_ptr = move_return_ptr); }; @@ -419,7 +408,7 @@ where // Implementation for common types (needs to be this crate due to orphan rule) mod scalars { use super::GodotFfi; - use crate as sys; + use crate::{self as sys, GodotFfiBorrowable}; /* macro_rules! impl_godot_marshalling { @@ -480,6 +469,12 @@ mod scalars { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } + impl GodotFfiBorrowable for bool { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *mut bool) + } + } + unsafe impl GodotFfi for i64 { fn variant_type() -> sys::VariantType { sys::VariantType::Int @@ -492,6 +487,12 @@ mod scalars { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } + impl GodotFfiBorrowable for i64 { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *mut i64) + } + } + unsafe impl GodotFfi for f64 { fn variant_type() -> sys::VariantType { sys::VariantType::Float @@ -504,22 +505,40 @@ mod scalars { ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } + impl GodotFfiBorrowable for f64 { + unsafe fn borrow_sys<'a>(ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &*(ptr as *mut f64) + } + } + unsafe impl GodotFfi for () { fn variant_type() -> sys::VariantType { sys::VariantType::Nil } - unsafe fn from_sys(_ptr: sys::GDExtensionTypePtr) -> Self { + unsafe fn new_from_sys(_ptr: sys::GDExtensionConstTypePtr) -> Self {} + + unsafe fn new_with_uninit( + _init: impl FnOnce(sys::GDExtensionUninitializedTypePtr), + ) -> Self { // Do nothing } - unsafe fn from_sys_init(_init: impl FnOnce(sys::GDExtensionUninitializedTypePtr)) -> Self { - // Do nothing + fn new_with_init(_init: impl FnOnce(&mut Self)) -> Self + where + Self: Sized, + { + // Do Nothing + } + + fn sys(&self) -> sys::GDExtensionConstTypePtr { + // ZST dummy pointer + self as *const _ as sys::GDExtensionConstTypePtr } - fn sys(&self) -> sys::GDExtensionTypePtr { + fn sys_mut(&mut self) -> sys::GDExtensionTypePtr { // ZST dummy pointer - self as *const _ as sys::GDExtensionTypePtr + self as *mut _ as sys::GDExtensionTypePtr } // SAFETY: @@ -540,4 +559,10 @@ mod scalars { // Do nothing } } + + impl GodotFfiBorrowable for () { + unsafe fn borrow_sys<'a>(_ptr: sys::GDExtensionConstTypePtr) -> &'a Self { + &() + } + } } diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index d5523f800..fbc2702f9 100644 --- a/godot-ffi/src/lib.rs +++ b/godot-ffi/src/lib.rs @@ -52,8 +52,8 @@ pub use paste; pub use gensym::gensym; pub use crate::godot_ffi::{ - from_sys_init_or_init_default, GodotFfi, GodotNullableFfi, PrimitiveConversionError, - PtrcallType, + from_sys_init_or_init_default, GodotFfi, GodotFfiBorrowable, GodotNullableFfi, + PrimitiveConversionError, PtrcallType, }; // Method tables diff --git a/itest/rust/src/builtin_tests/containers/variant_test.rs b/itest/rust/src/builtin_tests/containers/variant_test.rs index a1c65d7a2..9f3dbec42 100644 --- a/itest/rust/src/builtin_tests/containers/variant_test.rs +++ b/itest/rust/src/builtin_tests/containers/variant_test.rs @@ -261,7 +261,7 @@ fn variant_sys_conversion() { let v = Variant::from(7); let ptr = v.sys(); - let v2 = unsafe { Variant::from_sys(ptr) }; + let v2 = unsafe { Variant::new_from_sys(ptr) }; assert_eq!(v2, v); } @@ -281,7 +281,7 @@ fn variant_sys_conversion2() { }; let v2 = unsafe { - Variant::from_sys_init(|ptr| { + Variant::new_with_uninit(|ptr| { std::ptr::copy( buffer.as_ptr(), ptr as *mut u8, diff --git a/itest/rust/src/object_tests/object_test.rs b/itest/rust/src/object_tests/object_test.rs index 21a119601..7733405b8 100644 --- a/itest/rust/src/object_tests/object_test.rs +++ b/itest/rust/src/object_tests/object_test.rs @@ -55,7 +55,7 @@ fn object_user_roundtrip_return() { let ptr = raw.sys(); std::mem::forget(obj); - let raw2 = unsafe { RawGd::::from_sys(ptr) }; + let raw2 = unsafe { RawGd::::new_from_sys(ptr) }; let obj2 = Gd::from_ffi(raw2); assert_eq!(obj2.bind().value, value); } // drop @@ -70,7 +70,7 @@ fn object_user_roundtrip_write() { let raw = obj.to_ffi(); let raw2 = unsafe { - RawGd::::from_sys_init(|ptr| { + RawGd::::new_with_uninit(|ptr| { raw.move_return_ptr(sys::AsUninit::force_init(ptr), sys::PtrcallType::Standard) }) }; @@ -89,7 +89,7 @@ fn object_engine_roundtrip() { let raw = obj.to_ffi(); let ptr = raw.sys(); - let raw2 = unsafe { RawGd::::from_sys(ptr) }; + let raw2 = unsafe { RawGd::::new_from_sys(ptr) }; let obj2 = Gd::from_ffi(raw2); assert_eq!(obj2.get_position(), pos); obj.free(); diff --git a/itest/rust/src/register_tests/option_ffi_test.rs b/itest/rust/src/register_tests/option_ffi_test.rs index 32d61cdcb..f7f168532 100644 --- a/itest/rust/src/register_tests/option_ffi_test.rs +++ b/itest/rust/src/register_tests/option_ffi_test.rs @@ -19,7 +19,7 @@ fn option_some_sys_conversion() { let v_raw = v.to_ffi(); let ptr = v_raw.sys(); - let v2_raw = unsafe { RawGd::::from_sys(ptr) }; + let v2_raw = unsafe { RawGd::::new_from_sys(ptr) }; let v2 = Option::>::from_ffi(v2_raw); assert_eq!(v2, v); @@ -34,7 +34,7 @@ fn option_none_sys_conversion() { let v_raw = v.to_ffi(); let ptr = v_raw.sys(); - let v2_raw = unsafe { RawGd::::from_sys(ptr) }; + let v2_raw = unsafe { RawGd::::new_from_sys(ptr) }; let v2 = Option::>::from_ffi(v2_raw); assert_eq!(v2, v); }