diff --git a/.github/workflows/full-ci.yml b/.github/workflows/full-ci.yml index 2952a691c..04285b415 100644 --- a/.github/workflows/full-ci.yml +++ b/.github/workflows/full-ci.yml @@ -138,8 +138,7 @@ jobs: godot-itest: name: godot-itest (${{ matrix.name }}) runs-on: ${{ matrix.os }} - # TODO: continue-on-error: false, as soon as memory errors are fixed - continue-on-error: ${{ contains(matrix.name, 'memcheck') }} + continue-on-error: false timeout-minutes: 24 strategy: fail-fast: false # cancel all jobs as soon as one fails? @@ -165,6 +164,10 @@ jobs: rust-toolchain: stable godot-binary: godot.linuxbsd.editor.dev.x86_64 + # Special Godot binaries compiled with AddressSanitizer/LeakSanitizer to detect UB/leaks. + # Additionally, the Godot source is patched to make dlclose() a no-op, as unloading dynamic libraries loses stacktrace and + # cause false positives like println!. See https://github.com/google/sanitizers/issues/89. + # The gcc version can possibly be removed later, as it is slower and needs a larger artifact than the clang one. - name: linux-memcheck-gcc os: ubuntu-20.04 rust-toolchain: stable diff --git a/godot-codegen/src/class_generator.rs b/godot-codegen/src/class_generator.rs index f8fcde25d..b3de7e61b 100644 --- a/godot-codegen/src/class_generator.rs +++ b/godot-codegen/src/class_generator.rs @@ -686,7 +686,7 @@ impl VariantFfi { fn type_ptr() -> Self { Self { sys_method: ident("sys_const"), - from_sys_init_method: ident("from_sys_init"), + from_sys_init_method: ident("from_sys_init_default"), } } } @@ -876,7 +876,7 @@ fn make_return( } (None, Some(return_ty)) => { quote! { - <#return_ty as sys::GodotFfi>::from_sys_init(|return_ptr| { + <#return_ty as sys::GodotFfi>::from_sys_init_default(|return_ptr| { #ptrcall_invocation }) } diff --git a/godot-core/src/builtin/array.rs b/godot-core/src/builtin/array.rs index 5ed37737f..a5e7ef04a 100644 --- a/godot-core/src/builtin/array.rs +++ b/godot-core/src/builtin/array.rs @@ -70,12 +70,11 @@ impl_builtin_froms!(Array; impl TypedArray { fn from_opaque(opaque: sys::types::OpaqueArray) -> Self { - let array = Self { + // Note: type is not yet checked at this point, because array has not yet been initialized! + Self { opaque, _phantom: PhantomData, - }; - array.check_type(); - array + } } /// Returns the number of elements in the array. Equivalent of `size()` in Godot. @@ -320,8 +319,9 @@ impl TypedArray { /// # Panics /// /// If the inner type doesn't match `T` or no type is set at all. - fn check_type(&self) { + fn with_checked_type(self) -> Self { assert_eq!(self.type_info(), TypeInfo::new::()); + self } /// Sets the type of the inner array. Can only be called once, directly after creation. @@ -567,17 +567,9 @@ impl TypedArray { // } impl GodotFfi for TypedArray { - ffi_methods! { - type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn write_sys; - } - - unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - // Can't use uninitialized pointer -- Array CoW implementation in C++ expects that on - // assignment, the target CoW pointer is either initialized or nullptr + 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 @@ -598,28 +590,25 @@ impl fmt::Debug for TypedArray { /// [`Array::duplicate_deep()`]. impl Share for TypedArray { fn share(&self) -> Self { - unsafe { + let array = unsafe { Self::from_sys_init(|self_ptr| { let ctor = ::godot_ffi::builtin_fn!(array_construct_copy); let args = [self.sys_const()]; ctor(self_ptr, args.as_ptr()); }) - } + }; + array.with_checked_type() } } impl Default for TypedArray { #[inline] fn default() -> Self { - // Note: can't use from_sys_init(), as that calls the default constructor - // (because most assignments expect initialized target type) - let mut uninit = std::mem::MaybeUninit::>::uninit(); let mut array = unsafe { - let self_ptr = (*uninit.as_mut_ptr()).sys_mut(); - sys::builtin_call! { - array_construct_default(self_ptr, std::ptr::null_mut()) - }; - uninit.assume_init() + Self::from_sys_init(|self_ptr| { + let ctor = sys::builtin_fn!(array_construct_default); + ctor(self_ptr, std::ptr::null_mut()) + }) }; array.init_inner_type(); array @@ -661,14 +650,14 @@ impl FromVariant for TypedArray { if variant.get_type() != Self::variant_type() { return Err(VariantConversionError); } - let result = unsafe { - Self::from_sys_init(|self_ptr| { + let array = unsafe { + Self::from_sys_init_default(|self_ptr| { let array_from_variant = sys::builtin_fn!(array_from_variant); array_from_variant(self_ptr, variant.var_sys()); }) }; - Ok(result) + Ok(array.with_checked_type()) } } @@ -773,7 +762,7 @@ impl PartialEq for TypedArray { let mut result = false; sys::builtin_call! { array_operator_equal(self.sys(), other.sys(), result.sys_mut()) - }; + } result } } diff --git a/godot-core/src/builtin/dictionary.rs b/godot-core/src/builtin/dictionary.rs index bc5f36822..82e08c9f3 100644 --- a/godot-core/src/builtin/dictionary.rs +++ b/godot-core/src/builtin/dictionary.rs @@ -240,17 +240,9 @@ impl Dictionary { // Traits impl GodotFfi for Dictionary { - ffi_methods! { - type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn write_sys; - } - - unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - // Can't use uninitialized pointer -- Dictionary CoW implementation in C++ expects that on - // assignment, the target CoW pointer is either initialized or nullptr + 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 diff --git a/godot-core/src/builtin/macros.rs b/godot-core/src/builtin/macros.rs index 8f4360424..6173f1e23 100644 --- a/godot-core/src/builtin/macros.rs +++ b/godot-core/src/builtin/macros.rs @@ -33,7 +33,7 @@ macro_rules! impl_builtin_traits_inner { #[inline] fn clone(&self) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::from_sys_init_default(|self_ptr| { let ctor = ::godot_ffi::builtin_fn!($gd_method); let args = [self.sys_const()]; ctor(self_ptr, args.as_ptr()); @@ -116,7 +116,7 @@ macro_rules! impl_builtin_traits_inner { return Err($crate::builtin::variant::VariantConversionError) } let result = unsafe { - Self::from_sys_init(|self_ptr| { + Self::from_sys_init_default(|self_ptr| { let converter = sys::builtin_fn!($gd_method); converter(self_ptr, variant.var_sys()); }) diff --git a/godot-core/src/builtin/node_path.rs b/godot-core/src/builtin/node_path.rs index 5a38f3a3d..7e389c4b6 100644 --- a/godot-core/src/builtin/node_path.rs +++ b/godot-core/src/builtin/node_path.rs @@ -6,7 +6,7 @@ use crate::builtin::GodotString; use godot_ffi as sys; -use godot_ffi::{ffi_methods, GodotFfi}; +use godot_ffi::{ffi_methods, GDExtensionTypePtr, GodotFfi}; use std::fmt::{Display, Formatter, Result as FmtResult}; pub struct NodePath { @@ -21,12 +21,18 @@ impl NodePath { impl GodotFfi for NodePath { ffi_methods! { type sys::GDExtensionTypePtr = *mut Opaque; .. } + + unsafe fn from_sys_init_default(init_fn: impl FnOnce(GDExtensionTypePtr)) -> Self { + let mut result = Self::default(); + init_fn(result.sys_mut()); + result + } } impl From<&GodotString> for NodePath { fn from(path: &GodotString) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::from_sys_init_default(|self_ptr| { let ctor = sys::builtin_fn!(node_path_from_string); let args = [path.sys_const()]; ctor(self_ptr, args.as_ptr()); @@ -38,7 +44,7 @@ impl From<&GodotString> for NodePath { impl From<&NodePath> for GodotString { fn from(path: &NodePath) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::from_sys_init_default(|self_ptr| { let ctor = sys::builtin_fn!(string_from_node_path); let args = [path.sys_const()]; ctor(self_ptr, args.as_ptr()); diff --git a/godot-core/src/builtin/others.rs b/godot-core/src/builtin/others.rs index 99f40a32f..82c956526 100644 --- a/godot-core/src/builtin/others.rs +++ b/godot-core/src/builtin/others.rs @@ -50,7 +50,7 @@ impl Callable { // upcast not needed let method = method.into(); unsafe { - Self::from_sys_init(|self_ptr| { + Self::from_sys_init_default(|self_ptr| { let ctor = sys::builtin_fn!(callable_from_object_method); let args = [object.sys_const(), method.sys_const()]; ctor(self_ptr, args.as_ptr()); @@ -66,6 +66,7 @@ impl Callable { impl_builtin_traits! { for Callable { + // Default => callable_construct_default; FromVariant => callable_from_variant; } } diff --git a/godot-core/src/builtin/packed_array.rs b/godot-core/src/builtin/packed_array.rs index f240e6e6f..52d39750a 100644 --- a/godot-core/src/builtin/packed_array.rs +++ b/godot-core/src/builtin/packed_array.rs @@ -386,17 +386,9 @@ macro_rules! impl_packed_array { } impl GodotFfi for $PackedArray { - ffi_methods! { - type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn write_sys; - } - - unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - // Can't use uninitialized pointer -- Vector CoW implementation in C++ expects that - // on assignment, the target CoW pointer is either initialized or nullptr + 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 diff --git a/godot-core/src/builtin/string.rs b/godot-core/src/builtin/string.rs index e74322904..3611728ff 100644 --- a/godot-core/src/builtin/string.rs +++ b/godot-core/src/builtin/string.rs @@ -37,43 +37,18 @@ impl GodotString { } impl GodotFfi for GodotString { - ffi_methods! { - type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn write_sys; - } - - unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - // Can't use uninitialized pointer -- String CoW implementation in C++ expects that on assignment, - // the target CoW pointer is either initialized or nullptr + 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 } } -impl Default for GodotString { - fn default() -> Self { - // Note: can't use from_sys_init(), as that calls the default constructor - // (because most assignments expect initialized target type) - - let mut uninit = std::mem::MaybeUninit::::uninit(); - - unsafe { - let self_ptr = (*uninit.as_mut_ptr()).sys_mut(); - sys::builtin_call! { - string_construct_default(self_ptr, std::ptr::null_mut()) - }; - - uninit.assume_init() - } - } -} - impl_builtin_traits! { for GodotString { + Default => string_construct_default; Clone => string_construct_copy; Drop => string_destroy; Eq => string_operator_equal; diff --git a/godot-core/src/builtin/string_name.rs b/godot-core/src/builtin/string_name.rs index ac5df05d0..394c64e33 100644 --- a/godot-core/src/builtin/string_name.rs +++ b/godot-core/src/builtin/string_name.rs @@ -33,17 +33,9 @@ impl StringName { } impl GodotFfi for StringName { - ffi_methods! { - type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn write_sys; - } - - unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - // Can't use uninitialized pointer -- StringName implementation in C++ expects that on assignment, - // the target type is a valid string (possibly empty) + 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 @@ -106,7 +98,7 @@ impl Hash for StringName { impl From<&GodotString> for StringName { fn from(s: &GodotString) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::from_sys_init_default(|self_ptr| { let ctor = sys::builtin_fn!(string_name_from_string); let args = [s.sys_const()]; ctor(self_ptr, args.as_ptr()); @@ -128,7 +120,7 @@ where impl From<&StringName> for GodotString { fn from(s: &StringName) -> Self { unsafe { - Self::from_sys_init(|self_ptr| { + Self::from_sys_init_default(|self_ptr| { let ctor = sys::builtin_fn!(string_from_string_name); let args = [s.sys_const()]; 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 b0c8f9728..53006ce83 100644 --- a/godot-core/src/builtin/variant/impls.rs +++ b/godot-core/src/builtin/variant/impls.rs @@ -55,19 +55,21 @@ macro_rules! impl_variant_traits { impl FromVariant for $T { fn try_from_variant(variant: &Variant) -> Result { + // Type check -- at the moment, a strict match is required. + if variant.get_type() != Self::variant_type() { + return Err(VariantConversionError) + } + // In contrast to T -> Variant, the conversion Variant -> T assumes // that the destination is initialized (at least for some T). For example: // void String::operator=(const String &p_str) { _cowdata._ref(p_str._cowdata); } // does a copy-on-write and explodes if this->_cowdata is not initialized. // We can thus NOT use Self::from_sys_init(). - if variant.get_type() != Self::variant_type() { - return Err(VariantConversionError) - } - let mut value = <$T>::default(); let result = unsafe { - let converter = sys::builtin_fn!($to_fn); - converter(value.sys_mut(), variant.var_sys()); - value + Self::from_sys_init_default(|self_ptr| { + let converter = sys::builtin_fn!($to_fn); + converter(self_ptr, variant.var_sys()); + }) }; Ok(result) diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index 487bec3ca..ea76024e4 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -26,11 +26,7 @@ pub struct Variant { impl Variant { /// Create an empty variant (`null` value in GDScript). pub fn nil() -> Self { - unsafe { - Self::from_var_sys_init(|variant_ptr| { - interface_fn!(variant_new_nil)(variant_ptr); - }) - } + Self::default() } /// Create a variant holding a non-nil value. @@ -154,17 +150,6 @@ impl Variant { sys::to_const_ptr(self.var_sys()) } - /*#[doc(hidden)] - pub unsafe fn from_var_sys_init(init_fn: impl FnOnce(sys::GDExtensionVariantPtr)) -> Self { - // Can't use uninitialized pointer -- Variant implementation in C++ expects that on assignment, - // the target pointer is an initialized Variant - - #[allow(unused_mut)] - let mut result = Self::default(); - init_fn(result.var_sys()); - result - }*/ - pub(crate) fn ptr_from_sys(variant_ptr: sys::GDExtensionVariantPtr) -> *const Variant { assert!(!variant_ptr.is_null(), "ptr_from_sys: null variant pointer"); variant_ptr as *const Variant @@ -180,17 +165,9 @@ impl Variant { } impl GodotFfi for Variant { - ffi_methods! { - type sys::GDExtensionTypePtr = *mut Opaque; - fn from_sys; - fn sys; - fn write_sys; - } - - unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self { - // Can't use uninitialized pointer -- Variant implementation in C++ expects that on assignment, - // the target pointer is an initialized 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 @@ -217,7 +194,11 @@ impl Drop for Variant { impl Default for Variant { fn default() -> Self { - Self::nil() + unsafe { + Self::from_var_sys_init(|variant_ptr| { + interface_fn!(variant_new_nil)(variant_ptr); + }) + } } } diff --git a/godot-core/src/log.rs b/godot-core/src/log.rs index e159eee08..23ff22fec 100644 --- a/godot-core/src/log.rs +++ b/godot-core/src/log.rs @@ -88,7 +88,7 @@ pub fn print(varargs: &[Variant]) { args.extend(varargs.iter().map(Variant::sys_const)); let args_ptr = args.as_ptr(); - let _variant = Variant::from_sys_init(|return_ptr| { + let _variant = Variant::from_sys_init_default(|return_ptr| { call_fn(return_ptr, args_ptr, args.len() as i32); }); } diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index c74d7ab72..38554dbe1 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -52,6 +52,9 @@ pub struct Gd { // The former is the standard FFI type, while the latter is used in object-specific GDExtension engines. // pub(crate) because accessed in obj::dom pub(crate) opaque: OpaqueObject, + + // Last known instance ID -- this may no longer be valid! + cached_instance_id: std::cell::Cell>, _marker: PhantomData<*const T>, } @@ -206,18 +209,39 @@ impl Gd { } fn from_opaque(opaque: OpaqueObject) -> Self { - Self { + let obj = Self { opaque, + cached_instance_id: std::cell::Cell::new(None), _marker: PhantomData, - } + }; + + // Initialize instance ID cache + let id = unsafe { interface_fn!(object_get_instance_id)(obj.obj_sys()) }; + let instance_id = InstanceId::try_from_u64(id) + .expect("instance ID must be non-zero at time of initialization"); + obj.cached_instance_id.set(Some(instance_id)); + + obj } /// Returns the instance ID of this object, or `None` if the object is dead. - /// pub fn instance_id_or_none(&self) -> Option { - // Note: bit 'id & (1 << 63)' determines if the instance is ref-counted - let id = unsafe { interface_fn!(object_get_instance_id)(self.obj_sys()) }; - InstanceId::try_from_u64(id) + let known_id = match self.cached_instance_id.get() { + // Already dead + None => return None, + + // Possibly alive + Some(id) => id, + }; + + // Refreshes the internal cached ID on every call, as we cannot be sure that the object has not been + // destroyed since last time. The only reliable way to find out is to call is_instance_id_valid(). + if engine::utilities::is_instance_id_valid(known_id.to_i64()) { + Some(known_id) + } else { + self.cached_instance_id.set(None); + None + } } /// ⚠️ Returns the instance ID of this object (panics when dead). @@ -242,12 +266,8 @@ impl Gd { /// and will panic in a defined manner. Encountering such panics is almost always a bug you should fix, and not a /// runtime condition to check against. pub fn is_instance_valid(&self) -> bool { - // TODO Is this really necessary, or is Godot's instance_id() guaranteed to return 0 for destroyed objects? - if let Some(id) = self.instance_id_or_none() { - engine::utilities::is_instance_id_valid(id.to_i64()) - } else { - false - } + // This call refreshes the instance ID, and recognizes dead objects. + self.instance_id_or_none().is_some() } /// **Upcast:** convert into a smart pointer to a base class. Always succeeds. @@ -256,7 +276,6 @@ impl Gd { /// use this idiom: /// ```no_run /// # use godot::prelude::*; - /// /// #[derive(GodotClass)] /// #[class(init, base=Node2D)] /// struct MyClass {} diff --git a/godot-core/src/obj/instance_id.rs b/godot-core/src/obj/instance_id.rs index e7daa2c49..5d94acb86 100644 --- a/godot-core/src/obj/instance_id.rs +++ b/godot-core/src/obj/instance_id.rs @@ -16,6 +16,7 @@ use std::num::NonZeroU64; /// This is its own type for type safety and to deal with the inconsistent representation in Godot as both `u64` (C++) and `i64` (GDScript). /// You can usually treat this as an opaque value and pass it to and from GDScript; there are conversion methods however. #[derive(Copy, Clone, Eq, PartialEq, Hash, Ord, PartialOrd)] +#[repr(transparent)] pub struct InstanceId { // Note: in the public API, signed i64 is the canonical representation. // diff --git a/godot-ffi/src/godot_ffi.rs b/godot-ffi/src/godot_ffi.rs index 6d3449860..9187fc295 100644 --- a/godot-ffi/src/godot_ffi.rs +++ b/godot-ffi/src/godot_ffi.rs @@ -23,6 +23,28 @@ pub trait GodotFfi { /// `init_fn` must be a function that correctly handles a (possibly-uninitialized) _type ptr_. unsafe fn from_sys_init(init_fn: impl FnOnce(sys::GDExtensionTypePtr)) -> Self; + /// Like [`Self::from_sys_init`], but pre-initializes the sys pointer to a `Default::default()` instance + /// 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`]. + /// + /// # 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 + { + Self::from_sys_init(init_fn) + + // TODO consider using this, if all the implementors support it + // let mut result = Self::default(); + // init_fn(result.sys_mut()); + // result + } + /// Return Godot opaque pointer, for an immutable operation. /// /// Note that this is a `*mut` pointer despite taking `&self` by shared-ref. @@ -80,120 +102,120 @@ pub trait GodotFuncMarshal: Sized { #[macro_export] #[doc(hidden)] macro_rules! ffi_methods_one { - // type $Ptr = *mut Opaque - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { - $( #[$attr] )? $vis - unsafe fn $from_sys(ptr: $Ptr) -> Self { + // type $Ptr = *mut Opaque + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { + $( #[$attr] )? $vis + unsafe fn $from_sys(ptr: $Ptr) -> Self { let opaque = std::ptr::read(ptr as *mut _); Self::from_opaque(opaque) } - }; - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { - $( #[$attr] )? $vis - unsafe fn $from_sys_init(init: impl FnOnce($Ptr)) -> Self { + }; + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { + $( #[$attr] )? $vis + unsafe fn $from_sys_init(init: impl FnOnce($Ptr)) -> Self { let mut raw = std::mem::MaybeUninit::uninit(); init(raw.as_mut_ptr() as $Ptr); Self::from_opaque(raw.assume_init()) } - }; - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { - $( #[$attr] )? $vis - fn $sys(&self) -> $Ptr { + }; + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { + $( #[$attr] )? $vis + fn $sys(&self) -> $Ptr { &self.opaque as *const _ as $Ptr } - }; - (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $write_sys:ident = write_sys) => { - $( #[$attr] )? $vis - unsafe fn $write_sys(&self, dst: $Ptr) { + }; + (OpaquePtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $write_sys:ident = write_sys) => { + $( #[$attr] )? $vis + unsafe fn $write_sys(&self, dst: $Ptr) { // Note: this is the same impl as for impl_ffi_as_opaque_value, which is... interesting std::ptr::write(dst as *mut _, self.opaque) } - }; + }; - // type $Ptr = Opaque - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { - $( #[$attr] )? $vis - unsafe fn $from_sys(ptr: $Ptr) -> Self { + // 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) } - }; - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { - $( #[$attr] )? $vis - unsafe fn $from_sys_init(init: impl FnOnce($Ptr)) -> Self { + }; + (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { + $( #[$attr] )? $vis + unsafe fn $from_sys_init(init: impl FnOnce($Ptr)) -> Self { let mut raw = std::mem::MaybeUninit::uninit(); init(std::mem::transmute(raw.as_mut_ptr())); Self::from_opaque(raw.assume_init()) } - }; - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { - $( #[$attr] )? $vis - fn $sys(&self) -> $Ptr { + }; + (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { + $( #[$attr] )? $vis + fn $sys(&self) -> $Ptr { unsafe { std::mem::transmute(self.opaque) } } - }; - (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $write_sys:ident = write_sys) => { - $( #[$attr] )? $vis - unsafe fn $write_sys(&self, dst: $Ptr) { + }; + (OpaqueValue $Ptr:ty; $( #[$attr:meta] )? $vis:vis $write_sys:ident = write_sys) => { + $( #[$attr] )? $vis + unsafe fn $write_sys(&self, dst: $Ptr) { // Note: this is the same impl as for impl_ffi_as_opaque_value, which is... interesting std::ptr::write(dst as *mut _, self.opaque); } - }; + }; - // type $Ptr = *mut Self - (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { - $( #[$attr] )? $vis - unsafe fn $from_sys(ptr: $Ptr) -> Self { + // type $Ptr = *mut Self + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys:ident = from_sys) => { + $( #[$attr] )? $vis + unsafe fn $from_sys(ptr: $Ptr) -> Self { *(ptr as *mut Self) } - }; - (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { - $( #[$attr] )? $vis - unsafe fn $from_sys_init(init: impl FnOnce($Ptr)) -> Self { + }; + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $from_sys_init:ident = from_sys_init) => { + $( #[$attr] )? $vis + unsafe fn $from_sys_init(init: impl FnOnce($Ptr)) -> Self { let mut raw = std::mem::MaybeUninit::::uninit(); init(raw.as_mut_ptr() as $Ptr); raw.assume_init() } - }; - (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { - $( #[$attr] )? $vis - fn $sys(&self) -> $Ptr { + }; + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $sys:ident = sys) => { + $( #[$attr] )? $vis + fn $sys(&self) -> $Ptr { self as *const Self as $Ptr } - }; - (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $write_sys:ident = write_sys) => { - $( #[$attr] )? $vis - unsafe fn $write_sys(&self, dst: $Ptr) { + }; + (SelfPtr $Ptr:ty; $( #[$attr:meta] )? $vis:vis $write_sys:ident = write_sys) => { + $( #[$attr] )? $vis + unsafe fn $write_sys(&self, dst: $Ptr) { *(dst as *mut Self) = *self; } - }; + }; } #[macro_export] #[doc(hidden)] macro_rules! ffi_methods_rest { - ( // impl T: each method has a custom name and is annotated with 'pub' - $Impl:ident $Ptr:ty; $( fn $user_fn:ident = $sys_fn:ident; )* - ) => { - $( $crate::ffi_methods_one!($Impl $Ptr; #[doc(hidden)] pub $user_fn = $sys_fn); )* - }; - - ( // impl GodotFfi for T: methods have given names, no 'pub' needed - $Impl:ident $Ptr:ty; $( fn $sys_fn:ident; )* - ) => { - $( $crate::ffi_methods_one!($Impl $Ptr; $sys_fn = $sys_fn); )* - }; - - ( // impl GodotFfi for T (default all 4) - $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; sys = sys); - $crate::ffi_methods_one!($Impl $Ptr; write_sys = write_sys); - }; + ( // impl T: each method has a custom name and is annotated with 'pub' + $Impl:ident $Ptr:ty; $( fn $user_fn:ident = $sys_fn:ident; )* + ) => { + $( $crate::ffi_methods_one!($Impl $Ptr; #[doc(hidden)] pub $user_fn = $sys_fn); )* + }; + + ( // impl GodotFfi for T: methods have given names, no 'pub' needed + $Impl:ident $Ptr:ty; $( fn $sys_fn:ident; )* + ) => { + $( $crate::ffi_methods_one!($Impl $Ptr; $sys_fn = $sys_fn); )* + }; + + ( // impl GodotFfi for T (default all 4) + $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; sys = sys); + $crate::ffi_methods_one!($Impl $Ptr; write_sys = write_sys); + }; } /// Provides "sys" style methods for FFI and ptrcall integration with Godot. @@ -216,26 +238,26 @@ macro_rules! ffi_methods_rest { /// This cannot be checked easily, because Self cannot be used in size_of(). There would of course be workarounds. #[macro_export] macro_rules! ffi_methods { - ( // Sys pointer = address of opaque - type $Ptr:ty = *mut Opaque; - $( $rest:tt )* - ) => { - $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 )* - ) => { - $crate::ffi_methods_rest!(SelfPtr $Ptr; $($rest)*); - }; + ( // Sys pointer = address of opaque + type $Ptr:ty = *mut Opaque; + $( $rest:tt )* + ) => { + $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 )* + ) => { + $crate::ffi_methods_rest!(SelfPtr $Ptr; $($rest)*); + }; } // ---------------------------------------------------------------------------------------------------------------------------------------------- diff --git a/itest/rust/src/object_test.rs b/itest/rust/src/object_test.rs index 5bd07076c..95364f0fa 100644 --- a/itest/rust/src/object_test.rs +++ b/itest/rust/src/object_test.rs @@ -163,7 +163,7 @@ fn object_instance_id_when_freed() { node.share().free(); // destroys object without moving out of reference assert!(!node.is_instance_valid()); - expect_panic("instance_id() on dead obj", || { + expect_panic("instance_id() on dead object", move || { node.instance_id(); }); } diff --git a/itest/rust/src/string_test.rs b/itest/rust/src/string_test.rs index ed3889992..8c2a63d66 100644 --- a/itest/rust/src/string_test.rs +++ b/itest/rust/src/string_test.rs @@ -11,6 +11,7 @@ use godot::builtin::{GodotString, StringName}; pub fn run() -> bool { let mut ok = true; + ok &= string_default(); ok &= string_conversion(); ok &= string_equality(); ok &= string_ordering(); @@ -23,6 +24,14 @@ pub fn run() -> bool { ok } +#[itest] +fn string_default() { + let string = GodotString::new(); + let back = String::from(&string); + + assert_eq!(back.as_str(), ""); +} + #[itest] fn string_conversion() { let string = String::from("some string");