Skip to content

Commit 5e75d1c

Browse files
committed
Replace debug_assert! with strict/balanced assertions
1 parent e93446c commit 5e75d1c

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

41 files changed

+272
-259
lines changed

godot-core/src/builtin/aabb.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -366,7 +366,7 @@ impl Aabb {
366366
// Credits: https://tavianator.com/2011/ray_box.html
367367
fn compute_ray_tnear_tfar(self, ray_from: Vector3, ray_dir: Vector3) -> (real, real) {
368368
self.assert_nonnegative();
369-
debug_assert!(
369+
sys::balanced_assert!(
370370
ray_dir != Vector3::ZERO,
371371
"ray direction must not be zero; use contains_point() for point checks"
372372
);
@@ -763,9 +763,8 @@ mod test {
763763
);
764764
}
765765

766-
#[test]
767-
#[should_panic]
768-
#[cfg(debug_assertions)]
766+
#[test] // cfg_attr: no panic in disengaged level (although current CI doesn't run unit-tests).
767+
#[cfg_attr(safeguards_balanced, should_panic)]
769768
fn test_intersect_ray_zero_dir_inside() {
770769
let aabb = Aabb {
771770
position: Vector3::new(-1.5, 2.0, -2.5),
@@ -776,8 +775,7 @@ mod test {
776775
}
777776

778777
#[test]
779-
#[should_panic]
780-
#[cfg(debug_assertions)]
778+
#[cfg_attr(safeguards_balanced, should_panic)]
781779
fn test_intersect_ray_zero_dir_outside() {
782780
let aabb = Aabb {
783781
position: Vector3::new(-1.5, 2.0, -2.5),

godot-core/src/builtin/callable.rs

Lines changed: 23 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -309,22 +309,19 @@ impl Callable {
309309
_name: S,
310310
rust_function: F,
311311
thread_id: Option<ThreadId>,
312-
linked_obj_id: Option<InstanceId>,
312+
linked_object_id: Option<InstanceId>,
313313
) -> Self
314314
where
315315
F: FnMut(&[&Variant]) -> R,
316316
R: ToGodot,
317317
S: meta::AsArg<GString>,
318318
{
319-
#[cfg(safeguards_balanced)]
320-
meta::arg_into_owned!(_name);
321-
322319
let wrapper = FnWrapper {
323320
rust_function,
324321
#[cfg(safeguards_balanced)]
325-
name: _name,
322+
name: { _name.into_arg().cow_into_owned() },
326323
thread_id,
327-
linked_obj_id,
324+
linked_object_id,
328325
};
329326

330327
let object_id = wrapper.linked_object_id();
@@ -610,15 +607,28 @@ mod custom_callable {
610607
/// `None` if the callable is multi-threaded ([`Callable::from_sync_fn`]).
611608
pub(super) thread_id: Option<ThreadId>,
612609
/// `None` if callable is not linked with any object.
613-
pub(super) linked_obj_id: Option<InstanceId>,
610+
pub(super) linked_object_id: Option<InstanceId>,
614611
}
615612

616613
impl<F> FnWrapper<F> {
617614
pub(crate) fn linked_object_id(&self) -> GDObjectInstanceID {
618-
self.linked_obj_id.map(InstanceId::to_u64).unwrap_or(0)
615+
self.linked_object_id.map(InstanceId::to_u64).unwrap_or(0)
619616
}
620617
}
621618

619+
/// Returns the name for safeguard-enabled builds, or `"<optimized out>"` otherwise.
620+
macro_rules! name_or_optimized {
621+
($($code:tt)*) => {
622+
{
623+
#[cfg(safeguards_balanced)]
624+
{ $($code)* }
625+
626+
#[cfg(not(safeguards_balanced))]
627+
{ "<optimized out>" }
628+
}
629+
};
630+
}
631+
622632
/// Represents a custom callable object defined in Rust.
623633
///
624634
/// This trait has a single method, `invoke`, which is called upon invocation.
@@ -655,14 +665,11 @@ mod custom_callable {
655665
) {
656666
let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize);
657667

658-
#[cfg(safeguards_balanced)]
659-
let name = &{
668+
let name = name_or_optimized! {
660669
let c: &C = CallableUserdata::inner_from_raw(callable_userdata);
661670
c.to_string()
662671
};
663-
#[cfg(not(safeguards_balanced))]
664-
let name = "<optimized out>";
665-
let ctx = meta::CallContext::custom_callable(name);
672+
let ctx = meta::CallContext::custom_callable(&name);
666673

667674
crate::private::handle_fallible_varcall(&ctx, &mut *r_error, move || {
668675
// Get the RustCallable again inside closure so it doesn't have to be UnwindSafe.
@@ -685,23 +692,17 @@ mod custom_callable {
685692
{
686693
let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize);
687694

688-
#[cfg(safeguards_balanced)]
689-
let name = &{
695+
let name = name_or_optimized! {
690696
let w: &FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
691697
w.name.to_string()
692698
};
693-
#[cfg(not(safeguards_balanced))]
694-
let name = "<optimized out>";
695-
let ctx = meta::CallContext::custom_callable(name);
699+
let ctx = meta::CallContext::custom_callable(&name);
696700

697701
crate::private::handle_fallible_varcall(&ctx, &mut *r_error, move || {
698702
// Get the FnWrapper again inside closure so the FnMut doesn't have to be UnwindSafe.
699703
let w: &mut FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
700704

701-
#[cfg(safeguards_balanced)]
702-
let name = &w.name;
703-
#[cfg(not(safeguards_balanced))]
704-
let name = "<optimized out>";
705+
let name = name_or_optimized!(w.name.to_string());
705706

706707
// NOTE: this panic is currently not propagated to the caller, but results in an error message and Nil return.
707708
// See comments in itest callable_call() for details.

godot-core/src/builtin/collections/array.rs

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ impl<T: ArrayElement> Array<T> {
319319

320320
/// Clears the array, removing all elements.
321321
pub fn clear(&mut self) {
322-
self.debug_ensure_mutable();
322+
self.balanced_ensure_mutable();
323323

324324
// SAFETY: No new values are written to the array, we only remove values from the array.
325325
unsafe { self.as_inner_mut() }.clear();
@@ -331,7 +331,7 @@ impl<T: ArrayElement> Array<T> {
331331
///
332332
/// If `index` is out of bounds.
333333
pub fn set(&mut self, index: usize, value: impl AsArg<T>) {
334-
self.debug_ensure_mutable();
334+
self.balanced_ensure_mutable();
335335

336336
let ptr_mut = self.ptr_mut(index);
337337

@@ -348,7 +348,7 @@ impl<T: ArrayElement> Array<T> {
348348
#[doc(alias = "append")]
349349
#[doc(alias = "push_back")]
350350
pub fn push(&mut self, value: impl AsArg<T>) {
351-
self.debug_ensure_mutable();
351+
self.balanced_ensure_mutable();
352352

353353
meta::arg_into_ref!(value: T);
354354

@@ -362,7 +362,7 @@ impl<T: ArrayElement> Array<T> {
362362
/// On large arrays, this method is much slower than [`push()`][Self::push], as it will move all the array's elements.
363363
/// The larger the array, the slower `push_front()` will be.
364364
pub fn push_front(&mut self, value: impl AsArg<T>) {
365-
self.debug_ensure_mutable();
365+
self.balanced_ensure_mutable();
366366

367367
meta::arg_into_ref!(value: T);
368368

@@ -376,7 +376,7 @@ impl<T: ArrayElement> Array<T> {
376376
/// _Godot equivalent: `pop_back`_
377377
#[doc(alias = "pop_back")]
378378
pub fn pop(&mut self) -> Option<T> {
379-
self.debug_ensure_mutable();
379+
self.balanced_ensure_mutable();
380380

381381
(!self.is_empty()).then(|| {
382382
// SAFETY: We do not write any values to the array, we just remove one.
@@ -390,7 +390,7 @@ impl<T: ArrayElement> Array<T> {
390390
/// Note: On large arrays, this method is much slower than `pop()` as it will move all the
391391
/// array's elements. The larger the array, the slower `pop_front()` will be.
392392
pub fn pop_front(&mut self) -> Option<T> {
393-
self.debug_ensure_mutable();
393+
self.balanced_ensure_mutable();
394394

395395
(!self.is_empty()).then(|| {
396396
// SAFETY: We do not write any values to the array, we just remove one.
@@ -407,7 +407,7 @@ impl<T: ArrayElement> Array<T> {
407407
/// # Panics
408408
/// If `index > len()`.
409409
pub fn insert(&mut self, index: usize, value: impl AsArg<T>) {
410-
self.debug_ensure_mutable();
410+
self.balanced_ensure_mutable();
411411

412412
let len = self.len();
413413
assert!(
@@ -431,8 +431,7 @@ impl<T: ArrayElement> Array<T> {
431431
/// If `index` is out of bounds.
432432
#[doc(alias = "pop_at")]
433433
pub fn remove(&mut self, index: usize) -> T {
434-
self.debug_ensure_mutable();
435-
434+
self.balanced_ensure_mutable();
436435
self.check_bounds(index);
437436

438437
// SAFETY: We do not write any values to the array, we just remove one.
@@ -447,7 +446,7 @@ impl<T: ArrayElement> Array<T> {
447446
/// On large arrays, this method is much slower than [`pop()`][Self::pop], as it will move all the array's
448447
/// elements after the removed element.
449448
pub fn erase(&mut self, value: impl AsArg<T>) {
450-
self.debug_ensure_mutable();
449+
self.balanced_ensure_mutable();
451450

452451
meta::arg_into_ref!(value: T);
453452

@@ -458,7 +457,7 @@ impl<T: ArrayElement> Array<T> {
458457
/// Assigns the given value to all elements in the array. This can be used together with
459458
/// `resize` to create an array with a given size and initialized elements.
460459
pub fn fill(&mut self, value: impl AsArg<T>) {
461-
self.debug_ensure_mutable();
460+
self.balanced_ensure_mutable();
462461

463462
meta::arg_into_ref!(value: T);
464463

@@ -473,7 +472,7 @@ impl<T: ArrayElement> Array<T> {
473472
///
474473
/// If you know that the new size is smaller, then consider using [`shrink`](Array::shrink) instead.
475474
pub fn resize(&mut self, new_size: usize, value: impl AsArg<T>) {
476-
self.debug_ensure_mutable();
475+
self.balanced_ensure_mutable();
477476

478477
let original_size = self.len();
479478

@@ -505,7 +504,7 @@ impl<T: ArrayElement> Array<T> {
505504
/// If you want to increase the size of the array, use [`resize`](Array::resize) instead.
506505
#[doc(alias = "resize")]
507506
pub fn shrink(&mut self, new_size: usize) -> bool {
508-
self.debug_ensure_mutable();
507+
self.balanced_ensure_mutable();
509508

510509
if new_size >= self.len() {
511510
return false;
@@ -519,7 +518,7 @@ impl<T: ArrayElement> Array<T> {
519518

520519
/// Appends another array at the end of this array. Equivalent of `append_array` in GDScript.
521520
pub fn extend_array(&mut self, other: &Array<T>) {
522-
self.debug_ensure_mutable();
521+
self.balanced_ensure_mutable();
523522

524523
// SAFETY: `append_array` will only read values from `other`, and all types can be converted to `Variant`.
525524
let other: &VariantArray = unsafe { other.assume_type_ref::<Variant>() };
@@ -743,7 +742,7 @@ impl<T: ArrayElement> Array<T> {
743742

744743
/// Reverses the order of the elements in the array.
745744
pub fn reverse(&mut self) {
746-
self.debug_ensure_mutable();
745+
self.balanced_ensure_mutable();
747746

748747
// SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type.
749748
unsafe { self.as_inner_mut() }.reverse();
@@ -760,7 +759,7 @@ impl<T: ArrayElement> Array<T> {
760759
/// _Godot equivalent: `Array.sort()`_
761760
#[doc(alias = "sort")]
762761
pub fn sort_unstable(&mut self) {
763-
self.debug_ensure_mutable();
762+
self.balanced_ensure_mutable();
764763

765764
// SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type.
766765
unsafe { self.as_inner_mut() }.sort();
@@ -780,7 +779,7 @@ impl<T: ArrayElement> Array<T> {
780779
where
781780
F: FnMut(&T, &T) -> cmp::Ordering,
782781
{
783-
self.debug_ensure_mutable();
782+
self.balanced_ensure_mutable();
784783

785784
let godot_comparator = |args: &[&Variant]| {
786785
let lhs = T::from_variant(args[0]);
@@ -809,7 +808,7 @@ impl<T: ArrayElement> Array<T> {
809808
/// _Godot equivalent: `Array.sort_custom()`_
810809
#[doc(alias = "sort_custom")]
811810
pub fn sort_unstable_custom(&mut self, func: &Callable) {
812-
self.debug_ensure_mutable();
811+
self.balanced_ensure_mutable();
813812

814813
// SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type.
815814
unsafe { self.as_inner_mut() }.sort_custom(func);
@@ -819,7 +818,7 @@ impl<T: ArrayElement> Array<T> {
819818
/// global random number generator common to methods such as `randi`. Call `randomize` to
820819
/// ensure that a new seed will be used each time if you want non-reproducible shuffling.
821820
pub fn shuffle(&mut self) {
822-
self.debug_ensure_mutable();
821+
self.balanced_ensure_mutable();
823822

824823
// SAFETY: We do not write any values that don't already exist in the array, so all values have the correct type.
825824
unsafe { self.as_inner_mut() }.shuffle();
@@ -859,10 +858,10 @@ impl<T: ArrayElement> Array<T> {
859858

860859
/// Best-effort mutability check.
861860
///
862-
/// # Panics
863-
/// In Debug mode, if the array is marked as read-only.
864-
fn debug_ensure_mutable(&self) {
865-
debug_assert!(
861+
/// # Panics (safeguards-balanced)
862+
/// If the array is marked as read-only.
863+
fn balanced_ensure_mutable(&self) {
864+
sys::balanced_assert!(
866865
!self.is_read_only(),
867866
"mutating operation on read-only array"
868867
);
@@ -873,6 +872,7 @@ impl<T: ArrayElement> Array<T> {
873872
/// # Panics
874873
/// If `index` is out of bounds.
875874
fn check_bounds(&self, index: usize) {
875+
// Safety-relevant; explicitly *don't* use safeguards-dependent validation.
876876
let len = self.len();
877877
assert!(
878878
index < len,
@@ -1049,8 +1049,8 @@ impl<T: ArrayElement> Array<T> {
10491049
/// # Safety
10501050
/// Must only be called once, directly after creation.
10511051
unsafe fn init_inner_type(&mut self) {
1052-
debug_assert!(self.is_empty());
1053-
debug_assert!(
1052+
sys::strict_assert!(self.is_empty());
1053+
sys::strict_assert!(
10541054
self.cached_element_type.get().is_none(),
10551055
"init_inner_type() called twice"
10561056
);

godot-core/src/builtin/collections/dictionary.rs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ impl Dictionary {
163163
/// _Godot equivalent: `get_or_add`_
164164
#[doc(alias = "get_or_add")]
165165
pub fn get_or_insert<K: ToGodot, V: ToGodot>(&mut self, key: K, default: V) -> Variant {
166-
self.debug_ensure_mutable();
166+
self.balanced_ensure_mutable();
167167

168168
let key_variant = key.to_variant();
169169
let default_variant = default.to_variant();
@@ -237,7 +237,7 @@ impl Dictionary {
237237

238238
/// Removes all key-value pairs from the dictionary.
239239
pub fn clear(&mut self) {
240-
self.debug_ensure_mutable();
240+
self.balanced_ensure_mutable();
241241

242242
self.as_inner().clear()
243243
}
@@ -248,7 +248,7 @@ impl Dictionary {
248248
///
249249
/// _Godot equivalent: `dict[key] = value`_
250250
pub fn set<K: ToGodot, V: ToGodot>(&mut self, key: K, value: V) {
251-
self.debug_ensure_mutable();
251+
self.balanced_ensure_mutable();
252252

253253
let key = key.to_variant();
254254

@@ -263,7 +263,7 @@ impl Dictionary {
263263
/// If you don't need the previous value, use [`set()`][Self::set] instead.
264264
#[must_use]
265265
pub fn insert<K: ToGodot, V: ToGodot>(&mut self, key: K, value: V) -> Option<Variant> {
266-
self.debug_ensure_mutable();
266+
self.balanced_ensure_mutable();
267267

268268
let key = key.to_variant();
269269
let old_value = self.get(key.clone());
@@ -277,7 +277,7 @@ impl Dictionary {
277277
/// _Godot equivalent: `erase`_
278278
#[doc(alias = "erase")]
279279
pub fn remove<K: ToGodot>(&mut self, key: K) -> Option<Variant> {
280-
self.debug_ensure_mutable();
280+
self.balanced_ensure_mutable();
281281

282282
let key = key.to_variant();
283283
let old_value = self.get(key.clone());
@@ -318,7 +318,7 @@ impl Dictionary {
318318
/// _Godot equivalent: `merge`_
319319
#[doc(alias = "merge")]
320320
pub fn extend_dictionary(&mut self, other: &Self, overwrite: bool) {
321-
self.debug_ensure_mutable();
321+
self.balanced_ensure_mutable();
322322

323323
self.as_inner().merge(other, overwrite)
324324
}
@@ -408,10 +408,10 @@ impl Dictionary {
408408

409409
/// Best-effort mutability check.
410410
///
411-
/// # Panics
412-
/// In Debug mode, if the array is marked as read-only.
413-
fn debug_ensure_mutable(&self) {
414-
debug_assert!(
411+
/// # Panics (safeguards-balanced)
412+
/// If the dictionary is marked as read-only.
413+
fn balanced_ensure_mutable(&self) {
414+
sys::balanced_assert!(
415415
!self.is_read_only(),
416416
"mutating operation on read-only dictionary"
417417
);

0 commit comments

Comments
 (0)