diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs index 9fc67c6a6..6aa1eb0e9 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::SysPtr::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::SysPtr::force_mut(outer.sys()), } } } diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index 269a084ba..83e104c23 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -54,7 +54,6 @@ 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)] pub struct Array { // Safety Invariant: The type of all values in `opaque` matches the type `T`. opaque: sys::types::OpaqueArray, @@ -225,7 +224,7 @@ impl Array { /// # Panics /// /// If `index` is out of bounds. - fn ptr(&self, index: usize) -> *const Variant { + fn ptr(&self, index: usize) -> sys::GDExtensionConstVariantPtr { let ptr = self.ptr_or_null(index); assert!( !ptr.is_null(), @@ -236,7 +235,7 @@ impl Array { } /// Returns a pointer to the element at the given index, or null if out of bounds. - fn ptr_or_null(&self, index: usize) -> *const Variant { + fn ptr_or_null(&self, index: usize) -> sys::GDExtensionConstVariantPtr { // SAFETY: array_operator_index_const returns null for invalid indexes. let variant_ptr = unsafe { let index = to_i64(index); @@ -244,8 +243,7 @@ impl Array { }; // Signature is wrong in GDExtension, semantically this is a const ptr - let variant_ptr = sys::to_const_ptr(variant_ptr); - Variant::ptr_from_sys(variant_ptr) + sys::SysPtr::as_const(variant_ptr) } /// Returns a mutable pointer to the element at the given index. @@ -253,7 +251,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) -> sys::GDExtensionVariantPtr { let ptr = self.ptr_mut_or_null(index); assert!( !ptr.is_null(), @@ -264,14 +262,12 @@ 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) -> sys::GDExtensionVariantPtr { // SAFETY: array_operator_index returns null for invalid indexes. - let variant_ptr = unsafe { + unsafe { let index = to_i64(index); - interface_fn!(array_operator_index)(self.sys(), index) - }; - - Variant::ptr_from_sys_mut(variant_ptr) + interface_fn!(array_operator_index)(self.sys_mut(), index) + } } /// # Safety @@ -456,7 +452,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(), @@ -490,8 +486,8 @@ impl Array { // Panics on out-of-bounds let ptr = self.ptr(index); - // SAFETY: `ptr()` just verified that the index is not out of bounds. - let variant = unsafe { &*ptr }; + // SAFETY: `ptr` is a live pointer to a variant since `ptr.is_null()` just verified that the index is not out of bounds. + let variant = unsafe { Variant::borrow_var_sys(ptr) }; T::from_variant(variant) } @@ -501,8 +497,8 @@ impl Array { if ptr.is_null() { None } else { - // SAFETY: `ptr.is_null()` just verified that the index is not out of bounds. - let variant = unsafe { &*ptr }; + // SAFETY: `ptr` is a live pointer to a variant since `ptr.is_null()` just verified that the index is not out of bounds. + let variant = unsafe { Variant::borrow_var_sys(ptr) }; Some(T::from_variant(variant)) } } @@ -663,7 +659,7 @@ impl Array { // SAFETY: `ptr_mut` just checked that the index is not out of bounds. unsafe { - *ptr_mut = value.to_variant(); + value.to_variant().move_into_var_ptr(ptr_mut); } } @@ -744,24 +740,7 @@ unsafe impl GodotFfi for Array { sys::VariantType::Array } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn from_sys_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::from_sys(ptr); - std::mem::forget(array.clone()); - array - } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. } } impl GodotConvert for Array { @@ -825,9 +804,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 +870,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 +916,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_var_uninit(|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::SysPtr::force_mut(self.sys())); }) } } @@ -954,9 +933,9 @@ impl GodotFfiVariant for Array { } let array = unsafe { - sys::from_sys_init_or_init_default::(|self_ptr| { + sys::new_with_uninit_or_init::(|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::SysPtr::force_mut(variant.var_sys())); }) }; @@ -977,7 +956,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; @@ -987,14 +966,14 @@ impl From<&[T]> for Array { // the nulls with values of type `T`. unsafe { array.as_inner_mut() }.resize(to_i64(len)); - let ptr = array.ptr_mut_or_null(0); - for (i, element) in slice.iter().enumerate() { - // SAFETY: The array contains exactly `len` elements, stored contiguously in memory. - // Also, the pointer is non-null, as we checked for emptiness above. - unsafe { - *ptr.offset(to_isize(i)) = element.to_variant(); - } + // SAFETY: `array` has `len` elements since we just resized it, and they are all valid `Variant`s. Additionally, since + // the array was created in this function, and we do not access the array while this slice exists, the slice has unique + // access to the elements. + let elements = unsafe { Variant::borrow_slice_mut(array.ptr_mut(0), len) }; + for (element, array_slot) in slice.iter().zip(elements.iter_mut()) { + *array_slot = element.to_variant(); } + array } } @@ -1027,14 +1006,13 @@ impl From<&Array> for Vec { fn from(array: &Array) -> Vec { let len = array.len(); let mut vec = Vec::with_capacity(len); - let ptr = array.ptr(0); - for offset in 0..to_isize(len) { - // SAFETY: Arrays are stored contiguously in memory, so we can use pointer arithmetic - // instead of going through `array_operator_index_const` for every index. - let variant = unsafe { &*ptr.offset(offset) }; - let element = T::from_variant(variant); - vec.push(element); - } + + // SAFETY: Unless `experimental-threads` is enabled, then we cannot have concurrent access to this array. + // And since we dont concurrently access the array in this function, we can create a slice to its contents. + let elements = unsafe { Variant::borrow_slice(array.ptr(0), len) }; + + vec.extend(elements.iter().map(T::from_variant)); + vec } } @@ -1057,7 +1035,8 @@ impl<'a, T: GodotType + FromGodot> Iterator for Iter<'a, T> { let element_ptr = self.array.ptr_or_null(idx); // SAFETY: We just checked that the index is not out of bounds, so the pointer won't be null. - let variant = unsafe { &*element_ptr }; + // We immediately convert this to the right element, so barring `experimental-threads` the pointer wont be invalidated in time. + let variant = unsafe { Variant::borrow_var_sys(element_ptr) }; let element = T::from_variant(variant); Some(element) } else { diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index 6be70c97a..eebeb27be 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -25,7 +25,6 @@ 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))] pub struct Callable { opaque: sys::types::OpaqueCallable, } @@ -48,10 +47,10 @@ impl Callable { // upcast not needed let method = method_name.into(); unsafe { - sys::from_sys_init_or_init_default::(|self_ptr| { + sys::new_with_uninit_or_init::(|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 +139,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 +150,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()) }) @@ -270,10 +269,6 @@ impl Callable { pub fn as_inner(&self) -> inner::InnerCallable { inner::InnerCallable::from_outer(self) } - - fn inc_ref(&self) { - std::mem::forget(self.clone()) - } } impl_builtin_traits! { @@ -300,21 +295,17 @@ unsafe impl GodotFfi for Callable { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; + fn new_with_uninit; + fn from_arg_ptr; fn sys; - fn from_sys_init; + fn sys_mut; 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(); - callable - } - - 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 } } @@ -395,8 +386,7 @@ mod custom_callable { r_return: sys::GDExtensionVariantPtr, r_error: *mut sys::GDExtensionCallError, ) { - let arg_refs: &[&Variant] = - Variant::unbounded_refs_from_sys(p_args, p_argument_count as usize); + let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize); let c: &mut C = CallableUserdata::inner_from_raw(callable_userdata); @@ -413,8 +403,7 @@ mod custom_callable { ) where F: FnMut(&[&Variant]) -> Result, { - let arg_refs: &[&Variant] = - Variant::unbounded_refs_from_sys(p_args, p_argument_count as usize); + let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize); let w: &mut FnWrapper = CallableUserdata::inner_from_raw(callable_userdata); @@ -454,7 +443,7 @@ mod custom_callable { let c: &T = CallableUserdata::inner_from_raw(callable_userdata); let s = crate::builtin::GString::from(c.to_string()); - s.move_string_ptr(r_out); + s.move_into_string_ptr(r_out); *r_is_valid = true as sys::GDExtensionBool; } @@ -465,7 +454,7 @@ mod custom_callable { ) { let w: &mut FnWrapper = CallableUserdata::inner_from_raw(callable_userdata); - w.name.clone().move_string_ptr(r_out); + w.name.clone().move_into_string_ptr(r_out); *r_is_valid = true as sys::GDExtensionBool; } } diff --git a/godot-core/src/builtin/dictionary.rs b/godot-core/src/builtin/dictionary.rs index a5b54b821..242fca528 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, GodotFfi}; use super::meta::impl_godot_as_self; @@ -25,7 +25,6 @@ use super::meta::impl_godot_as_self; /// # Thread safety /// /// The same principles apply as for [`VariantArray`]. Consult its documentation for details. -#[repr(C)] pub struct Dictionary { opaque: OpaqueDictionary, } @@ -204,9 +203,9 @@ impl Dictionary { pub fn set(&mut self, key: K, value: V) { let key = key.to_variant(); - // SAFETY: always returns a valid pointer to a value in the dictionary; either pre-existing or newly inserted. + // SAFETY: `self.get_ptr_mut(key)` always returns a valid pointer to a value in the dictionary; either pre-existing or newly inserted. unsafe { - *self.get_ptr_mut(key) = value.to_variant(); + value.to_variant().move_into_var_ptr(self.get_ptr_mut(key)); } } @@ -240,16 +239,12 @@ impl Dictionary { /// Get the pointer corresponding to the given key in the dictionary. /// /// If there exists no value at the given key, a `NIL` variant will be inserted for that key. - fn get_ptr_mut(&mut self, key: K) -> *mut Variant { + fn get_ptr_mut(&mut self, key: K) -> sys::GDExtensionVariantPtr { 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()) - }; - // Never a null pointer, since entry either existed already or was inserted above. - Variant::ptr_from_sys_mut(ptr) + // SAFETY: accessing an unknown key _mutably_ creates that entry in the dictionary, with value `NIL`. + unsafe { interface_fn!(dictionary_operator_index)(self.sys_mut(), key.var_sys()) } } } @@ -270,24 +265,7 @@ unsafe impl GodotFfi for Dictionary { sys::VariantType::Dictionary } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn from_sys_init; - fn sys; - 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::from_sys(ptr); - std::mem::forget(dictionary.clone()); - dictionary - } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. } } impl_godot_as_self!(Dictionary); @@ -331,9 +309,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()); }) } @@ -459,7 +437,7 @@ impl<'a> DictionaryIter<'a> { fn call_init(dictionary: &Dictionary) -> Option { let variant: Variant = Variant::nil(); let iter_fn = |dictionary, next_value: sys::GDExtensionVariantPtr, valid| unsafe { - interface_fn!(variant_iter_init)(dictionary, next_value.as_uninit(), valid) + interface_fn!(variant_iter_init)(dictionary, sys::SysPtr::as_uninit(next_value), valid) }; Self::ffi_iterate(iter_fn, dictionary, variant) @@ -484,7 +462,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 +474,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 ff5a5715b..185989817 100644 --- a/godot-core/src/builtin/meta/mod.rs +++ b/godot-core/src/builtin/meta/mod.rs @@ -265,10 +265,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::SysPtr::force_mut(self.property_name.string_sys()), + class_name: sys::SysPtr::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::SysPtr::force_mut(self.hint_string.string_sys()), usage: u32::try_from(self.usage.ord()).expect("usage.ord()"), } } @@ -328,7 +328,7 @@ impl MethodInfo { let default_argument_vec = self .default_arguments .iter() - .map(|arg| arg.var_sys()) + .map(|arg| sys::SysPtr::force_mut(arg.var_sys())) .collect::>() .into_boxed_slice(); @@ -337,7 +337,7 @@ impl MethodInfo { sys::GDExtensionMethodInfo { id: self.id, - name: self.method_name.string_sys(), + name: sys::SysPtr::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..a0eae0ace 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::SysPtr::force_mut(v.var_sys())) + .collect(); let method_info_sys = sys::GDExtensionClassMethodInfo { - name: self.method_name.string_sys(), + name: sys::SysPtr::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..23eaa62dd 100644 --- a/godot-core/src/builtin/meta/return_marshal.rs +++ b/godot-core/src/builtin/meta/return_marshal.rs @@ -7,6 +7,7 @@ use crate::obj::{Gd, GodotClass}; use crate::sys; +use sys::GodotFfi; use super::{ConvertError, FromGodot, GodotType}; @@ -50,10 +51,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_val| { + let return_ptr = return_val.sys_mut(); + process_return_ptr(return_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 1c00a2a7d..369463d48 100644 --- a/godot-core/src/builtin/meta/signature.rs +++ b/godot-core/src/builtin/meta/signature.rs @@ -222,10 +222,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: Result = Variant::from_var_sys_init_result(|return_ptr| { + let variant: Result = Variant::new_with_var_uninit_result(|return_ptr| { let mut err = sys::default_call_error(); class_fn( method_bind.0, @@ -266,9 +266,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_var_uninit(|return_ptr| { let mut err = sys::default_call_error(); object_call_script_method( object_ptr, @@ -302,8 +302,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| { @@ -467,7 +467,7 @@ unsafe fn varcall_arg( args_ptr: *const sys::GDExtensionConstVariantPtr, call_ctx: &CallContext, ) -> Result { - let variant_ref = &*Variant::ptr_from_sys(*args_ptr.offset(N)); + let variant_ref = Variant::borrow_var_sys(*args_ptr.offset(N)); P::try_from_variant(variant_ref) .map_err(|err| CallError::failed_param_conversion::

(call_ctx, N, err)) diff --git a/godot-core/src/builtin/packed_array.rs b/godot-core/src/builtin/packed_array.rs index f8d78594f..aa44da970 100644 --- a/godot-core/src/builtin/packed_array.rs +++ b/godot-core/src/builtin/packed_array.rs @@ -71,7 +71,6 @@ 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)] pub struct $PackedArray { opaque: $Opaque, } @@ -362,13 +361,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()); @@ -475,31 +474,7 @@ macro_rules! impl_packed_array { sys::VariantType::$PackedArray } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn from_sys_init; - // SAFETY: - // Nothing special needs to be done beyond a `std::mem::swap` when returning a packed array. - fn move_return_ptr; - } - - // SAFETY: - // Packed arrays are properly initialized through a `from_sys` call, but the ref-count should be - // incremented as that is the callee's responsibility. - // - // 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 - } - - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result - } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. } } $crate::builtin::meta::impl_godot_as_self!($PackedArray); diff --git a/godot-core/src/builtin/plane.rs b/godot-core/src/builtin/plane.rs index 3630cdf6d..ef069f7dd 100644 --- a/godot-core/src/builtin/plane.rs +++ b/godot-core/src/builtin/plane.rs @@ -267,7 +267,20 @@ 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 new_with_uninit; + fn from_arg_ptr; + fn sys; + fn sys_mut; + fn move_return_ptr; + } + + fn new_with_init(init: impl FnOnce(&mut Self)) -> Self { + let mut plane = Plane::new(Vector3::UP, 0.0); + init(&mut plane); + plane + } } impl_godot_as_self!(Plane); diff --git a/godot-core/src/builtin/rid.rs b/godot-core/src/builtin/rid.rs index cc3da542f..08837ed10 100644 --- a/godot-core/src/builtin/rid.rs +++ b/godot-core/src/builtin/rid.rs @@ -8,7 +8,7 @@ use std::num::NonZeroU64; use godot_ffi as sys; -use sys::{ffi_methods, static_assert, static_assert_eq_size, GodotFfi}; +use sys::{ffi_methods, static_assert, static_assert_eq_size_align, GodotFfi}; use super::meta::impl_godot_as_self; @@ -45,7 +45,7 @@ pub enum Rid { // Ensure that `Rid`s actually have the layout we expect. Since `Rid` has the same size as `u64`, it cannot // have any padding. As the `Valid` variant must take up all but one of the niches (since it contains a // `NonZerou64`), and the `Invalid` variant must take up the final niche. -static_assert_eq_size!(Rid, u64); +static_assert_eq_size_align!(Rid, u64); // SAFETY: // As Rid and u64 have the same size, and `Rid::Invalid` is initialized, it must be represented by some `u64` @@ -123,7 +123,20 @@ 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 new_with_uninit; + fn from_arg_ptr; + fn sys; + fn sys_mut; + fn move_return_ptr; + } + + fn new_with_init(init: impl FnOnce(&mut Self)) -> Self { + let mut rid = Self::Invalid; + init(&mut rid); + rid + } } impl_godot_as_self!(Rid); diff --git a/godot-core/src/builtin/signal.rs b/godot-core/src/builtin/signal.rs index 906e18301..cd3a9a9df 100644 --- a/godot-core/src/builtin/signal.rs +++ b/godot-core/src/builtin/signal.rs @@ -21,7 +21,6 @@ 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))] pub struct Signal { opaque: sys::types::OpaqueSignal, } @@ -41,10 +40,10 @@ impl Signal { { let signal_name = signal_name.into(); unsafe { - sys::from_sys_init_or_init_default::(|self_ptr| { + sys::new_with_uninit_or_init::(|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 +54,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,20 +162,17 @@ unsafe impl GodotFfi for Signal { } ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; + fn new_from_sys; + fn new_with_uninit; + fn from_arg_ptr; fn sys; - fn from_sys_init; + fn sys_mut; fn move_return_ptr; } - unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, _call_type: sys::PtrcallType) -> Self { - Self::from_sys(ptr) - } - - #[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 } } diff --git a/godot-core/src/builtin/string/gstring.rs b/godot-core/src/builtin/string/gstring.rs index 435a76560..9b0c6eff5 100644 --- a/godot-core/src/builtin/string/gstring.rs +++ b/godot-core/src/builtin/string/gstring.rs @@ -54,7 +54,7 @@ use super::{NodePath, StringName}; // #[repr] is needed on GString itself rather than the opaque field, because PackedStringArray::as_slice() relies on a packed representation. #[repr(transparent)] pub struct GString { - opaque: OpaqueString, + _opaque: OpaqueString, } impl GString { @@ -63,10 +63,6 @@ impl GString { Self::default() } - fn from_opaque(opaque: OpaqueString) -> Self { - Self { opaque } - } - pub fn len(&self) -> usize { self.as_inner().length().try_into().unwrap() } @@ -123,19 +119,23 @@ impl GString { } ffi_methods! { - type sys::GDExtensionStringPtr = *mut Opaque; + type sys::GDExtensionStringPtr = *mut Self; - 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_string_uninit = 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. + /// Moves this string into a string sys pointer. This is the same as using [`GodotFfi::move_return_ptr`]. /// /// # Safety - /// `dst` must be a pointer to a `GString` which is suitable for ffi with Godot. - pub(crate) unsafe fn move_string_ptr(self, dst: sys::GDExtensionStringPtr) { - self.move_return_ptr(dst as *mut _, sys::PtrcallType::Standard); + /// + /// `dst` must be a valid string pointer. + pub(crate) unsafe fn move_into_string_ptr(self, dst: sys::GDExtensionStringPtr) { + let dst: sys::GDExtensionTypePtr = dst.cast(); + + self.move_return_ptr(dst, sys::PtrcallType::Standard); } #[doc(hidden)] @@ -158,24 +158,7 @@ unsafe impl GodotFfi for GString { sys::VariantType::String } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn from_sys_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 - } - - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result - } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } impl_godot_as_self!(GString); @@ -217,7 +200,7 @@ where let bytes = s.as_ref().as_bytes(); unsafe { - Self::from_string_sys_init(|string_ptr| { + Self::new_with_string_uninit(|string_ptr| { let ctor = interface_fn!(string_new_with_utf8_chars_and_len); ctor( string_ptr, @@ -273,9 +256,9 @@ impl FromStr for GString { impl From<&StringName> for GString { fn from(string: &StringName) -> Self { unsafe { - sys::from_sys_init_or_init_default::(|self_ptr| { + sys::new_with_uninit_or_init::(|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()); }) } @@ -294,9 +277,9 @@ impl From for GString { impl From<&NodePath> for GString { fn from(path: &NodePath) -> Self { unsafe { - sys::from_sys_init_or_init_default::(|self_ptr| { + sys::new_with_uninit_or_init::(|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..8197c5d29 100644 --- a/godot-core/src/builtin/string/node_path.rs +++ b/godot-core/src/builtin/string/node_path.rs @@ -8,7 +8,7 @@ use std::fmt; use godot_ffi as sys; -use godot_ffi::{ffi_methods, GDExtensionTypePtr, GodotFfi}; +use godot_ffi::{ffi_methods, GodotFfi}; use crate::builtin::inner; use crate::builtin::meta::impl_godot_as_self; @@ -16,7 +16,6 @@ use crate::builtin::meta::impl_godot_as_self; use super::{GString, StringName}; /// A pre-parsed scene tree path. -#[repr(C)] pub struct NodePath { opaque: sys::types::OpaqueNodePath, } @@ -58,24 +57,7 @@ unsafe impl GodotFfi for NodePath { sys::VariantType::NodePath } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn from_sys_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 - } - - unsafe fn from_sys_init_default(init_fn: impl FnOnce(GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result - } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. } } impl_godot_as_self!(NodePath); @@ -124,9 +106,9 @@ where impl From<&GString> for NodePath { fn from(string: &GString) -> Self { unsafe { - sys::from_sys_init_or_init_default::(|self_ptr| { + sys::new_with_uninit_or_init::(|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..d9ff97b60 100644 --- a/godot-core/src/builtin/string/string_name.rs +++ b/godot-core/src/builtin/string/string_name.rs @@ -26,7 +26,8 @@ use crate::builtin::{GString, NodePath}; /// relying on lexicographical ordering. /// /// Instead, we provide [`transient_ord()`][Self::transient_ord] for ordering relations. -#[repr(C)] +// Currently we rely on `transparent` for `borrow_string_sys`. +#[repr(transparent)] pub struct StringName { opaque: sys::types::OpaqueStringName, } @@ -59,7 +60,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_string_uninit(|ptr| { sys::interface_fn!(string_name_new_with_latin1_chars)( ptr, c_str.as_ptr(), @@ -115,14 +116,22 @@ impl StringName { 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_string_uninit = new_with_uninit; fn string_sys = sys; + fn string_sys_mut = sys_mut; } - #[doc(hidden)] - pub fn string_sys_const(&self) -> sys::GDExtensionConstStringNamePtr { - sys::to_const_ptr(self.string_sys()) + /// Convert a `StringName` sys pointer to a reference with unbounded lifetime. + /// + /// # Safety + /// + /// `ptr` must point to a live `StringName` for the duration of `'a`. + pub(crate) unsafe fn borrow_string_sys<'a>( + ptr: sys::GDExtensionConstStringNamePtr, + ) -> &'a StringName { + sys::static_assert_eq_size_align!(StringName, sys::types::OpaqueStringName); + &*(ptr.cast::()) } #[doc(hidden)] @@ -150,24 +159,7 @@ unsafe impl GodotFfi for StringName { sys::VariantType::StringName } - ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn from_sys_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 - } - - unsafe fn from_sys_init_default(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - let mut result = Self::default(); - init_fn(result.sys_mut()); - result - } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. } } impl_godot_as_self!(StringName); @@ -227,7 +219,7 @@ where // SAFETY: Rust guarantees validity and range of string. unsafe { - Self::from_string_sys_init(|ptr| { + Self::new_with_string_uninit(|ptr| { sys::interface_fn!(string_name_new_with_utf8_chars_and_len)( ptr, utf8.as_ptr() as *const std::ffi::c_char, @@ -241,9 +233,9 @@ where impl From<&GString> for StringName { fn from(string: &GString) -> Self { unsafe { - sys::from_sys_init_or_init_default::(|self_ptr| { + sys::new_with_uninit_or_init::(|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/variant/impls.rs b/godot-core/src/builtin/variant/impls.rs index ac963a247..c6fb2c67b 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_var_uninit(|variant_ptr| { let converter = sys::builtin_fn!($from_fn); - converter(variant_ptr, self.sys()); + converter(variant_ptr, sys::SysPtr::force_mut(self.sys())); }) }; @@ -54,9 +53,9 @@ macro_rules! impl_ffi_variant { // // This was changed in 4.1. let result = unsafe { - sys::from_sys_init_or_init_default(|self_ptr| { + sys::new_with_uninit_or_init(|self_ptr| { let converter = sys::builtin_fn!($to_fn); - converter(self_ptr, variant.var_sys()); + converter(self_ptr, sys::SysPtr::force_mut(variant.var_sys())); }) }; diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index d85012ed8..47be9bc7d 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -24,9 +24,10 @@ pub use sys::{VariantOperator, VariantType}; /// value. /// /// See also [Godot documentation for `Variant`](https://docs.godotengine.org/en/stable/classes/class_variant.html). -#[repr(C, align(8))] +// We rely on the layout of `Variant` being the same as Godot's layout in `borrow_slice` and `borrow_slice_mut`. +#[repr(transparent)] pub struct Variant { - opaque: OpaqueVariant, + _opaque: OpaqueVariant, } impl Variant { @@ -81,7 +82,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::SysPtr::force_mut(self.var_sys())); }) }; @@ -111,13 +112,13 @@ 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| { + Variant::new_with_var_uninit_or_init(|variant_ptr| { interface_fn!(variant_call)( - self.var_sys(), + sys::SysPtr::force_mut(self.var_sys()), method.string_sys(), args_sys.as_ptr(), args_sys.len() as i64, @@ -145,7 +146,7 @@ impl Variant { let mut is_valid = false as u8; let result = unsafe { - Self::from_var_sys_init_or_init_default(|variant_ptr| { + Self::new_with_var_uninit_or_init(|variant_ptr| { interface_fn!(variant_evaluate)( op_sys, self.var_sys(), @@ -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 } @@ -204,81 +205,106 @@ impl Variant { unsafe { interface_fn!(variant_booleanize)(self.var_sys()) != 0 } } - fn from_opaque(opaque: OpaqueVariant) -> Self { - Self { opaque } - } - // Conversions from/to Godot C++ `Variant*` pointers ffi_methods! { - type sys::GDExtensionVariantPtr = *mut Opaque; + type sys::GDExtensionVariantPtr = *mut Self; - 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_var_uninit = new_with_uninit; + fn new_with_var_init = new_with_init; 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 +// All manually implemented unsafe functions on `Variant`. +// Deny `unsafe_op_in_unsafe_fn` so we don't forget to check safety invariants. +#[doc(hidden)] +#[deny(unsafe_op_in_unsafe_fn)] +impl Variant { + /// Moves this variant into a variant sys pointer. This is the same as using [`GodotFfi::move_return_ptr`]. + /// + /// # Safety + /// + /// `dst` must be a valid variant pointer. + pub(crate) unsafe fn move_into_var_ptr(self, dst: sys::GDExtensionVariantPtr) { + let dst: sys::GDExtensionTypePtr = dst.cast(); + // SAFETY: `dst` is a valid Variant pointer. Additionally `Variant` doesn't behave differently for `Standard` and `Virtual` + // pointer calls. + unsafe { + self.move_return_ptr(dst, sys::PtrcallType::Standard); + } } /// # Safety - /// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. + /// + /// For Godot 4.0, see [`GodotFfi::new_with_init`]. + /// For all other versions, see [`GodotFfi::new_with_uninit`]. #[cfg(before_api = "4.1")] - pub unsafe fn from_var_sys_init_or_init_default( + #[doc(hidden)] + pub unsafe fn new_with_var_uninit_or_init( init_fn: impl FnOnce(sys::GDExtensionVariantPtr), ) -> Self { - Self::from_var_sys_init_default(init_fn) + // SAFETY: We're in Godot 4.0, and so the caller must ensure this is safe. + Self::new_with_var_init(|value| init_fn(value.var_sys_mut())) } /// # Safety - /// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. + /// + /// For Godot 4.0, see [`GodotFfi::new_with_init`]. + /// For all other versions, see [`GodotFfi::new_with_uninit`]. #[cfg(since_api = "4.1")] #[doc(hidden)] - pub unsafe fn from_var_sys_init_or_init_default( + pub unsafe fn new_with_var_uninit_or_init( init_fn: impl FnOnce(sys::GDExtensionUninitializedVariantPtr), ) -> Self { - Self::from_var_sys_init(init_fn) + // SAFETY: We're not in Godot 4.0, and so the caller must ensure this is safe. + unsafe { Self::new_with_var_uninit(init_fn) } } + /// Fallible construction of a `Variant` using a fallible initialization function. + /// /// # Safety - /// See [`GodotFfi::from_sys_init`]. + /// + /// If `init_fn` returns `Ok(())`, then it must have initialized the pointer passed to it in accordance with [`GodotFfi::new_with_uninit`]. #[doc(hidden)] - pub unsafe fn from_var_sys_init_result( + pub unsafe fn new_with_var_uninit_result( init_fn: impl FnOnce(sys::GDExtensionUninitializedVariantPtr) -> Result<(), E>, ) -> Result { // Relies on current macro expansion of from_var_sys_init() having a certain implementation. - let mut raw = std::mem::MaybeUninit::::uninit(); + let mut raw = std::mem::MaybeUninit::::uninit(); let var_uninit_ptr = - raw.as_mut_ptr() as ::Ptr; + raw.as_mut_ptr() as ::Uninit; - init_fn(var_uninit_ptr).map(|_err| Self::from_opaque(raw.assume_init())) + // SAFETY: `map` only runs the provided closure for the `Ok(())` variant, in which case `raw` has definitely been initialized. + init_fn(var_uninit_ptr).map(|_success| unsafe { raw.assume_init() }) } - #[doc(hidden)] - pub fn var_sys_const(&self) -> sys::GDExtensionConstVariantPtr { - sys::to_const_ptr(self.var_sys()) - } + /// Convert a `Variant` sys pointer to a reference to a `Variant`. + /// + /// # Safety + /// + /// `ptr` must point to a live `Variant` for the duration of `'a`. + pub(crate) unsafe fn borrow_var_sys<'a>(ptr: sys::GDExtensionConstVariantPtr) -> &'a Variant { + sys::static_assert_eq_size_align!(Variant, sys::types::OpaqueVariant); - /// Converts to variant pointer; can be a null pointer. - pub(crate) fn ptr_from_sys(variant_ptr: sys::GDExtensionConstVariantPtr) -> *const Variant { - variant_ptr as *const Variant + // SAFETY: `ptr` is a pointer to a live `Variant` for the duration of `'a`. + unsafe { &*(ptr.cast::()) } } + /// Convert an array of `Variant` sys pointers to a slice of `Variant` references all with unbounded lifetimes. + /// /// # Safety - /// `variant_ptr_array` must be a valid pointer to an array of `length` variant pointers. - /// The caller is responsible of keeping the backing storage alive while the unbounded references exist. - pub(crate) unsafe fn unbounded_refs_from_sys<'a>( + /// + /// Either `variant_ptr_array` is null, or it must be safe to call [`std::slice::from_raw_parts`] with + /// `variant_ptr_array` cast to `*const &'a Variant` and `length`. + pub(crate) unsafe fn borrow_ref_slice<'a>( variant_ptr_array: *const sys::GDExtensionConstVariantPtr, length: usize, ) -> &'a [&'a Variant] { + sys::static_assert_eq_size_align!(Variant, sys::types::OpaqueVariant); // Godot may pass null to signal "no arguments" (e.g. in custom callables). if variant_ptr_array.is_null() { debug_assert_eq!( @@ -288,25 +314,66 @@ impl Variant { return &[]; } - let variant_ptr_array: &'a [sys::GDExtensionConstVariantPtr] = - std::slice::from_raw_parts(variant_ptr_array, length); - - // SAFETY: raw pointers and references have the same memory layout. + // Note: Raw pointers and references have the same memory layout. // See https://doc.rust-lang.org/reference/type-layout.html#pointers-and-references-layout. - unsafe { std::mem::transmute(variant_ptr_array) } + let variant_ptr_array = variant_ptr_array.cast::<&Variant>(); + + // SAFETY: `variant_ptr_array` isn't null so it is safe to call `from_raw_parts` on the pointer cast to `*const &Variant`. + unsafe { std::slice::from_raw_parts(variant_ptr_array, length) } } - /// Converts to variant mut pointer; can be a null pointer. - pub(crate) fn ptr_from_sys_mut(variant_ptr: sys::GDExtensionVariantPtr) -> *mut Variant { - variant_ptr as *mut Variant + /// Convert an array of `Variant` sys pointers to a slice with unbounded lifetime. + /// + /// # Safety + /// + /// Either `variant_array` is null, or it must be safe to call [`std::slice::from_raw_parts`] with + /// `variant_array` cast to `*const Variant` and `length`. + pub(crate) unsafe fn borrow_slice<'a>( + variant_array: sys::GDExtensionConstVariantPtr, + length: usize, + ) -> &'a [Variant] { + sys::static_assert_eq_size_align!(Variant, sys::types::OpaqueVariant); + + // Godot may pass null to signal "no arguments" (e.g. in custom callables). + if variant_array.is_null() { + debug_assert_eq!( + length, 0, + "Variant::unbounded_refs_from_sys(): pointer is null but length is not 0" + ); + return &[]; + } + + let variant_array = variant_array.cast::(); + + // SAFETY: `variant_array` isn't null so it is safe to call `from_raw_parts` on the pointer cast to `*const Variant`. + unsafe { std::slice::from_raw_parts(variant_array, length) } } - /// Move `self` into a system pointer. This transfers ownership and thus does not call the destructor. + /// Convert an array of `Variant` sys pointers to a mutable slice with unbounded lifetime. /// /// # Safety - /// `dst` must be a pointer to a [`Variant`] which is suitable for ffi with Godot. - pub(crate) unsafe fn move_var_ptr(self, dst: sys::GDExtensionVariantPtr) { - self.move_return_ptr(dst as *mut _, sys::PtrcallType::Standard); + /// + /// Either `variant_array` is null, or it must be safe to call [`std::slice::from_raw_parts_mut`] with + /// `variant_array` cast to `*mut Variant` and `length`. + pub(crate) unsafe fn borrow_slice_mut<'a>( + variant_array: sys::GDExtensionVariantPtr, + length: usize, + ) -> &'a mut [Variant] { + sys::static_assert_eq_size_align!(Variant, sys::types::OpaqueVariant); + + // Godot may pass null to signal "no arguments" (e.g. in custom callables). + if variant_array.is_null() { + debug_assert_eq!( + length, 0, + "Variant::unbounded_refs_from_sys(): pointer is null but length is not 0" + ); + return &mut []; + } + + let variant_array = variant_array.cast::(); + + // 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) } } } @@ -318,13 +385,7 @@ unsafe impl GodotFfi for Variant { sys::VariantType::Nil } - 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 - } + ffi_methods! { type sys::GDExtensionTypePtr = *mut Self; .. } } impl_godot_as_self!(Variant); @@ -332,7 +393,7 @@ impl_godot_as_self!(Variant); impl Default for Variant { fn default() -> Self { unsafe { - Self::from_var_sys_init(|variant_ptr| { + Self::new_with_var_uninit(|variant_ptr| { interface_fn!(variant_new_nil)(variant_ptr); }) } @@ -342,7 +403,7 @@ impl Default for Variant { impl Clone for Variant { fn clone(&self) -> Self { unsafe { - Self::from_var_sys_init(|variant_ptr| { + Self::new_with_var_uninit(|variant_ptr| { interface_fn!(variant_new_copy)(variant_ptr, self.var_sys()); }) } @@ -352,7 +413,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 009bbae8d..6af78402c 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 } @@ -376,7 +358,7 @@ mod script_instance_info { /// /// - We expect the engine to provide a valid variant pointer the return value can be moved into. unsafe fn transfer_variant_to_godot(variant: Variant, return_ptr: sys::GDExtensionVariantPtr) { - variant.move_var_ptr(return_ptr) + variant.move_into_var_ptr(return_ptr) } /// # Safety @@ -408,7 +390,7 @@ mod script_instance_info { /// /// - The engine has to provide a valid string return pointer. unsafe fn transfer_string_to_godot(string: GString, return_ptr: sys::GDExtensionStringPtr) { - string.move_string_ptr(return_ptr); + string.move_into_string_ptr(return_ptr); } /// # Safety @@ -452,8 +434,8 @@ mod script_instance_info { p_name: sys::GDExtensionConstStringNamePtr, p_value: sys::GDExtensionConstVariantPtr, ) -> sys::GDExtensionBool { - let name = transfer_string_name_from_godot(p_name); - let value = &*Variant::ptr_from_sys(p_value); + let name = StringName::new_from_string_sys(p_name); + let value = Variant::borrow_var_sys(p_value); let ctx = || format!("error when calling {}::set", type_name::()); let result = handle_panic(ctx, || { @@ -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,8 +578,8 @@ mod script_instance_info { r_return: sys::GDExtensionVariantPtr, r_error: *mut sys::GDExtensionCallError, ) { - let method = transfer_string_name_from_godot(p_method); - let args = Variant::unbounded_refs_from_sys(p_args, p_argument_count as usize); + let method = StringName::new_from_string_sys(p_method); + let args = Variant::borrow_ref_slice(p_args, p_argument_count as usize); let ctx = || format!("error when calling {}::call", type_name::()); let result = handle_panic(ctx, || { @@ -668,7 +650,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, || { @@ -730,7 +712,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); @@ -902,7 +884,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!( @@ -936,8 +918,8 @@ mod script_instance_info { p_name: sys::GDExtensionConstStringNamePtr, p_value: sys::GDExtensionConstVariantPtr, ) -> sys::GDExtensionBool { - let name = transfer_string_name_from_godot(p_name); - let value = &*Variant::ptr_from_sys(p_value); + let name = StringName::new_from_string_sys(p_name); + let value = Variant::borrow_var_sys(p_value); let ctx = || { format!( diff --git a/godot-core/src/init/mod.rs b/godot-core/src/init/mod.rs index e5ad8fe87..82b6a5f54 100644 --- a/godot-core/src/init/mod.rs +++ b/godot-core/src/init/mod.rs @@ -289,8 +289,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 d72b14493..03cd9e8dc 100644 --- a/godot-core/src/log.rs +++ b/godot-core/src/log.rs @@ -123,11 +123,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_var| { + call_fn(return_var.sys_mut(), args_ptr, args.len() as i32); }); } @@ -147,11 +147,11 @@ pub fn print_rich(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_var| { + call_fn(return_var.sys_mut(), args_ptr, args.len() as i32); }); } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index f9f43f63e..7b4f687c9 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -10,7 +10,7 @@ use std::ops::{Deref, DerefMut}; use godot_ffi as sys; -use sys::{static_assert_eq_size, VariantType}; +use sys::{static_assert_eq_size_align, VariantType}; use crate::builtin::meta::{ CallContext, ConvertError, FromFfiError, FromGodot, GodotConvert, GodotType, ToGodot, @@ -94,7 +94,7 @@ pub struct Gd { } // Size equality check (should additionally be covered by mem::transmute()) -static_assert_eq_size!( +static_assert_eq_size_align!( sys::GDExtensionObjectPtr, sys::types::OpaqueObject, "Godot FFI: pointer type `Object*` should have size advertised in JSON extension file" @@ -407,6 +407,14 @@ impl Gd { self.raw.obj_sys() } + #[doc(hidden)] + pub fn script_sys(&self) -> sys::GDExtensionScriptLanguagePtr + where + T: Inherits, + { + self.raw.script_sys() + } + /// Returns a callable referencing a method from this object named `method_name`. /// /// This is shorter syntax for [`Callable::from_object_method(self, method_name)`][Callable::from_object_method]. @@ -445,7 +453,7 @@ impl Gd { pub unsafe fn from_sys_init_opt(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Option { // TODO(uninit) - should we use GDExtensionUninitializedTypePtr instead? Then update all the builtin codegen... let init_fn = |ptr| { - init_fn(sys::AsUninit::force_init(ptr)); + init_fn(sys::SysPtr::force_init(ptr)); }; // Note: see _call_native_mb_ret_obj() in godot-cpp, which does things quite different (e.g. querying the instance binding). diff --git a/godot-core/src/obj/raw.rs b/godot-core/src/obj/raw.rs index 43ffb0cc5..c044e8224 100644 --- a/godot-core/src/obj/raw.rs +++ b/godot-core/src/obj/raw.rs @@ -349,6 +349,13 @@ impl RawGd { pub(super) fn obj_sys(&self) -> sys::GDExtensionObjectPtr { self.obj as sys::GDExtensionObjectPtr } + + pub(super) fn script_sys(&self) -> sys::GDExtensionScriptLanguagePtr + where + T: super::Inherits, + { + self.obj.cast() + } } impl RawGd @@ -444,17 +451,31 @@ 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 { - self.obj as sys::GDExtensionTypePtr + fn new_with_init(init: impl FnOnce(&mut Self)) -> Self { + let mut obj = Self { + obj: std::ptr::null_mut(), + cached_rtti: None, + }; + + init(&mut obj); + obj + } + + fn sys(&self) -> sys::GDExtensionConstTypePtr { + self.obj.cast() + } + + fn sys_mut(&mut self) -> sys::GDExtensionTypePtr { + self.obj.cast() } // For more context around `ref_get_object` and `ref_set_object`, see: @@ -504,7 +525,7 @@ where // In 4.1, argument pointers were standardized to always be `T**`. #[cfg(before_api = "4.1")] { - self.sys_const() + self.sys() } #[cfg(since_api = "4.1")] @@ -647,7 +668,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_var_uninit(|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 +691,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::SysPtr::force_mut(variant.var_sys())); }) }; diff --git a/godot-core/src/registry/callbacks.rs b/godot-core/src/registry/callbacks.rs index 5ed7a2333..f27cdd6cd 100644 --- a/godot-core/src/registry/callbacks.rs +++ b/godot-core/src/registry/callbacks.rs @@ -103,9 +103,8 @@ pub unsafe extern "C" fn get_virtual( 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()) } @@ -115,9 +114,8 @@ pub unsafe extern "C" fn default_get_virtual( 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()) } @@ -135,7 +133,7 @@ pub unsafe extern "C" fn to_string( let string = T::__godot_to_string(&*instance); // Transfer ownership to Godot - string.move_string_ptr(out_string); + string.move_into_string_ptr(out_string); } #[cfg(before_api = "4.2")] @@ -168,13 +166,11 @@ 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)); - - std::mem::forget(property.clone()); + let property = StringName::new_from_string_sys(name); match T::__godot_get_property(&*instance, property) { Some(value) => { - value.move_var_ptr(ret); + value.move_into_var_ptr(ret); true as sys::GDExtensionBool } None => false as sys::GDExtensionBool, @@ -189,11 +185,8 @@ 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::new_from_string_sys(name); + let value = Variant::new_from_var_sys(value); T::__godot_set_property(&mut *instance, property, value) as sys::GDExtensionBool } diff --git a/godot-ffi/src/extras.rs b/godot-ffi/src/extras.rs index d82b06247..188babd5d 100644 --- a/godot-ffi/src/extras.rs +++ b/godot-ffi/src/extras.rs @@ -23,37 +23,70 @@ impl Distinct for GDExtensionConstTypePtr {} // Extension traits for conversion /// Convert a GDExtension pointer type to its uninitialized version. -pub trait AsUninit { - type Ptr; +pub trait SysPtr { + type Const; + type Uninit; #[allow(clippy::wrong_self_convention)] - fn as_uninit(self) -> Self::Ptr; + fn as_const(self) -> Self::Const; + #[allow(clippy::wrong_self_convention)] + fn as_uninit(self) -> Self::Uninit; - fn force_init(uninit: Self::Ptr) -> Self; + fn force_mut(const_ptr: Self::Const) -> Self; + fn force_init(uninit_ptr: Self::Uninit) -> Self; } -macro_rules! impl_as_uninit { - ($Ptr:ty, $Uninit:ty) => { - impl AsUninit for $Ptr { - type Ptr = $Uninit; +macro_rules! impl_sys_ptr { + ($Ptr:ty, $Const:ty, $Uninit:ty) => { + impl SysPtr for $Ptr { + type Const = $Const; + type Uninit = $Uninit; + + fn as_const(self) -> Self::Const { + self as Self::Const + } + + #[allow(clippy::wrong_self_convention)] + fn as_uninit(self) -> Self::Uninit { + self as Self::Uninit + } - fn as_uninit(self) -> $Uninit { - self as $Uninit + fn force_mut(const_ptr: Self::Const) -> Self { + const_ptr as Self } - fn force_init(uninit: Self::Ptr) -> Self { - uninit as Self + fn force_init(uninit_ptr: Self::Uninit) -> Self { + uninit_ptr as Self } } }; } -#[rustfmt::skip] -impl_as_uninit!(GDExtensionStringNamePtr, GDExtensionUninitializedStringNamePtr); -impl_as_uninit!(GDExtensionVariantPtr, GDExtensionUninitializedVariantPtr); -impl_as_uninit!(GDExtensionStringPtr, GDExtensionUninitializedStringPtr); -impl_as_uninit!(GDExtensionObjectPtr, GDExtensionUninitializedObjectPtr); -impl_as_uninit!(GDExtensionTypePtr, GDExtensionUninitializedTypePtr); +impl_sys_ptr!( + GDExtensionStringNamePtr, + GDExtensionConstStringNamePtr, + GDExtensionUninitializedStringNamePtr +); +impl_sys_ptr!( + GDExtensionVariantPtr, + GDExtensionConstVariantPtr, + GDExtensionUninitializedVariantPtr +); +impl_sys_ptr!( + GDExtensionStringPtr, + GDExtensionConstStringPtr, + GDExtensionUninitializedStringPtr +); +impl_sys_ptr!( + GDExtensionObjectPtr, + GDExtensionConstObjectPtr, + GDExtensionUninitializedObjectPtr +); +impl_sys_ptr!( + GDExtensionTypePtr, + GDExtensionConstTypePtr, + GDExtensionUninitializedTypePtr +); // ---------------------------------------------------------------------------------------------------------------------------------------------- // Helper functions diff --git a/godot-ffi/src/godot_ffi.rs b/godot-ffi/src/godot_ffi.rs index 73d124f32..251468a61 100644 --- a/godot-ffi/src/godot_ffi.rs +++ b/godot-ffi/src/godot_ffi.rs @@ -15,78 +15,67 @@ use std::marker::PhantomData; /// /// # Safety /// -/// [`from_arg_ptr`](GodotFfi::from_arg_ptr) and [`move_return_ptr`](GodotFfi::move_return_ptr) +/// - [`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. +/// +/// - [`new_with_uninit`](GodotFfi::new_with_uninit) must call `init_fn` with a pointer to a *new* +/// [allocated object](https://doc.rust-lang.org/std/ptr/index.html#safety). +/// +/// - [`new_with_init`](GodotFfi::new_with_init) must call `init_fn` with a reference to a *new* value. #[doc(hidden)] // shows up in implementors otherwise pub unsafe trait GodotFfi { + #[doc(hidden)] fn variant_type() -> sys::VariantType; + + #[doc(hidden)] fn default_param_metadata() -> sys::GDExtensionClassMethodArgumentMetadata { sys::GDEXTENSION_METHOD_ARGUMENT_METADATA_NONE } /// Construct from Godot opaque pointer. /// + /// This will increment reference counts if the type is reference counted. If you need to avoid this, then a `borrow_sys` associated + /// function should usually be used. That function that takes a sys-pointer and returns it as a `&Self` reference. This must be manually + /// implemented for each relevant type, as not all types can be borrowed like this. + /// /// # 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; + #[doc(hidden)] + 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; + #[doc(hidden)] + 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 - /// before calling `init_fn`. + /// Like [`new_with_uninit`](GodotFfi::new_with_uninit), but pre-initializes the sys pointer to a default instance (usually + /// [`Default::default()`]) before calling `init_fn`. /// /// 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`]. + /// # Note /// - /// # 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 - 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 - } + /// This does call `init_fn` with a `&mut Self` reference, but in some cases initializing the reference to a more appropriate + /// value may involve violating the value's safety invariant. In those cases it is important to ensure that this violation isn't + /// leaked to user-code. + #[doc(hidden)] + fn new_with_init(init_fn: impl FnOnce(&mut Self)) -> Self; /// 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; + #[doc(hidden)] + 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() - } + #[doc(hidden)] + fn sys_mut(&mut self) -> sys::GDExtensionTypePtr; + #[doc(hidden)] fn as_arg_ptr(&self) -> sys::GDExtensionConstTypePtr { - self.sys_const() + self.sys() } /// Construct from a pointer to an argument in a call. @@ -96,6 +85,7 @@ pub unsafe trait GodotFfi { /// which is different depending on the type. /// /// * `ptr` must encode `Self` according to the given `call_type`'s encoding of argument values. + #[doc(hidden)] unsafe fn from_arg_ptr(ptr: sys::GDExtensionTypePtr, call_type: PtrcallType) -> Self; /// Move self into the pointer in pointer `dst`, dropping what is already in `dst. @@ -106,6 +96,7 @@ pub unsafe trait GodotFfi { /// /// * `dst` must be able to accept a value of type `Self` encoded according to the given /// `call_type`'s encoding of return values. + #[doc(hidden)] unsafe fn move_return_ptr(self, dst: sys::GDExtensionTypePtr, call_type: PtrcallType); } @@ -115,22 +106,22 @@ pub unsafe trait GodotFfi { /// # Safety /// -/// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. +/// See [`GodotFfi::new_with_uninit`] and [`GodotFfi::new_with_init`]. #[cfg(before_api = "4.1")] -pub unsafe fn from_sys_init_or_init_default( +pub unsafe fn new_with_uninit_or_init( init_fn: impl FnOnce(sys::GDExtensionTypePtr), ) -> T { - T::from_sys_init_default(init_fn) + T::new_with_init(|value| init_fn(value.sys_mut())) } /// # Safety /// -/// See [`GodotFfi::from_sys_init`] and [`GodotFfi::from_sys_init_default`]. +/// See [`GodotFfi::new_with_uninit`] and [`GodotFfi::new_with_init`]. #[cfg(since_api = "4.1")] -pub unsafe fn from_sys_init_or_init_default( +pub unsafe fn new_with_uninit_or_init( init_fn: impl FnOnce(sys::GDExtensionUninitializedTypePtr), ) -> T { - T::from_sys_init(init_fn) + T::new_with_uninit(init_fn) } // ---------------------------------------------------------------------------------------------------------------------------------------------- @@ -183,73 +174,105 @@ 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::SysPtr>::Const) -> Self { + // TODO: Directly use copy constructors here? + let opaque = std::ptr::read(ptr.cast()); + let new = Self::from_opaque(opaque); + std::mem::forget(new.clone()); + new } }; - (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 { + unsafe fn $new_with_uninit(init: impl FnOnce(<$Ptr as $crate::SysPtr>::Uninit)) -> Self { let mut raw = std::mem::MaybeUninit::uninit(); - init(raw.as_mut_ptr() as <$Ptr as $crate::AsUninit>::Ptr); + init(raw.as_mut_ptr() as *mut _); Self::from_opaque(raw.assume_init()) } }; + (OpaquePtr $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 = Default::default(); + init(&mut default); + default + } + }; (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { $( #[$attr] )? $vis - fn $sys(&self) -> $Ptr { - &self.opaque as *const _ as $Ptr + fn $sys(&self) -> <$Ptr as $crate::SysPtr>::Const { + std::ptr::from_ref(&self.opaque).cast() + } + }; + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys_mut:ident = sys_mut) => { + $( #[$attr] )? $vis + fn $sys_mut(&mut self) -> $Ptr { + std::ptr::from_mut(&mut self.opaque).cast() } }; (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.cast()) } }; (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 as *mut _, std::ptr::addr_of_mut!(self.opaque)) + std::ptr::swap(dst.cast(), std::ptr::addr_of_mut!(self.opaque)) } }; // 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::SysPtr>::Const) -> Self { + let borrowed = &*ptr.cast::(); + borrowed.clone() } }; - (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::SysPtr>::Uninit)) -> Self { let mut raw = std::mem::MaybeUninit::::uninit(); - init(raw.as_mut_ptr() as <$Ptr as $crate::AsUninit>::Ptr); + init(raw.as_mut_ptr().cast()); 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 = 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::SysPtr>::Const { + std::ptr::from_ref(self).cast() + } + }; + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys_mut:ident = sys_mut) => { + $( #[$attr] )? $vis + fn $sys_mut(&mut self) -> $Ptr { + std::ptr::from_mut(self).cast() } }; (SelfPtr $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 { - *(ptr as *mut Self) + Self::new_from_sys(ptr.cast()) } }; (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $move_return_ptr:ident = move_return_ptr) => { $( #[$attr] )? $vis unsafe fn $move_return_ptr(self, dst: $Ptr, _call_type: $crate::PtrcallType) { - *(dst as *mut Self) = self + *(dst.cast::()) = self } }; } @@ -272,9 +295,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); }; @@ -288,11 +313,6 @@ macro_rules! ffi_methods_rest { /// The **address of** the `Opaque` field is used as the sys pointer. /// Expects a `from_opaque()` constructor and a `opaque` field. /// -/// * `Opaque`
-/// Implements FFI methods for a type with `Opaque` data. -/// The sys pointer is directly reinterpreted from/to the `Opaque` and **not** its address. -/// Expects a `from_opaque()` constructor and a `opaque` field. -/// /// * `*mut Self`
/// Implements FFI methods for a type implemented with standard Rust fields (not opaque). /// The address of `Self` is directly reinterpreted as the sys pointer. @@ -307,12 +327,6 @@ macro_rules! ffi_methods_rest { /// dereferenced argument pointer. /// Returning a value from a pointer call is simply calling [`std::ptr::swap`] on the return pointer /// and the address to the `opaque` field. -/// -/// ## Using `Opaque` -/// -/// Turning pointer call arguments into a value is simply calling `from_opaque` on the argument pointer. -/// Returning a value from a pointer call is simply calling [`std::ptr::swap`] on the return pointer -/// and the `opaque` field transmuted into a pointer. /// /// ## Using `*mut Self` /// @@ -327,13 +341,6 @@ macro_rules! ffi_methods { $crate::ffi_methods_rest!(OpaquePtr $Ptr; $($rest)*); }; - ( // Sys pointer = value of opaque - type $Ptr:ty = Opaque; - $( $rest:tt )* - ) => { - $crate::ffi_methods_rest!(OpaqueValue $Ptr; $($rest)*); - }; - ( // Sys pointer = address of self type $Ptr:ty = *mut Self; $( $rest:tt )* @@ -474,17 +481,28 @@ mod scalars { sys::VariantType::Nil } - unsafe fn from_sys(_ptr: sys::GDExtensionTypePtr) -> Self { + unsafe fn new_from_sys(_ptr: sys::GDExtensionConstTypePtr) -> Self { + // Do nothing + } + + unsafe fn new_with_uninit( + _init: impl FnOnce(sys::GDExtensionUninitializedTypePtr), + ) -> Self { // Do nothing } - unsafe fn from_sys_init(_init: impl FnOnce(sys::GDExtensionUninitializedTypePtr)) -> Self { + fn new_with_init(_init: impl FnOnce(&mut ())) -> Self { // Do nothing } - fn sys(&self) -> sys::GDExtensionTypePtr { + fn sys(&self) -> sys::GDExtensionConstTypePtr { + // ZST dummy pointer + std::ptr::from_ref(self).cast() + } + + fn sys_mut(&mut self) -> sys::GDExtensionTypePtr { // ZST dummy pointer - self as *const _ as sys::GDExtensionTypePtr + std::ptr::from_mut(self).cast() } // SAFETY: diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index d5523f800..f9e0b6725 100644 --- a/godot-ffi/src/lib.rs +++ b/godot-ffi/src/lib.rs @@ -52,8 +52,7 @@ pub use paste; pub use gensym::gensym; pub use crate::godot_ffi::{ - from_sys_init_or_init_default, GodotFfi, GodotNullableFfi, PrimitiveConversionError, - PtrcallType, + new_with_uninit_or_init, GodotFfi, GodotNullableFfi, PrimitiveConversionError, PtrcallType, }; // Method tables diff --git a/godot-ffi/src/toolbox.rs b/godot-ffi/src/toolbox.rs index 5bd39f790..4b82c7c8b 100644 --- a/godot-ffi/src/toolbox.rs +++ b/godot-ffi/src/toolbox.rs @@ -24,14 +24,21 @@ macro_rules! static_assert { }; } -/// Verifies at compile time that two types `T` and `U` have the same size. +/// Verifies at compile time that two types `T` and `U` have the same size and alignment. #[macro_export] -macro_rules! static_assert_eq_size { +macro_rules! static_assert_eq_size_align { ($T:ty, $U:ty) => { - godot_ffi::static_assert!(std::mem::size_of::<$T>() == std::mem::size_of::<$U>()); + godot_ffi::static_assert!( + std::mem::size_of::<$T>() == std::mem::size_of::<$U>() + && std::mem::align_of::<$T>() == std::mem::align_of::<$U>() + ); }; ($T:ty, $U:ty, $msg:literal) => { - godot_ffi::static_assert!(std::mem::size_of::<$T>() == std::mem::size_of::<$U>(), $msg); + godot_ffi::static_assert!( + std::mem::size_of::<$T>() == std::mem::size_of::<$U>() + && std::mem::align_of::<$T>() == std::mem::align_of::<$U>(), + $msg + ); }; } diff --git a/godot-macros/src/class/godot_api.rs b/godot-macros/src/class/godot_api.rs index a594bf587..710193582 100644 --- a/godot-macros/src/class/godot_api.rs +++ b/godot-macros/src/class/godot_api.rs @@ -427,7 +427,7 @@ fn add_virtual_script_call( let code = quote! { let object_ptr = #object_ptr; let method_sname = ::godot::builtin::StringName::from(#method_name_str); - let method_sname_ptr = method_sname.string_sys_const(); + let method_sname_ptr = method_sname.string_sys(); let has_virtual_method = unsafe { ::godot::private::has_virtual_script_method(object_ptr, method_sname_ptr) }; if has_virtual_method { 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 bf7b7dac1..55534182a 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,8 +70,8 @@ fn object_user_roundtrip_write() { let raw = obj.to_ffi(); let raw2 = unsafe { - RawGd::::from_sys_init(|ptr| { - raw.move_return_ptr(sys::AsUninit::force_init(ptr), sys::PtrcallType::Standard) + RawGd::::new_with_uninit(|ptr| { + raw.move_return_ptr(sys::SysPtr::force_init(ptr), sys::PtrcallType::Standard) }) }; let obj2 = Gd::from_ffi(raw2); @@ -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); }