diff --git a/godot-core/src/builtin/aabb.rs b/godot-core/src/builtin/aabb.rs index b0a66e640..dc986e86a 100644 --- a/godot-core/src/builtin/aabb.rs +++ b/godot-core/src/builtin/aabb.rs @@ -366,7 +366,7 @@ impl Aabb { // Credits: https://tavianator.com/2011/ray_box.html fn compute_ray_tnear_tfar(self, ray_from: Vector3, ray_dir: Vector3) -> (real, real) { self.assert_nonnegative(); - debug_assert!( + sys::balanced_assert!( ray_dir != Vector3::ZERO, "ray direction must not be zero; use contains_point() for point checks" ); @@ -763,9 +763,8 @@ mod test { ); } - #[test] - #[should_panic] - #[cfg(debug_assertions)] + #[test] // cfg_attr: no panic in disengaged level (although current CI doesn't run unit-tests). + #[cfg_attr(safeguards_balanced, should_panic)] fn test_intersect_ray_zero_dir_inside() { let aabb = Aabb { position: Vector3::new(-1.5, 2.0, -2.5), @@ -776,8 +775,7 @@ mod test { } #[test] - #[should_panic] - #[cfg(debug_assertions)] + #[cfg_attr(safeguards_balanced, should_panic)] fn test_intersect_ray_zero_dir_outside() { let aabb = Aabb { position: Vector3::new(-1.5, 2.0, -2.5), diff --git a/godot-core/src/builtin/callable.rs b/godot-core/src/builtin/callable.rs index 783c6ef93..28e68cd38 100644 --- a/godot-core/src/builtin/callable.rs +++ b/godot-core/src/builtin/callable.rs @@ -5,6 +5,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use std::thread::ThreadId; use std::{fmt, ptr}; use godot_ffi as sys; @@ -162,16 +163,7 @@ impl Callable { F: 'static + FnMut(&[&Variant]) -> R, S: meta::AsArg, { - #[cfg(debug_assertions)] - meta::arg_into_owned!(name); - - Self::from_fn_wrapper::(FnWrapper { - rust_function, - #[cfg(debug_assertions)] - name, - thread_id: Some(std::thread::current().id()), - linked_obj_id: None, - }) + Self::from_fn_wrapper(name, rust_function, Some(std::thread::current().id()), None) } /// Creates a new callable linked to the given object from **single-threaded** Rust function or closure. @@ -189,16 +181,12 @@ impl Callable { F: 'static + FnMut(&[&Variant]) -> R, S: meta::AsArg, { - #[cfg(debug_assertions)] - meta::arg_into_owned!(name); - - Self::from_fn_wrapper::(FnWrapper { - rust_function, - #[cfg(debug_assertions)] + Self::from_fn_wrapper( name, - thread_id: Some(std::thread::current().id()), - linked_obj_id: Some(linked_object.instance_id()), - }) + rust_function, + Some(std::thread::current().id()), + Some(linked_object.instance_id()), + ) } /// This constructor is being phased out in favor of [`from_fn()`][Self::from_fn], but kept through v0.4 for smoother migration. @@ -258,16 +246,8 @@ impl Callable { F: FnMut(&[&Variant]) -> Variant, Fc: FnOnce(&Callable) -> R, { - #[cfg(debug_assertions)] - meta::arg_into_owned!(name); - - let callable = Self::from_fn_wrapper::(FnWrapper { - rust_function, - #[cfg(debug_assertions)] - name, - thread_id: Some(std::thread::current().id()), - linked_obj_id: None, - }); + let callable = + Self::from_fn_wrapper(name, rust_function, Some(std::thread::current().id()), None); callable_usage(&callable) } @@ -298,16 +278,7 @@ impl Callable { F: 'static + Send + Sync + FnMut(&[&Variant]) -> R, S: meta::AsArg, { - #[cfg(debug_assertions)] - meta::arg_into_owned!(name); - - Self::from_fn_wrapper::(FnWrapper { - rust_function, - #[cfg(debug_assertions)] - name, - thread_id: None, - linked_obj_id: None, - }) + Self::from_fn_wrapper(name, rust_function, None, None) } /// Create a highly configurable callable from Rust. @@ -334,21 +305,34 @@ impl Callable { Self::from_custom_info(info) } - fn from_fn_wrapper(inner: FnWrapper) -> Self + fn from_fn_wrapper( + _name: S, + rust_function: F, + thread_id: Option, + linked_object_id: Option, + ) -> Self where F: FnMut(&[&Variant]) -> R, R: ToGodot, + S: meta::AsArg, { - let object_id = inner.linked_object_id(); + let wrapper = FnWrapper { + rust_function, + #[cfg(safeguards_balanced)] + name: { _name.into_arg().cow_into_owned() }, + thread_id, + linked_object_id, + }; - let userdata = CallableUserdata { inner }; + let object_id = wrapper.linked_object_id(); + let userdata = CallableUserdata { inner: wrapper }; let info = CallableCustomInfo { object_id, callable_userdata: Box::into_raw(Box::new(userdata)) as *mut std::ffi::c_void, call_func: Some(rust_callable_call_fn::), free_func: Some(rust_callable_destroy::>), - #[cfg(debug_assertions)] + #[cfg(safeguards_balanced)] to_string_func: Some(rust_callable_to_string_named::), is_valid_func: Some(rust_callable_is_valid), ..Self::default_callable_custom_info() @@ -617,21 +601,34 @@ mod custom_callable { pub(crate) struct FnWrapper { pub(super) rust_function: F, - #[cfg(debug_assertions)] + #[cfg(safeguards_balanced)] pub(super) name: GString, /// `None` if the callable is multi-threaded ([`Callable::from_sync_fn`]). pub(super) thread_id: Option, /// `None` if callable is not linked with any object. - pub(super) linked_obj_id: Option, + pub(super) linked_object_id: Option, } impl FnWrapper { pub(crate) fn linked_object_id(&self) -> GDObjectInstanceID { - self.linked_obj_id.map(InstanceId::to_u64).unwrap_or(0) + self.linked_object_id.map(InstanceId::to_u64).unwrap_or(0) } } + /// Returns the name for safeguard-enabled builds, or `""` otherwise. + macro_rules! name_or_optimized { + ($($code:tt)*) => { + { + #[cfg(safeguards_balanced)] + { $($code)* } + + #[cfg(not(safeguards_balanced))] + { "" } + } + }; + } + /// Represents a custom callable object defined in Rust. /// /// This trait has a single method, `invoke`, which is called upon invocation. @@ -668,14 +665,11 @@ mod custom_callable { ) { let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize); - #[cfg(debug_assertions)] - let name = &{ + let name = name_or_optimized! { let c: &C = CallableUserdata::inner_from_raw(callable_userdata); c.to_string() }; - #[cfg(not(debug_assertions))] - let name = ""; - let ctx = meta::CallContext::custom_callable(name); + let ctx = meta::CallContext::custom_callable(&name); crate::private::handle_fallible_varcall(&ctx, &mut *r_error, move || { // Get the RustCallable again inside closure so it doesn't have to be UnwindSafe. @@ -698,34 +692,25 @@ mod custom_callable { { let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize); - #[cfg(debug_assertions)] - let name = &{ + let name = name_or_optimized! { let w: &FnWrapper = CallableUserdata::inner_from_raw(callable_userdata); w.name.to_string() }; - #[cfg(not(debug_assertions))] - let name = ""; - let ctx = meta::CallContext::custom_callable(name); + let ctx = meta::CallContext::custom_callable(&name); crate::private::handle_fallible_varcall(&ctx, &mut *r_error, move || { // Get the FnWrapper again inside closure so the FnMut doesn't have to be UnwindSafe. let w: &mut FnWrapper = CallableUserdata::inner_from_raw(callable_userdata); - if w.thread_id - .is_some_and(|tid| tid != std::thread::current().id()) - { - #[cfg(debug_assertions)] - let name = &w.name; - #[cfg(not(debug_assertions))] - let name = ""; - // NOTE: this panic is currently not propagated to the caller, but results in an error message and Nil return. - // See comments in itest callable_call() for details. - panic!( - "Callable '{}' created with from_fn() must be called from the same thread it was created in.\n\ - If you need to call it from any thread, use from_sync_fn() instead (requires `experimental-threads` feature).", - name - ); - } + let name = name_or_optimized!(w.name.to_string()); + + // NOTE: this panic is currently not propagated to the caller, but results in an error message and Nil return. + // See comments in itest callable_call() for details. + sys::balanced_assert!( + w.thread_id.is_none() || w.thread_id == Some(std::thread::current().id()), + "Callable '{name}' created with from_fn() must be called from the same thread it was created in.\n\ + If you need to call it from any thread, use from_sync_fn() instead (requires `experimental-threads` feature)." + ); let result = (w.rust_function)(arg_refs).to_variant(); meta::varcall_return_checked(Ok(result), r_return, r_error); @@ -769,7 +754,7 @@ mod custom_callable { *r_is_valid = sys::conv::SYS_TRUE; } - #[cfg(debug_assertions)] + #[cfg(safeguards_balanced)] pub unsafe extern "C" fn rust_callable_to_string_named( callable_userdata: *mut std::ffi::c_void, r_is_valid: *mut sys::GDExtensionBool, diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index 561b93534..3a716bdc4 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -319,7 +319,7 @@ impl Array { /// Clears the array, removing all elements. pub fn clear(&mut self) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); // SAFETY: No new values are written to the array, we only remove values from the array. unsafe { self.as_inner_mut() }.clear(); @@ -331,7 +331,7 @@ impl Array { /// /// If `index` is out of bounds. pub fn set(&mut self, index: usize, value: impl AsArg) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let ptr_mut = self.ptr_mut(index); @@ -348,7 +348,7 @@ impl Array { #[doc(alias = "append")] #[doc(alias = "push_back")] pub fn push(&mut self, value: impl AsArg) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); meta::arg_into_ref!(value: T); @@ -362,7 +362,7 @@ impl Array { /// On large arrays, this method is much slower than [`push()`][Self::push], as it will move all the array's elements. /// The larger the array, the slower `push_front()` will be. pub fn push_front(&mut self, value: impl AsArg) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); meta::arg_into_ref!(value: T); @@ -376,7 +376,7 @@ impl Array { /// _Godot equivalent: `pop_back`_ #[doc(alias = "pop_back")] pub fn pop(&mut self) -> Option { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); (!self.is_empty()).then(|| { // SAFETY: We do not write any values to the array, we just remove one. @@ -390,7 +390,7 @@ impl Array { /// Note: On large arrays, this method is much slower than `pop()` as it will move all the /// array's elements. The larger the array, the slower `pop_front()` will be. pub fn pop_front(&mut self) -> Option { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); (!self.is_empty()).then(|| { // SAFETY: We do not write any values to the array, we just remove one. @@ -407,7 +407,7 @@ impl Array { /// # Panics /// If `index > len()`. pub fn insert(&mut self, index: usize, value: impl AsArg) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let len = self.len(); assert!( @@ -431,8 +431,7 @@ impl Array { /// If `index` is out of bounds. #[doc(alias = "pop_at")] pub fn remove(&mut self, index: usize) -> T { - self.debug_ensure_mutable(); - + self.balanced_ensure_mutable(); self.check_bounds(index); // SAFETY: We do not write any values to the array, we just remove one. @@ -447,7 +446,7 @@ impl Array { /// On large arrays, this method is much slower than [`pop()`][Self::pop], as it will move all the array's /// elements after the removed element. pub fn erase(&mut self, value: impl AsArg) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); meta::arg_into_ref!(value: T); @@ -458,7 +457,7 @@ impl Array { /// Assigns the given value to all elements in the array. This can be used together with /// `resize` to create an array with a given size and initialized elements. pub fn fill(&mut self, value: impl AsArg) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); meta::arg_into_ref!(value: T); @@ -473,7 +472,7 @@ impl Array { /// /// If you know that the new size is smaller, then consider using [`shrink`](Array::shrink) instead. pub fn resize(&mut self, new_size: usize, value: impl AsArg) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let original_size = self.len(); @@ -505,7 +504,7 @@ impl Array { /// If you want to increase the size of the array, use [`resize`](Array::resize) instead. #[doc(alias = "resize")] pub fn shrink(&mut self, new_size: usize) -> bool { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); if new_size >= self.len() { return false; @@ -519,7 +518,7 @@ impl Array { /// Appends another array at the end of this array. Equivalent of `append_array` in GDScript. pub fn extend_array(&mut self, other: &Array) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); // SAFETY: `append_array` will only read values from `other`, and all types can be converted to `Variant`. let other: &VariantArray = unsafe { other.assume_type_ref::() }; @@ -743,7 +742,7 @@ impl Array { /// Reverses the order of the elements in the array. pub fn reverse(&mut self) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); // SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type. unsafe { self.as_inner_mut() }.reverse(); @@ -760,7 +759,7 @@ impl Array { /// _Godot equivalent: `Array.sort()`_ #[doc(alias = "sort")] pub fn sort_unstable(&mut self) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); // SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type. unsafe { self.as_inner_mut() }.sort(); @@ -780,7 +779,7 @@ impl Array { where F: FnMut(&T, &T) -> cmp::Ordering, { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let godot_comparator = |args: &[&Variant]| { let lhs = T::from_variant(args[0]); @@ -809,7 +808,7 @@ impl Array { /// _Godot equivalent: `Array.sort_custom()`_ #[doc(alias = "sort_custom")] pub fn sort_unstable_custom(&mut self, func: &Callable) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); // SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type. unsafe { self.as_inner_mut() }.sort_custom(func); @@ -819,7 +818,7 @@ impl Array { /// global random number generator common to methods such as `randi`. Call `randomize` to /// ensure that a new seed will be used each time if you want non-reproducible shuffling. pub fn shuffle(&mut self) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); // SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type. unsafe { self.as_inner_mut() }.shuffle(); @@ -859,10 +858,10 @@ impl Array { /// Best-effort mutability check. /// - /// # Panics - /// In Debug mode, if the array is marked as read-only. - fn debug_ensure_mutable(&self) { - debug_assert!( + /// # Panics (safeguards-balanced) + /// If the array is marked as read-only. + fn balanced_ensure_mutable(&self) { + sys::balanced_assert!( !self.is_read_only(), "mutating operation on read-only array" ); @@ -873,6 +872,7 @@ impl Array { /// # Panics /// If `index` is out of bounds. fn check_bounds(&self, index: usize) { + // Safety-relevant; explicitly *don't* use safeguards-dependent validation. let len = self.len(); assert!( index < len, @@ -1049,8 +1049,8 @@ impl Array { /// # Safety /// Must only be called once, directly after creation. unsafe fn init_inner_type(&mut self) { - debug_assert!(self.is_empty()); - debug_assert!( + sys::strict_assert!(self.is_empty()); + sys::strict_assert!( self.cached_element_type.get().is_none(), "init_inner_type() called twice" ); diff --git a/godot-core/src/builtin/collections/dictionary.rs b/godot-core/src/builtin/collections/dictionary.rs index 166c48bde..799c3b853 100644 --- a/godot-core/src/builtin/collections/dictionary.rs +++ b/godot-core/src/builtin/collections/dictionary.rs @@ -163,7 +163,7 @@ impl Dictionary { /// _Godot equivalent: `get_or_add`_ #[doc(alias = "get_or_add")] pub fn get_or_insert(&mut self, key: K, default: V) -> Variant { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let key_variant = key.to_variant(); let default_variant = default.to_variant(); @@ -237,7 +237,7 @@ impl Dictionary { /// Removes all key-value pairs from the dictionary. pub fn clear(&mut self) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); self.as_inner().clear() } @@ -248,7 +248,7 @@ impl Dictionary { /// /// _Godot equivalent: `dict[key] = value`_ pub fn set(&mut self, key: K, value: V) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let key = key.to_variant(); @@ -263,7 +263,7 @@ impl Dictionary { /// If you don't need the previous value, use [`set()`][Self::set] instead. #[must_use] pub fn insert(&mut self, key: K, value: V) -> Option { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let key = key.to_variant(); let old_value = self.get(key.clone()); @@ -277,7 +277,7 @@ impl Dictionary { /// _Godot equivalent: `erase`_ #[doc(alias = "erase")] pub fn remove(&mut self, key: K) -> Option { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); let key = key.to_variant(); let old_value = self.get(key.clone()); @@ -318,7 +318,7 @@ impl Dictionary { /// _Godot equivalent: `merge`_ #[doc(alias = "merge")] pub fn extend_dictionary(&mut self, other: &Self, overwrite: bool) { - self.debug_ensure_mutable(); + self.balanced_ensure_mutable(); self.as_inner().merge(other, overwrite) } @@ -408,10 +408,10 @@ impl Dictionary { /// Best-effort mutability check. /// - /// # Panics - /// In Debug mode, if the array is marked as read-only. - fn debug_ensure_mutable(&self) { - debug_assert!( + /// # Panics (safeguards-balanced) + /// If the dictionary is marked as read-only. + fn balanced_ensure_mutable(&self) { + sys::balanced_assert!( !self.is_read_only(), "mutating operation on read-only dictionary" ); diff --git a/godot-core/src/builtin/collections/extend_buffer.rs b/godot-core/src/builtin/collections/extend_buffer.rs index 30e518155..3bf2d89c0 100644 --- a/godot-core/src/builtin/collections/extend_buffer.rs +++ b/godot-core/src/builtin/collections/extend_buffer.rs @@ -8,6 +8,8 @@ use std::mem::MaybeUninit; use std::ptr; +use crate::sys; + /// A fixed-size buffer that does not do any allocations, and can hold up to `N` elements of type `T`. /// /// This is used to implement [`PackedArray::extend()`][crate::builtin::PackedArray::extend] in an efficient way, because it forms a middle @@ -55,7 +57,7 @@ impl ExtendBufferTrait for ExtendBuffer { if N == 0 { return &mut []; } - debug_assert!(self.len <= N); + sys::strict_assert!(self.len <= N); let len = self.len; self.len = 0; @@ -77,7 +79,7 @@ impl Drop for ExtendBuffer { if N == 0 { return; } - debug_assert!(self.len <= N); + sys::strict_assert!(self.len <= N); // SAFETY: `slice_from_raw_parts_mut` by itself is not unsafe, but to make the resulting slice safe to use: // - `self.buf[0]` is a valid pointer, exactly `self.len` elements are initialized. diff --git a/godot-core/src/builtin/collections/packed_array.rs b/godot-core/src/builtin/collections/packed_array.rs index 4e0a215f4..600e7f55c 100644 --- a/godot-core/src/builtin/collections/packed_array.rs +++ b/godot-core/src/builtin/collections/packed_array.rs @@ -409,9 +409,11 @@ impl PackedArray { /// * Source data must not be dropped later. unsafe fn move_from_slice(&mut self, src: *const T, dst: usize, len: usize) { let ptr = self.ptr_mut(dst); - debug_assert_eq!(len, self.len() - dst, "length precondition violated"); + sys::strict_assert_eq!(len, self.len() - dst, "length precondition violated"); + // Drops all elements in place. Drop impl must not panic. ptr::drop_in_place(ptr::slice_from_raw_parts_mut(ptr, len)); + // Copy is okay since all elements are dropped. ptr.copy_from_nonoverlapping(src, len); } @@ -958,7 +960,7 @@ impl PackedByteArray { let size: i64 = self .as_inner() .decode_var_size(byte_offset as i64, allow_objects); - debug_assert_ne!(size, -1); // must not happen if we just decoded variant. + sys::strict_assert_ne!(size, -1); // must not happen if we just decoded variant. Ok((variant, size as usize)) } diff --git a/godot-core/src/builtin/quaternion.rs b/godot-core/src/builtin/quaternion.rs index 4f1877ef1..63f374ac6 100644 --- a/godot-core/src/builtin/quaternion.rs +++ b/godot-core/src/builtin/quaternion.rs @@ -77,11 +77,11 @@ impl Quaternion { /// /// *Godot equivalent: `Quaternion(arc_from: Vector3, arc_to: Vector3)`* pub fn from_rotation_arc(arc_from: Vector3, arc_to: Vector3) -> Self { - debug_assert!( + sys::balanced_assert!( arc_from.is_normalized(), "input 1 (`arc_from`) in `Quaternion::from_rotation_arc` must be a unit vector" ); - debug_assert!( + sys::balanced_assert!( arc_to.is_normalized(), "input 2 (`arc_to`) in `Quaternion::from_rotation_arc` must be a unit vector" ); diff --git a/godot-core/src/builtin/string/node_path.rs b/godot-core/src/builtin/string/node_path.rs index c518da209..e48d7ca99 100644 --- a/godot-core/src/builtin/string/node_path.rs +++ b/godot-core/src/builtin/string/node_path.rs @@ -54,13 +54,14 @@ impl NodePath { /// godot_print!("{}", path.get_name(2)); // "Sprite" /// ``` /// - /// # Panics - /// In Debug mode, if `index` is out of bounds. In Release, a Godot error is generated and the result is unspecified (but safe). + /// # Panics (safeguards-balanced) + /// If `index` is out of bounds. In safeguards-disengaged level, a Godot error is generated and the result is unspecified (but safe). pub fn get_name(&self, index: usize) -> StringName { let inner = self.as_inner(); let index = index as i64; - debug_assert!( + // Not safety-critical, Godot will do another check. But better error message. + sys::balanced_assert!( index < inner.get_name_count(), "NodePath '{self}': name at index {index} is out of bounds" ); @@ -80,13 +81,13 @@ impl NodePath { /// godot_print!("{}", path.get_subname(1)); // "resource_name" /// ``` /// - /// # Panics - /// In Debug mode, if `index` is out of bounds. In Release, a Godot error is generated and the result is unspecified (but safe). + /// # Panics (safeguards-balanced) + /// If `index` is out of bounds. In safeguards-disengaged level, a Godot error is generated and the result is unspecified (but safe). pub fn get_subname(&self, index: usize) -> StringName { let inner = self.as_inner(); let index = index as i64; - debug_assert!( + sys::balanced_assert!( index < inner.get_subname_count(), "NodePath '{self}': subname at index {index} is out of bounds" ); diff --git a/godot-core/src/builtin/string/string_macros.rs b/godot-core/src/builtin/string/string_macros.rs index f5fb5f403..0d47350da 100644 --- a/godot-core/src/builtin/string/string_macros.rs +++ b/godot-core/src/builtin/string/string_macros.rs @@ -19,12 +19,12 @@ macro_rules! impl_shared_string_api { impl $Builtin { /// Returns the Unicode code point ("character") at position `index`. /// - /// # Panics - /// In debug builds, if `index` is out of bounds. In Release builds, `0` is returned instead. + /// # Panics (safeguards-balanced) + /// If `index` is out of bounds. In disengaged level, `0` is returned instead. // Unicode conversion panic is not documented because we rely on Godot strings having valid Unicode. // TODO implement Index/IndexMut (for GString; StringName may have varying reprs). pub fn unicode_at(&self, index: usize) -> char { - debug_assert!(index < self.len(), "unicode_at: index {} out of bounds (len {})", index, self.len()); + sys::balanced_assert!(index < self.len(), "unicode_at: index {} out of bounds (len {})", index, self.len()); let char_i64 = self.as_inner().unicode_at(index as i64); diff --git a/godot-core/src/builtin/variant/mod.rs b/godot-core/src/builtin/variant/mod.rs index bec3fa10b..15e2a2e16 100644 --- a/godot-core/src/builtin/variant/mod.rs +++ b/godot-core/src/builtin/variant/mod.rs @@ -343,7 +343,7 @@ impl Variant { /// /// Does not check again that the variant has type `OBJECT`. pub(crate) fn is_object_alive(&self) -> bool { - debug_assert_eq!(self.get_type(), VariantType::OBJECT); + sys::strict_assert_eq!(self.get_type(), VariantType::OBJECT); crate::global::is_instance_valid(self) @@ -425,12 +425,10 @@ impl Variant { 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!( - length, 0, - "Variant::unbounded_refs_from_sys(): pointer is null but length is not 0" - ); + Self::strict_ensure_zero_length(length); return &[]; } @@ -456,10 +454,7 @@ impl Variant { // 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" - ); + Self::strict_ensure_zero_length(length); return &[]; } @@ -483,10 +478,7 @@ impl Variant { // 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" - ); + Self::strict_ensure_zero_length(length); return &mut []; } @@ -496,6 +488,14 @@ impl Variant { unsafe { std::slice::from_raw_parts_mut(variant_array, length) } } + fn strict_ensure_zero_length(_length: usize) { + sys::strict_assert_eq!( + _length, + 0, + "Variant::borrow_slice*(): pointer is null but length is not 0" + ); + } + /// Consumes self and turns it into a sys-ptr, should be used together with [`from_owned_var_sys`](Self::from_owned_var_sys). /// /// This will leak memory unless `from_owned_var_sys` is called on the returned pointer. diff --git a/godot-core/src/classes/class_runtime.rs b/godot-core/src/classes/class_runtime.rs index ef7cd0df5..75c2d779b 100644 --- a/godot-core/src/classes/class_runtime.rs +++ b/godot-core/src/classes/class_runtime.rs @@ -7,17 +7,27 @@ //! Runtime checks and inspection of Godot classes. -use crate::builtin::{GString, StringName, Variant, VariantType}; +use crate::builtin::{GString, StringName, Variant}; +use crate::obj::{bounds, Bounds, Gd, GodotClass, InstanceId, RawGd}; +use crate::sys; + #[cfg(safeguards_strict)] -use crate::classes::{ClassDb, Object}; +mod strict { + pub use crate::builtin::VariantType; + pub use crate::classes::{ClassDb, Object}; + pub use crate::meta::ClassId; + pub use crate::obj::Singleton; +} + #[cfg(safeguards_balanced)] -use crate::meta::CallContext; -#[cfg(safeguards_strict)] -use crate::meta::ClassId; +mod balanced { + pub use crate::meta::CallContext; +} + +#[cfg(safeguards_balanced)] +use balanced::*; #[cfg(safeguards_strict)] -use crate::obj::Singleton; -use crate::obj::{bounds, Bounds, Gd, GodotClass, InstanceId, RawGd}; -use crate::sys; +use strict::*; pub(crate) fn debug_string( obj: &Gd, @@ -38,7 +48,7 @@ pub(crate) fn debug_string_variant( f: &mut std::fmt::Formatter<'_>, ty: &str, ) -> std::fmt::Result { - debug_assert_eq!(obj.get_type(), VariantType::OBJECT); + sys::strict_assert_eq!(obj.get_type(), VariantType::OBJECT); let id = obj .object_id_unchecked() @@ -73,7 +83,7 @@ pub(crate) fn debug_string_variant( f: &mut std::fmt::Formatter<'_>, ty: &str, ) -> std::fmt::Result { - debug_assert_eq!(obj.get_type(), VariantType::OBJECT); + sys::strict_assert_eq!(obj.get_type(), VariantType::OBJECT); match obj.try_to::>() { Ok(obj) => { diff --git a/godot-core/src/lib.rs b/godot-core/src/lib.rs index 9eabe0a11..a57539790 100644 --- a/godot-core/src/lib.rs +++ b/godot-core/src/lib.rs @@ -33,7 +33,7 @@ pub mod tools; mod storage; pub use godot_ffi as sys; -pub use crate::private::{get_gdext_panic_context, set_gdext_hook}; +pub use crate::private::{fetch_last_panic_context, set_gdext_hook}; // ---------------------------------------------------------------------------------------------------------------------------------------------- // Validations (see also godot/lib.rs) diff --git a/godot-core/src/meta/class_id.rs b/godot-core/src/meta/class_id.rs index 01108837a..4ab0e9815 100644 --- a/godot-core/src/meta/class_id.rs +++ b/godot-core/src/meta/class_id.rs @@ -253,7 +253,7 @@ impl ClassIdCache { /// /// Returns the `ClassId` for the given name. /// - /// # Panics (Debug) + /// # Panics (safeguards-balanced) /// If `expect_first` is true and the string is already present in the cache. fn insert_class_id( &mut self, @@ -263,8 +263,7 @@ impl ClassIdCache { ) -> ClassId { if expect_first { // Debug verification that we're indeed the first to register this string. - #[cfg(debug_assertions)] - assert!( + sys::balanced_assert!( !self.string_to_index.contains_key(source.as_ref()), "insert_class_name() called for already-existing string: {}", source diff --git a/godot-core/src/meta/element_type.rs b/godot-core/src/meta/element_type.rs index 33b8b9054..9063890d7 100644 --- a/godot-core/src/meta/element_type.rs +++ b/godot-core/src/meta/element_type.rs @@ -131,8 +131,8 @@ impl ElementType { } }); - // Debug validation for cached Untyped values. - #[cfg(debug_assertions)] + // Consistency validation for cached Untyped values. + #[cfg(safeguards_strict)] if matches!(cached, ElementType::Untyped) { let sys_variant_type = get_builtin_type(); let variant_type = diff --git a/godot-core/src/meta/error/call_error.rs b/godot-core/src/meta/error/call_error.rs index c8d8c36cc..93eb90d93 100644 --- a/godot-core/src/meta/error/call_error.rs +++ b/godot-core/src/meta/error/call_error.rs @@ -254,7 +254,7 @@ impl CallError { // This specializes on reflection-style calls, e.g. call(), rpc() etc. // In these cases, varargs are the _actual_ arguments, with required args being metadata such as method name. - debug_assert_ne!(err.error, sys::GDEXTENSION_CALL_OK); // already checked outside + sys::strict_assert_ne!(err.error, sys::GDEXTENSION_CALL_OK); // already checked outside let sys::GDExtensionCallError { error, diff --git a/godot-core/src/meta/error/call_error_type.rs b/godot-core/src/meta/error/call_error_type.rs index 5240d88b0..16b5942d5 100644 --- a/godot-core/src/meta/error/call_error_type.rs +++ b/godot-core/src/meta/error/call_error_type.rs @@ -64,7 +64,7 @@ impl CallErrorType { sys::GDEXTENSION_CALL_ERROR_INSTANCE_IS_NULL => Err(Self::InstanceIsNull), sys::GDEXTENSION_CALL_ERROR_METHOD_NOT_CONST => Err(Self::MethodNotConst), _ => { - debug_assert!(false, "Unknown GDExtensionCallErrorType value: {value}"); + sys::strict_assert!(false, "Unknown GDExtensionCallErrorType value: {value}"); Err(Self::InvalidMethod) // in Release builds, return known error. } } diff --git a/godot-core/src/meta/signed_range.rs b/godot-core/src/meta/signed_range.rs index d5aa318ea..ddb4ff1b1 100644 --- a/godot-core/src/meta/signed_range.rs +++ b/godot-core/src/meta/signed_range.rs @@ -94,12 +94,12 @@ where /// Returns a tuple of `(from, to)` from a Rust range. /// -/// # Panics -/// In debug mode, when from > to. +/// # Panics (safeguards-strict) +/// When `from` > `to`. pub(crate) fn to_godot_range_fromto(range: impl SignedRange) -> (i64, i64) { match range.signed() { (from, Some(to)) => { - debug_assert!(from <= to, "range: start ({from}) > end ({to})"); + crate::sys::strict_assert!(from <= to, "range: start ({from}) > end ({to})"); (from, to) } (from, None) => (from, 0), @@ -113,7 +113,7 @@ pub(crate) fn to_godot_range_fromto(range: impl SignedRange) -> (i64, i64) { pub(crate) fn to_godot_range_fromlen(range: impl SignedRange, unbounded: i64) -> (i64, i64) { match range.signed() { (from, Some(to)) => { - debug_assert!(from <= to, "range: start ({from}) > end ({to})"); + crate::sys::strict_assert!(from <= to, "range: start ({from}) > end ({to})"); (from, to - from) } (from, None) => (from, unbounded), diff --git a/godot-core/src/obj/base.rs b/godot-core/src/obj/base.rs index 05cd82e47..676e34302 100644 --- a/godot-core/src/obj/base.rs +++ b/godot-core/src/obj/base.rs @@ -5,14 +5,14 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -#[cfg(safeguards_strict)] +#[cfg(safeguards_balanced)] use std::cell::Cell; use std::cell::RefCell; use std::collections::hash_map::Entry; use std::collections::HashMap; use std::fmt::{Debug, Display, Formatter, Result as FmtResult}; use std::mem::ManuallyDrop; -#[cfg(safeguards_strict)] +#[cfg(safeguards_balanced)] use std::rc::Rc; use crate::builtin::Callable; @@ -28,7 +28,7 @@ thread_local! { } /// Represents the initialization state of a `Base` object. -#[cfg(safeguards_strict)] +#[cfg(safeguards_balanced)] // TODO(v0.5 or v0.6): relax to strict state, once people are used to checks. #[derive(Debug, Copy, Clone, PartialEq, Eq)] enum InitState { /// Object is being constructed (inside `I*::init()` or `Gd::from_init_fn()`). @@ -39,14 +39,14 @@ enum InitState { Script, } -#[cfg(safeguards_strict)] +#[cfg(safeguards_balanced)] macro_rules! base_from_obj { ($obj:expr, $state:expr) => { Base::from_obj($obj, $state) }; } -#[cfg(not(safeguards_strict))] +#[cfg(not(safeguards_balanced))] macro_rules! base_from_obj { ($obj:expr, $state:expr) => { Base::from_obj($obj) @@ -83,7 +83,7 @@ pub struct Base { /// Tracks the initialization state of this `Base` in Debug mode. /// /// Rc allows to "copy-construct" the base from an existing one, while still affecting the user-instance through the original `Base`. - #[cfg(safeguards_strict)] + #[cfg(safeguards_balanced)] init_state: Rc>, } @@ -96,8 +96,7 @@ impl Base { /// `base` must be alive at the time of invocation, i.e. user `init()` (which could technically destroy it) must not have run yet. /// If `base` is destroyed while the returned `Base` is in use, that constitutes a logic error, not a safety issue. pub(crate) unsafe fn from_base(base: &Base) -> Base { - #[cfg(safeguards_balanced)] - assert!( + sys::balanced_assert!( base.obj.is_instance_valid(), "Cannot construct Base; was object freed during initialization?" ); @@ -106,7 +105,7 @@ impl Base { Self { obj: ManuallyDrop::new(obj), - #[cfg(safeguards_strict)] + #[cfg(safeguards_balanced)] init_state: Rc::clone(&base.init_state), } } @@ -119,8 +118,7 @@ impl Base { /// `gd` must be alive at the time of invocation. If it is destroyed while the returned `Base` is in use, that constitutes a logic /// error, not a safety issue. pub(crate) unsafe fn from_script_gd(gd: &Gd) -> Self { - #[cfg(safeguards_strict)] - assert!(gd.is_instance_valid()); + sys::balanced_assert!(gd.is_instance_valid()); let obj = Gd::from_obj_sys_weak(gd.obj_sys()); base_from_obj!(obj, InitState::Script) @@ -134,7 +132,7 @@ impl Base { /// `base_ptr` must point to a valid, live object at the time of invocation. If it is destroyed while the returned `Base` is in use, /// that constitutes a logic error, not a safety issue. pub(crate) unsafe fn from_sys(base_ptr: sys::GDExtensionObjectPtr) -> Self { - assert!(!base_ptr.is_null(), "instance base is null pointer"); + sys::balanced_assert!(!base_ptr.is_null(), "instance base is null pointer"); // Initialize only as weak pointer (don't increment reference count). let obj = Gd::from_obj_sys_weak(base_ptr); @@ -147,7 +145,7 @@ impl Base { base_from_obj!(obj, InitState::ObjectConstructing) } - #[cfg(safeguards_strict)] + #[cfg(safeguards_balanced)] fn from_obj(obj: Gd, init_state: InitState) -> Self { Self { obj: ManuallyDrop::new(obj), @@ -155,7 +153,7 @@ impl Base { } } - #[cfg(not(safeguards_strict))] + #[cfg(not(safeguards_balanced))] fn from_obj(obj: Gd) -> Self { Self { obj: ManuallyDrop::new(obj), @@ -182,8 +180,7 @@ impl Base { /// # Panics (Debug) /// If called outside an initialization function, or for ref-counted objects on a non-main thread. pub fn to_init_gd(&self) -> Gd { - #[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols. - assert!( + sys::balanced_assert!( self.is_initializing(), "Base::to_init_gd() can only be called during object initialization, inside I*::init() or Gd::from_init_fn()" ); @@ -194,7 +191,7 @@ impl Base { return Gd::clone(&self.obj); } - debug_assert!( + sys::balanced_assert!( sys::is_main_thread(), "Base::to_init_gd() can only be called on the main thread for ref-counted objects (for now)" ); @@ -236,7 +233,7 @@ impl Base { PENDING_STRONG_REFS.with(|refs| { let mut pending_refs = refs.borrow_mut(); let strong_ref = pending_refs.remove(&instance_id); - assert!( + sys::strict_assert!( strong_ref.is_some(), "Base unexpectedly had its strong ref rug-pulled" ); @@ -254,7 +251,7 @@ impl Base { /// Finalizes the initialization of this `Base` and returns whether pub(crate) fn mark_initialized(&mut self) { - #[cfg(safeguards_strict)] + #[cfg(safeguards_balanced)] { assert_eq!( self.init_state.get(), @@ -268,18 +265,6 @@ impl Base { // May return whether there is a "surplus" strong ref in the future, as self.extra_strong_ref.borrow().is_some(). } - /// Returns a [`Gd`] referencing the base object, assuming the derived object is fully constructed. - #[doc(hidden)] - pub fn __fully_constructed_gd(&self) -> Gd { - #[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols. - assert!( - !self.is_initializing(), - "WithBaseField::to_gd(), base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()" - ); - - (*self.obj).clone() - } - /// Returns a [`Gd`] referencing the base object, for use in script contexts only. #[doc(hidden)] pub fn __script_gd(&self) -> Gd { @@ -302,7 +287,7 @@ impl Base { /// Returns a passive reference to the base object, for use in script contexts only. pub(crate) fn to_script_passive(&self) -> PassiveGd { - #[cfg(safeguards_strict)] + #[cfg(safeguards_balanced)] assert_eq!( self.init_state.get(), InitState::Script, @@ -314,16 +299,21 @@ impl Base { } /// Returns `true` if this `Base` is currently in the initializing state. - #[cfg(safeguards_strict)] + #[cfg(safeguards_balanced)] fn is_initializing(&self) -> bool { self.init_state.get() == InitState::ObjectConstructing } + /// Always returns `false` when balanced safeguards are disabled. + #[cfg(not(safeguards_balanced))] + fn is_initializing(&self) -> bool { + false + } + /// Returns a [`Gd`] referencing the base object, assuming the derived object is fully constructed. #[doc(hidden)] pub fn __constructed_gd(&self) -> Gd { - #[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols. - assert!( + sys::balanced_assert!( !self.is_initializing(), "WithBaseField::to_gd(), base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()" ); @@ -339,8 +329,7 @@ impl Base { /// # Safety /// Caller must ensure that the underlying object remains valid for the entire lifetime of the returned `PassiveGd`. pub(crate) unsafe fn constructed_passive(&self) -> PassiveGd { - #[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols. - assert!( + sys::balanced_assert!( !self.is_initializing(), "WithBaseField::base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()" ); diff --git a/godot-core/src/obj/casts.rs b/godot-core/src/obj/casts.rs index 9571963e2..9bbec2bfe 100644 --- a/godot-core/src/obj/casts.rs +++ b/godot-core/src/obj/casts.rs @@ -11,6 +11,7 @@ use std::mem::ManuallyDrop; use godot_ffi::GodotNullableFfi; use crate::obj::{GodotClass, RawGd}; +use crate::sys; /// Represents a successful low-level cast from `T` to `U`. /// @@ -66,7 +67,7 @@ impl CastSuccess { /// This trade is needed because the result is a weak pointer (no ref-count increment). By submitting a strong pointer in its place, /// we can retain the overall ref-count balance. pub fn into_dest(self, traded_source: RawGd) -> RawGd { - debug_assert_eq!( + sys::strict_assert_eq!( traded_source.instance_id_unchecked(), self.dest.instance_id_unchecked(), "traded_source must point to the same object as the destination" diff --git a/godot-core/src/obj/gd.rs b/godot-core/src/obj/gd.rs index 18af4b086..b0b4691b7 100644 --- a/godot-core/src/obj/gd.rs +++ b/godot-core/src/obj/gd.rs @@ -611,7 +611,7 @@ impl Gd { /// This is the default for most initializations from FFI. In cases where reference counter /// should explicitly **not** be updated, [`Self::from_obj_sys_weak`] is available. pub(crate) unsafe fn from_obj_sys(ptr: sys::GDExtensionObjectPtr) -> Self { - debug_assert!( + sys::strict_assert!( !ptr.is_null(), "Gd::from_obj_sys() called with null pointer" ); diff --git a/godot-core/src/obj/raw_gd.rs b/godot-core/src/obj/raw_gd.rs index e260064f3..afdd2b0ba 100644 --- a/godot-core/src/obj/raw_gd.rs +++ b/godot-core/src/obj/raw_gd.rs @@ -12,9 +12,9 @@ use sys::{interface_fn, ExtVariantType, GodotFfi, GodotNullableFfi, PtrcallType} use crate::builtin::{Variant, VariantType}; use crate::meta::error::{ConvertError, FromVariantError}; -use crate::meta::{ - CallContext, ClassId, FromGodot, GodotConvert, GodotFfiVariant, GodotType, RefArg, ToGodot, -}; +#[cfg(safeguards_balanced)] +use crate::meta::CallContext; +use crate::meta::{ClassId, FromGodot, GodotConvert, GodotFfiVariant, GodotType, RefArg, ToGodot}; use crate::obj::bounds::{Declarer, DynMemory as _}; use crate::obj::casts::CastSuccess; use crate::obj::rtti::ObjectRtti; @@ -242,7 +242,7 @@ impl RawGd { /// # Safety /// `self` must not be null. pub(crate) unsafe fn as_non_null(&self) -> &Gd { - debug_assert!( + sys::strict_assert!( !self.is_null(), "RawGd::as_non_null() called on null pointer; this is UB" ); @@ -357,7 +357,7 @@ impl RawGd { { // Validation object identity. self.check_rtti("upcast_ref"); - debug_assert!(!self.is_null(), "cannot upcast null object refs"); + sys::balanced_assert!(!self.is_null(), "cannot upcast null object refs"); // In Debug builds, go the long path via Godot FFI to verify the results are the same. #[cfg(safeguards_strict)] diff --git a/godot-core/src/obj/traits.rs b/godot-core/src/obj/traits.rs index 2a69012c9..9d522259c 100644 --- a/godot-core/src/obj/traits.rs +++ b/godot-core/src/obj/traits.rs @@ -705,7 +705,6 @@ pub mod cap { use super::*; use crate::builtin::{StringName, Variant}; use crate::meta::PropertyInfo; - use crate::obj::{Base, Bounds, Gd}; use crate::storage::{IntoVirtualMethodReceiver, VirtualMethodReceiver}; /// Trait for all classes that are default-constructible from the Godot engine. @@ -741,7 +740,7 @@ pub mod cap { // 1. Separate trait `GodotUserDefault` for user classes, which then proliferates through all APIs and makes abstraction harder. // 2. Repeatedly implementing __godot_default() that forwards to something like Gd::default_user_instance(). Possible, but this // will make the step toward builder APIs more difficult, as users would need to re-implement this as well. - debug_assert_eq!( + sys::strict_assert_eq!( std::any::TypeId::of::<::Declarer>(), std::any::TypeId::of::(), "__godot_default() called on engine class; must be overridden for engine classes" diff --git a/godot-core/src/private.rs b/godot-core/src/private.rs index d92a2d7c3..4cc225061 100644 --- a/godot-core/src/private.rs +++ b/godot-core/src/private.rs @@ -5,7 +5,7 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ -#[cfg(debug_assertions)] +#[cfg(safeguards_strict)] use std::cell::RefCell; use std::io::Write; use std::sync::atomic; @@ -254,7 +254,7 @@ pub fn extract_panic_message(err: &(dyn Send + std::any::Any)) -> String { pub fn format_panic_message(panic_info: &std::panic::PanicHookInfo) -> String { let mut msg = extract_panic_message(panic_info.payload()); - if let Some(context) = get_gdext_panic_context() { + if let Some(context) = fetch_last_panic_context() { msg = format!("{msg}\nContext: {context}"); } @@ -276,7 +276,7 @@ pub fn format_panic_message(panic_info: &std::panic::PanicHookInfo) -> String { } // Macro instead of function, to avoid 1 extra frame in backtrace. -#[cfg(debug_assertions)] +#[cfg(safeguards_strict)] #[macro_export] macro_rules! format_backtrace { ($prefix:expr, $backtrace:expr) => {{ @@ -302,7 +302,7 @@ macro_rules! format_backtrace { }; } -#[cfg(not(debug_assertions))] +#[cfg(not(safeguards_strict))] #[macro_export] macro_rules! format_backtrace { ($prefix:expr $(, $backtrace:expr)? ) => { @@ -344,13 +344,13 @@ pub(crate) fn has_error_print_level(level: u8) -> bool { /// Internal type used to store context information for debug purposes. Debug context is stored on the thread-local /// ERROR_CONTEXT_STACK, which can later be used to retrieve the current context in the event of a panic. This value -/// probably shouldn't be used directly; use ['get_gdext_panic_context()'](get_gdext_panic_context) instead. -#[cfg(debug_assertions)] +/// probably shouldn't be used directly; use ['get_gdext_panic_context()'](fetch_last_panic_context) instead. +#[cfg(safeguards_strict)] struct ScopedFunctionStack { functions: Vec<*const dyn Fn() -> String>, } -#[cfg(debug_assertions)] +#[cfg(safeguards_strict)] impl ScopedFunctionStack { /// # Safety /// Function must be removed (using [`pop_function()`](Self::pop_function)) before lifetime is invalidated. @@ -375,7 +375,7 @@ impl ScopedFunctionStack { } } -#[cfg(debug_assertions)] +#[cfg(safeguards_strict)] thread_local! { static ERROR_CONTEXT_STACK: RefCell = const { RefCell::new(ScopedFunctionStack { functions: Vec::new() }) @@ -383,11 +383,11 @@ thread_local! { } // Value may return `None`, even from panic hook, if called from a non-Godot thread. -pub fn get_gdext_panic_context() -> Option { - #[cfg(debug_assertions)] +pub fn fetch_last_panic_context() -> Option { + #[cfg(safeguards_strict)] return ERROR_CONTEXT_STACK.with(|cell| cell.borrow().get_last()); - #[cfg(not(debug_assertions))] + #[cfg(not(safeguards_strict))] None } @@ -424,10 +424,10 @@ where E: Fn() -> String, F: FnOnce() -> R + std::panic::UnwindSafe, { - #[cfg(not(debug_assertions))] + #[cfg(not(safeguards_strict))] let _ = error_context; // Unused in Release. - #[cfg(debug_assertions)] + #[cfg(safeguards_strict)] ERROR_CONTEXT_STACK.with(|cell| unsafe { // SAFETY: &error_context is valid for lifetime of function, and is removed from LAST_ERROR_CONTEXT before end of function. cell.borrow_mut().push_function(&error_context) @@ -435,7 +435,7 @@ where let result = std::panic::catch_unwind(code).map_err(PanicPayload::new); - #[cfg(debug_assertions)] + #[cfg(safeguards_strict)] ERROR_CONTEXT_STACK.with(|cell| cell.borrow_mut().pop_function()); result } diff --git a/godot-core/src/registry/property/mod.rs b/godot-core/src/registry/property/mod.rs index a866a3727..8f33a4074 100644 --- a/godot-core/src/registry/property/mod.rs +++ b/godot-core/src/registry/property/mod.rs @@ -236,6 +236,7 @@ pub mod export_info_functions { use crate::meta::{GodotType, PropertyHintInfo, PropertyInfo}; use crate::obj::EngineEnum; use crate::registry::property::Export; + use crate::sys; /// Turn a list of variables into a comma separated string containing only the identifiers corresponding /// to a true boolean variable. @@ -426,7 +427,7 @@ pub mod export_info_functions { ) -> PropertyHintInfo { let field_ty = T::Via::property_info(""); let filter = filter.as_ref(); - debug_assert!(is_file || filter.is_empty()); // Dir never has filter. + sys::strict_assert!(is_file || filter.is_empty()); // Dir never has filter. export_file_or_dir_inner(&field_ty, is_file, is_global, filter) } diff --git a/godot-core/src/registry/signal/connect_handle.rs b/godot-core/src/registry/signal/connect_handle.rs index d7a65f9f2..b31b85f16 100644 --- a/godot-core/src/registry/signal/connect_handle.rs +++ b/godot-core/src/registry/signal/connect_handle.rs @@ -10,6 +10,7 @@ use std::borrow::Cow; use crate::builtin::Callable; use crate::classes::Object; use crate::obj::Gd; +use crate::sys; /// Handle representing a typed signal connection to a receiver. /// @@ -39,10 +40,10 @@ impl ConnectHandle { /// Disconnects the signal from the connected callable. /// - /// Panics (Debug) + /// # Panics (safeguards-balanced) /// If the connection does not exist. Use [`is_connected()`][Self::is_connected] to make sure the connection exists. pub fn disconnect(mut self) { - debug_assert!(self.is_connected()); + sys::balanced_assert!(self.is_connected()); self.receiver_object .disconnect(&*self.signal_name, &self.callable); diff --git a/godot-core/src/storage/instance_storage.rs b/godot-core/src/storage/instance_storage.rs index 57a765a0f..52ae1eef6 100644 --- a/godot-core/src/storage/instance_storage.rs +++ b/godot-core/src/storage/instance_storage.rs @@ -287,7 +287,7 @@ pub unsafe fn destroy_storage(instance_ptr: sys::GDExtensionClass // - Letting Gd and InstanceStorage know about this specific object state and panicking in the next Rust call might be an option, // but we still can't control direct access to the T. // - // For now we choose option 2 in Debug mode, and 4 in Release. + // For now we choose option 2 in strict+balanced levels, and 4 in disengaged level. let mut leak_rust_object = false; if (*raw).is_bound() { let error = format!( @@ -298,9 +298,9 @@ pub unsafe fn destroy_storage(instance_ptr: sys::GDExtensionClass (*raw).base() ); - // In Debug mode, crash which may trigger breakpoint. - // In Release mode, leak player object (Godot philosophy: don't crash if somehow avoidable). Likely leads to follow-up issues. - if cfg!(debug_assertions) { + // In strict+balanced level, crash which may trigger breakpoint. + // In disengaged level, leak player object (Godot philosophy: don't crash if somehow avoidable). Likely leads to follow-up issues. + if cfg!(safeguards_balanced) { let error = crate::builtin::GString::from(&error); crate::classes::Os::singleton().crash(&error); } else { diff --git a/godot-core/src/task/futures.rs b/godot-core/src/task/futures.rs index 2c1533d45..ae01ce0da 100644 --- a/godot-core/src/task/futures.rs +++ b/godot-core/src/task/futures.rs @@ -15,11 +15,12 @@ use std::thread::ThreadId; use crate::builtin::{Callable, RustCallable, Signal, Variant}; use crate::classes::object::ConnectFlags; -use crate::godot_error; +use crate::global::godot_error; use crate::meta::sealed::Sealed; use crate::meta::InParamTuple; use crate::obj::{Gd, GodotClass, WithSignals}; use crate::registry::signal::TypedSignal; +use crate::sys; // ---------------------------------------------------------------------------------------------------------------------------------------------- // Internal re-exports @@ -197,9 +198,11 @@ pub struct FallibleSignalFuture { impl FallibleSignalFuture { fn new(signal: Signal) -> Self { - debug_assert!( + sys::strict_assert!( !signal.is_null(), - "Failed to create a future for an invalid Signal!\nEither the signal object was already freed or the signal was not registered in the object before using it.", + "Failed to create future for invalid signal:\n\ + Either the signal object was already freed, or it\n\ + was not registered in the object before being used.", ); let data = Arc::new(Mutex::new(SignalFutureData::default())); diff --git a/godot-ffi/src/assertions.rs b/godot-ffi/src/assertions.rs new file mode 100644 index 000000000..0dc4127bb --- /dev/null +++ b/godot-ffi/src/assertions.rs @@ -0,0 +1,117 @@ +/* + * Copyright (c) godot-rust; Bromeon and contributors. + * This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this + * file, You can obtain one at https://mozilla.org/MPL/2.0/. + */ + +//! Assertion macros for compile-time and runtime checks with different safeguard levels. + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Compile-time assertions + +/// Verifies a condition at compile time. +// https://blog.rust-lang.org/2021/12/02/Rust-1.57.0.html#panic-in-const-contexts +#[macro_export] +macro_rules! static_assert { + ($cond:expr) => { + const _: () = assert!($cond); + }; + ($cond:expr, $msg:literal) => { + const _: () = assert!($cond, $msg); + }; +} + +/// Verifies at compile time that two types `T` and `U` have the same size and alignment. +#[macro_export] +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>() + && 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>() + && std::mem::align_of::<$T>() == std::mem::align_of::<$U>(), + $msg + ); + }; +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Runtime assertions - strict mode + +/// Acts like `assert!` when `safeguards_strict` is enabled (default in debug builds), and becomes a no-op otherwise. +#[macro_export] +macro_rules! strict_assert { + ($($arg:tt)*) => { + #[cfg(safeguards_strict)] + assert!($($arg)*); + }; +} + +/// Acts like `assert_eq!` when `safeguards_strict` is enabled (default in debug builds), and becomes a no-op otherwise. +#[macro_export] +macro_rules! strict_assert_eq { + ($actual:expr, $expected:expr) => { + #[cfg(safeguards_strict)] + assert_eq!($actual, $expected); + }; + ($actual:expr, $expected:expr, $($arg:tt)*) => { + #[cfg(safeguards_strict)] + assert_eq!($actual, $expected, $($arg)*); + }; +} + +/// Acts like `assert_ne!` when `safeguards_strict` is enabled (default in debug builds), and becomes a no-op otherwise. +#[macro_export] +macro_rules! strict_assert_ne { + ($actual:expr, $expected:expr) => { + #[cfg(safeguards_strict)] + assert_ne!($actual, $expected); + }; + ($actual:expr, $expected:expr, $($arg:tt)*) => { + #[cfg(safeguards_strict)] + assert_ne!($actual, $expected, $($arg)*); + }; +} + +// ---------------------------------------------------------------------------------------------------------------------------------------------- +// Runtime assertions - balanced mode + +/// Acts like `assert!` when `safeguards_balanced` is enabled, and becomes a no-op otherwise. +#[macro_export] +macro_rules! balanced_assert { + ($($arg:tt)*) => { + #[cfg(safeguards_balanced)] + assert!($($arg)*); + }; +} + +/// Acts like `assert_eq!` when `safeguards_balanced` is enabled, and becomes a no-op otherwise. +#[macro_export] +macro_rules! balanced_assert_eq { + ($actual:expr, $expected:expr) => { + #[cfg(safeguards_balanced)] + assert_eq!($actual, $expected); + }; + ($actual:expr, $expected:expr, $($arg:tt)*) => { + #[cfg(safeguards_balanced)] + assert_eq!($actual, $expected, $($arg)*); + }; +} + +/// Acts like `assert_ne!` when `safeguards_balanced` is enabled, and becomes a no-op otherwise. +#[macro_export] +macro_rules! balanced_assert_ne { + ($actual:expr, $expected:expr) => { + #[cfg(safeguards_balanced)] + assert_ne!($actual, $expected); + }; + ($actual:expr, $expected:expr, $($arg:tt)*) => { + #[cfg(safeguards_balanced)] + assert_ne!($actual, $expected, $($arg)*); + }; +} diff --git a/godot-ffi/src/binding/mod.rs b/godot-ffi/src/binding/mod.rs index 12d325ff4..3cd18a2a6 100644 --- a/godot-ffi/src/binding/mod.rs +++ b/godot-ffi/src/binding/mod.rs @@ -86,10 +86,10 @@ unsafe impl Send for ClassLibraryPtr {} /// # Safety /// The table must not have been initialized yet. -unsafe fn initialize_table(table: &ManualInitCell, value: T, what: &str) { - debug_assert!( +unsafe fn initialize_table(table: &ManualInitCell, value: T, _what: &str) { + crate::strict_assert!( !table.is_initialized(), - "method table for {what} should only be initialized once" + "method table for {_what} should only be initialized once" ); table.set(value) @@ -97,8 +97,8 @@ unsafe fn initialize_table(table: &ManualInitCell, value: T, what: &str) { /// # Safety /// The table must have been initialized. -unsafe fn get_table(table: &'static ManualInitCell, msg: &str) -> &'static T { - debug_assert!(table.is_initialized(), "{msg}"); +unsafe fn get_table(table: &'static ManualInitCell, _msg: &str) -> &'static T { + crate::strict_assert!(table.is_initialized(), "{_msg}"); table.get_unchecked() } diff --git a/godot-ffi/src/binding/multi_threaded.rs b/godot-ffi/src/binding/multi_threaded.rs index 5ecace4bb..e60c69fff 100644 --- a/godot-ffi/src/binding/multi_threaded.rs +++ b/godot-ffi/src/binding/multi_threaded.rs @@ -72,7 +72,7 @@ impl BindingStorage { pub unsafe fn get_binding_unchecked() -> &'static GodotBinding { let storage = Self::storage(); - debug_assert!( + crate::strict_assert!( storage.binding.is_initialized(), "Godot engine not available; make sure you are not calling it from unit/doc tests" ); diff --git a/godot-ffi/src/binding/single_threaded.rs b/godot-ffi/src/binding/single_threaded.rs index 54920bf88..c1f0bd10e 100644 --- a/godot-ffi/src/binding/single_threaded.rs +++ b/godot-ffi/src/binding/single_threaded.rs @@ -121,28 +121,7 @@ impl BindingStorage { // and this function is called from the main thread. let storage = unsafe { Self::storage() }; - // We only check if we are in the main thread in debug builds if we aren't building for a non-threaded Godot build, - // since we could otherwise assume there won't be multi-threading. - #[cfg(all(debug_assertions, not(wasm_nothreads)))] - { - if !crate::is_main_thread() { - // If a binding is accessed the first time, this will panic and start unwinding. It can then happen that during unwinding, - // another FFI call happens (e.g. Godot destructor), which would cause immediate abort, swallowing the error message. - // Thus check if a panic is already in progress. - - if std::thread::panicking() { - eprintln!( - "ERROR: Attempted to access binding from different thread than main thread; this is UB.\n\ - Cannot panic because panic unwind is already in progress. Please check surrounding messages to fix the bug." - ); - } else { - panic!( - "attempted to access binding from different thread than main thread; \ - this is UB - use the \"experimental-threads\" feature." - ) - }; - } - } + Self::ensure_main_thread(); // SAFETY: This function can only be called when the binding is initialized and from the main thread, so we know that it's initialized. unsafe { storage.binding.get_unchecked() } @@ -154,6 +133,29 @@ impl BindingStorage { storage.initialized() } + + fn ensure_main_thread() { + // Check that we're on the main thread. Only enabled with balanced+ safeguards and, for Wasm, in threaded builds. + // In wasm_nothreads, there's only one thread, so no check is needed. + #[cfg(all(safeguards_balanced, not(wasm_nothreads)))] + if !crate::is_main_thread() { + // If a binding is accessed the first time, this will panic and start unwinding. It can then happen that during unwinding, + // another FFI call happens (e.g. Godot destructor), which would cause immediate abort, swallowing the error message. + // Thus check if a panic is already in progress. + + if std::thread::panicking() { + eprintln!( + "ERROR: Attempted to access binding from different thread than main thread; this is UB.\n\ + Cannot panic because panic unwind is already in progress. Please check surrounding messages to fix the bug." + ); + } else { + panic!( + "attempted to access binding from different thread than main thread; \ + this is UB - use the \"experimental-threads\" feature." + ) + }; + } + } } // SAFETY: We ensure that `binding` is only ever accessed from the same thread that initialized it. @@ -161,6 +163,8 @@ unsafe impl Sync for BindingStorage {} // SAFETY: We ensure that `binding` is only ever accessed from the same thread that initialized it. unsafe impl Send for BindingStorage {} +// ---------------------------------------------------------------------------------------------------------------------------------------------- + pub struct GdextConfig { pub tool_only_in_editor: bool, is_editor: std::cell::OnceCell, diff --git a/godot-ffi/src/global.rs b/godot-ffi/src/global.rs index ebd5318fe..d8a55eb4a 100644 --- a/godot-ffi/src/global.rs +++ b/godot-ffi/src/global.rs @@ -127,7 +127,7 @@ mod global_guard { /// /// The value must be initialized. pub(super) unsafe fn new_unchecked(mutex_guard: MutexGuard<'a, OnceCell>) -> Self { - debug_assert!( + crate::strict_assert!( mutex_guard.get().is_some(), "safety precondition violated: cell not initialized" ); diff --git a/godot-ffi/src/lib.rs b/godot-ffi/src/lib.rs index 2dd2a5c37..2c0991ab4 100644 --- a/godot-ffi/src/lib.rs +++ b/godot-ffi/src/lib.rs @@ -50,6 +50,7 @@ pub(crate) mod gen { pub mod conv; +mod assertions; mod extras; mod global; mod godot_ffi; diff --git a/godot-ffi/src/toolbox.rs b/godot-ffi/src/toolbox.rs index b6c6f613e..7d55c1bfd 100644 --- a/godot-ffi/src/toolbox.rs +++ b/godot-ffi/src/toolbox.rs @@ -14,36 +14,6 @@ use crate as sys; // ---------------------------------------------------------------------------------------------------------------------------------------------- // Macros -/// Verifies a condition at compile time. -// https://blog.rust-lang.org/2021/12/02/Rust-1.57.0.html#panic-in-const-contexts -#[macro_export] -macro_rules! static_assert { - ($cond:expr) => { - const _: () = assert!($cond); - }; - ($cond:expr, $msg:literal) => { - const _: () = assert!($cond, $msg); - }; -} - -/// Verifies at compile time that two types `T` and `U` have the same size and alignment. -#[macro_export] -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>() - && 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>() - && std::mem::align_of::<$T>() == std::mem::align_of::<$U>(), - $msg - ); - }; -} - /// Trace output. #[cfg(feature = "debug-log")] #[macro_export] @@ -129,7 +99,7 @@ where #[inline] pub fn c_str(s: &[u8]) -> *const std::ffi::c_char { // Ensure null-terminated - debug_assert!(!s.is_empty() && s[s.len() - 1] == 0); + crate::strict_assert!(!s.is_empty() && s[s.len() - 1] == 0); s.as_ptr() as *const std::ffi::c_char } diff --git a/itest/rust/src/builtin_tests/containers/array_test.rs b/itest/rust/src/builtin_tests/containers/array_test.rs index 94c55bc8e..eb118e6c4 100644 --- a/itest/rust/src/builtin_tests/containers/array_test.rs +++ b/itest/rust/src/builtin_tests/containers/array_test.rs @@ -290,12 +290,12 @@ fn array_set() { fn array_set_readonly() { let mut array = array![1, 2].into_read_only(); - #[cfg(debug_assertions)] - expect_panic("Mutating read-only array in Debug mode", || { + #[cfg(safeguards_balanced)] + expect_panic("Mutating read-only array with balanced safeguards", || { array.set(0, 3); }); - #[cfg(not(debug_assertions))] + #[cfg(not(safeguards_balanced))] array.set(0, 3); // silently fails. assert_eq!(array.at(0), 1); diff --git a/itest/rust/src/builtin_tests/containers/callable_test.rs b/itest/rust/src/builtin_tests/containers/callable_test.rs index e2c055a79..66588b9b4 100644 --- a/itest/rust/src/builtin_tests/containers/callable_test.rs +++ b/itest/rust/src/builtin_tests/containers/callable_test.rs @@ -414,14 +414,12 @@ pub mod custom_callable { let crosser = ThreadCrosser::new(callable); // Create separate thread and ensure calling fails. - // Why expect_panic for (single-threaded && Debug) but not (multi-threaded || Release) mode: - // - Check is only enabled in Debug, not Release. - // - We currently can't catch panics from Callable invocations, see above. True for both single/multi-threaded. - // - In single-threaded mode, there's an FFI access check which panics as soon as another thread is invoked. *This* panics. - // - In multi-threaded, we need to observe the effect instead (see below). - - if !cfg!(feature = "experimental-threads") && cfg!(debug_assertions) { - // Single-threaded and Debug. + // Why expect_panic for (safeguards_balanced && single-threaded) but not otherwise: + // - In single-threaded mode with balanced safeguards, there's an FFI access check which panics when another thread is invoked. + // - In multi-threaded mode OR with safeguards disengaged, the callable may or may not execute, but won't panic at the FFI level. + // - We can't catch panics from Callable invocations yet (see above), only the FFI access panics. + if cfg!(safeguards_balanced) && !cfg!(feature = "experimental-threads") { + // Single-threaded with balanced safeguards: FFI access check will panic. crate::framework::expect_panic( "Callable created with from_fn() must panic when invoked on other thread", || { @@ -432,18 +430,17 @@ pub mod custom_callable { }, ); } else { - // Multi-threaded OR Release. + // Multi-threaded OR safeguards disengaged: No FFI panic, but callable may or may not execute. quick_thread(|| { let callable = unsafe { crosser.extract() }; callable.callv(&varray![5]); }); } - assert_eq!( - *GLOBAL.lock(), - 0, - "Callable created with from_fn() must not run when invoked on other thread" - ); + // Expected value depends on whether thread checks are enforced. + // 777: callable *is* executed on other thread. + let expected = if cfg!(safeguards_balanced) { 0 } else { 777 }; + assert_eq!(*GLOBAL.lock(), expected); } #[itest] diff --git a/itest/rust/src/builtin_tests/containers/dictionary_test.rs b/itest/rust/src/builtin_tests/containers/dictionary_test.rs index 6bb5f838a..55edd4935 100644 --- a/itest/rust/src/builtin_tests/containers/dictionary_test.rs +++ b/itest/rust/src/builtin_tests/containers/dictionary_test.rs @@ -12,7 +12,9 @@ use godot::classes::RefCounted; use godot::meta::{ElementType, FromGodot, ToGodot}; use godot::obj::NewGd; -use crate::framework::{assert_match, create_gdscript, expect_panic, itest}; +use crate::framework::{ + assert_match, create_gdscript, expect_panic, expect_panic_or_nothing, itest, +}; #[itest] fn dictionary_default() { @@ -279,13 +281,13 @@ fn dictionary_set() { fn dictionary_set_readonly() { let mut dictionary = vdict! { "zero": 0, "one": 1 }.into_read_only(); - #[cfg(debug_assertions)] - expect_panic("Mutating read-only dictionary in Debug mode", || { - dictionary.set("zero", 2); - }); - - #[cfg(not(debug_assertions))] - dictionary.set("zero", 2); // silently fails. + // Fails silently in safeguards-disengaged (no UB). + expect_panic_or_nothing( + "Mutating read-only dictionary (safeguards-balanced)", + || { + dictionary.set("zero", 2); + }, + ); assert_eq!(dictionary.at("zero"), 0.to_variant()); } diff --git a/itest/rust/src/builtin_tests/containers/signal_disconnect_test.rs b/itest/rust/src/builtin_tests/containers/signal_disconnect_test.rs index 7493e86fe..568d4acf5 100644 --- a/itest/rust/src/builtin_tests/containers/signal_disconnect_test.rs +++ b/itest/rust/src/builtin_tests/containers/signal_disconnect_test.rs @@ -10,7 +10,7 @@ use godot::classes::Object; use godot::obj::{Base, Gd, NewAlloc}; use godot::register::{godot_api, ConnectHandle, GodotClass}; -use crate::framework::{expect_debug_panic_or_release_ok, expect_panic, itest}; +use crate::framework::{expect_panic, expect_panic_or_nothing, itest}; #[derive(GodotClass)] #[class(init, base=Object)] @@ -223,7 +223,7 @@ fn test_handle_recognizes_non_valid_state(disconnect_function: impl FnOnce(&mut let is_valid = handle.is_connected(); assert!(!is_valid); - expect_debug_panic_or_release_ok("disconnect invalid handle", || { + expect_panic_or_nothing("disconnect invalid handle", || { handle.disconnect(); }); diff --git a/itest/rust/src/builtin_tests/string/gstring_test.rs b/itest/rust/src/builtin_tests/string/gstring_test.rs index 39c714059..2b7744fca 100644 --- a/itest/rust/src/builtin_tests/string/gstring_test.rs +++ b/itest/rust/src/builtin_tests/string/gstring_test.rs @@ -9,7 +9,7 @@ use std::collections::HashSet; use godot::builtin::{Encoding, GString, PackedStringArray}; -use crate::framework::{expect_debug_panic_or_release_ok, itest}; +use crate::framework::{expect_panic_or_nothing, itest}; // TODO use tests from godot-rust/gdnative @@ -95,7 +95,7 @@ fn string_unicode_at() { assert_eq!(s.unicode_at(3), '💡'); // Release mode: out-of-bounds prints Godot error, but returns 0. - expect_debug_panic_or_release_ok("unicode_at() out-of-bounds panics", || { + expect_panic_or_nothing("unicode_at() out-of-bounds panics", || { assert_eq!(s.unicode_at(4), '\0'); }); } diff --git a/itest/rust/src/builtin_tests/string/node_path_test.rs b/itest/rust/src/builtin_tests/string/node_path_test.rs index 918d6348e..4f9ffdf7c 100644 --- a/itest/rust/src/builtin_tests/string/node_path_test.rs +++ b/itest/rust/src/builtin_tests/string/node_path_test.rs @@ -10,7 +10,7 @@ use std::collections::HashSet; use godot::builtin::{GString, NodePath}; use godot::meta::wrapped; -use crate::framework::{expect_debug_panic_or_release_ok, itest}; +use crate::framework::{expect_panic_or_nothing, itest}; #[itest] fn node_path_default() { @@ -113,7 +113,7 @@ fn node_path_get_name() { assert_eq!(path.get_name(1), "RigidBody2D".into()); assert_eq!(path.get_name(2), "Sprite2D".into()); - expect_debug_panic_or_release_ok("NodePath::get_name() out of bounds", || { + expect_panic_or_nothing("NodePath::get_name() out of bounds", || { assert_eq!(path.get_name(3), "".into()); }) } @@ -124,7 +124,7 @@ fn node_path_get_subname() { assert_eq!(path.get_subname(0), "texture".into()); assert_eq!(path.get_subname(1), "resource_name".into()); - expect_debug_panic_or_release_ok("NodePath::get_subname() out of bounds", || { + expect_panic_or_nothing("NodePath::get_subname() out of bounds", || { assert_eq!(path.get_subname(2), "".into()); }) } diff --git a/itest/rust/src/framework/mod.rs b/itest/rust/src/framework/mod.rs index 52684eeb3..86e57eb8f 100644 --- a/itest/rust/src/framework/mod.rs +++ b/itest/rust/src/framework/mod.rs @@ -199,7 +199,7 @@ pub fn expect_panic(context: &str, code: impl FnOnce()) { ); } -/// Run for code that panics in *strict* and *balanced* safeguard levels, but cause UB in *disengaged* level. +/// Run for code that should panic in *strict* and *balanced* safeguard levels, but cause UB in *disengaged* level. /// /// The code is not executed for the latter. pub fn expect_panic_or_ub(_context: &str, _code: impl FnOnce()) { @@ -207,6 +207,17 @@ pub fn expect_panic_or_ub(_context: &str, _code: impl FnOnce()) { expect_panic(_context, _code); } +/// Run for code that should panic in *strict* and *balanced* safeguard levels, but do nothing in *disengaged* level. +/// +/// The code is executed either way, and must not cause UB. +pub fn expect_panic_or_nothing(_context: &str, code: impl FnOnce()) { + #[cfg(safeguards_balanced)] + expect_panic(_context, code); + + #[cfg(not(safeguards_balanced))] + code() +} + pin_project_lite::pin_project! { pub struct ExpectPanicFuture { context: &'static str, @@ -250,14 +261,6 @@ pub fn expect_async_panic( ExpectPanicFuture { context, future } } -pub fn expect_debug_panic_or_release_ok(_context: &str, code: impl FnOnce()) { - #[cfg(debug_assertions)] - expect_panic(_context, code); - - #[cfg(not(debug_assertions))] - code() -} - /// Run code asynchronously, at the very start of the next _process_ frame (before `INode::process()`). /// /// If there is a custom `MainLoop` present, it will be run _before_ this. diff --git a/itest/rust/src/object_tests/class_name_test.rs b/itest/rust/src/object_tests/class_name_test.rs index 543e09016..84f5cc745 100644 --- a/itest/rust/src/object_tests/class_name_test.rs +++ b/itest/rust/src/object_tests/class_name_test.rs @@ -153,7 +153,7 @@ fn class_name_debug() { assert_eq!(format!("{static_name:?}"), "ClassId(\"MyStaticClass\")"); } -#[cfg(debug_assertions)] +#[cfg(safeguards_balanced)] #[itest] fn class_name_alloc_panic() { // ASCII. diff --git a/itest/rust/src/object_tests/dynamic_call_test.rs b/itest/rust/src/object_tests/dynamic_call_test.rs index 76e29d667..5079d79d5 100644 --- a/itest/rust/src/object_tests/dynamic_call_test.rs +++ b/itest/rust/src/object_tests/dynamic_call_test.rs @@ -152,7 +152,7 @@ fn dynamic_call_with_panic() { std::panic::set_hook(Box::new(move |panic_info| { let error_message = godot::private::format_panic_message(panic_info); *panic_message_clone.lock().unwrap() = - Some((error_message, godot::private::get_gdext_panic_context())); + Some((error_message, godot::private::fetch_last_panic_context())); })); let mut obj = ObjPayload::new_alloc(); @@ -187,9 +187,9 @@ fn dynamic_call_with_panic() { .map(|context| format!("\n Context: {context}")) .unwrap_or_default(); - // In Debug, there is a context -> message is multi-line -> '\n' is inserted after [panic ...]. - // In Release, simpler message -> single line -> no '\n'. - let expected_panic_message = if cfg!(debug_assertions) { + // In strict level, there is a context -> message is multi-line -> '\n' is inserted after [panic ...]. + // In balanced+disengaged level, simpler message -> single line -> no '\n'. + let expected_panic_message = if cfg!(safeguards_strict) { format!("[panic {path}:{line}]\n do_panic exploded 💥{context}") } else { format!("[panic {path}:{line}] do_panic exploded 💥")