Skip to content

Commit 53043c9

Browse files
authored
Merge pull request #1395 from godot-rust/qol/safeguard-assertions
Start phasing out `#[cfg(debug_assertions)]` in favor of safeguards
2 parents 9182513 + 5e75d1c commit 53043c9

Some content is hidden

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

43 files changed

+424
-339
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: 57 additions & 72 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
66
*/
77

8+
use std::thread::ThreadId;
89
use std::{fmt, ptr};
910

1011
use godot_ffi as sys;
@@ -162,16 +163,7 @@ impl Callable {
162163
F: 'static + FnMut(&[&Variant]) -> R,
163164
S: meta::AsArg<GString>,
164165
{
165-
#[cfg(debug_assertions)]
166-
meta::arg_into_owned!(name);
167-
168-
Self::from_fn_wrapper::<F, R>(FnWrapper {
169-
rust_function,
170-
#[cfg(debug_assertions)]
171-
name,
172-
thread_id: Some(std::thread::current().id()),
173-
linked_obj_id: None,
174-
})
166+
Self::from_fn_wrapper(name, rust_function, Some(std::thread::current().id()), None)
175167
}
176168

177169
/// Creates a new callable linked to the given object from **single-threaded** Rust function or closure.
@@ -189,16 +181,12 @@ impl Callable {
189181
F: 'static + FnMut(&[&Variant]) -> R,
190182
S: meta::AsArg<GString>,
191183
{
192-
#[cfg(debug_assertions)]
193-
meta::arg_into_owned!(name);
194-
195-
Self::from_fn_wrapper::<F, R>(FnWrapper {
196-
rust_function,
197-
#[cfg(debug_assertions)]
184+
Self::from_fn_wrapper(
198185
name,
199-
thread_id: Some(std::thread::current().id()),
200-
linked_obj_id: Some(linked_object.instance_id()),
201-
})
186+
rust_function,
187+
Some(std::thread::current().id()),
188+
Some(linked_object.instance_id()),
189+
)
202190
}
203191

204192
/// 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 {
258246
F: FnMut(&[&Variant]) -> Variant,
259247
Fc: FnOnce(&Callable) -> R,
260248
{
261-
#[cfg(debug_assertions)]
262-
meta::arg_into_owned!(name);
263-
264-
let callable = Self::from_fn_wrapper::<F, Variant>(FnWrapper {
265-
rust_function,
266-
#[cfg(debug_assertions)]
267-
name,
268-
thread_id: Some(std::thread::current().id()),
269-
linked_obj_id: None,
270-
});
249+
let callable =
250+
Self::from_fn_wrapper(name, rust_function, Some(std::thread::current().id()), None);
271251

272252
callable_usage(&callable)
273253
}
@@ -298,16 +278,7 @@ impl Callable {
298278
F: 'static + Send + Sync + FnMut(&[&Variant]) -> R,
299279
S: meta::AsArg<GString>,
300280
{
301-
#[cfg(debug_assertions)]
302-
meta::arg_into_owned!(name);
303-
304-
Self::from_fn_wrapper::<F, R>(FnWrapper {
305-
rust_function,
306-
#[cfg(debug_assertions)]
307-
name,
308-
thread_id: None,
309-
linked_obj_id: None,
310-
})
281+
Self::from_fn_wrapper(name, rust_function, None, None)
311282
}
312283

313284
/// Create a highly configurable callable from Rust.
@@ -334,21 +305,34 @@ impl Callable {
334305
Self::from_custom_info(info)
335306
}
336307

337-
fn from_fn_wrapper<F, R>(inner: FnWrapper<F>) -> Self
308+
fn from_fn_wrapper<F, R, S>(
309+
_name: S,
310+
rust_function: F,
311+
thread_id: Option<ThreadId>,
312+
linked_object_id: Option<InstanceId>,
313+
) -> Self
338314
where
339315
F: FnMut(&[&Variant]) -> R,
340316
R: ToGodot,
317+
S: meta::AsArg<GString>,
341318
{
342-
let object_id = inner.linked_object_id();
319+
let wrapper = FnWrapper {
320+
rust_function,
321+
#[cfg(safeguards_balanced)]
322+
name: { _name.into_arg().cow_into_owned() },
323+
thread_id,
324+
linked_object_id,
325+
};
343326

344-
let userdata = CallableUserdata { inner };
327+
let object_id = wrapper.linked_object_id();
328+
let userdata = CallableUserdata { inner: wrapper };
345329

346330
let info = CallableCustomInfo {
347331
object_id,
348332
callable_userdata: Box::into_raw(Box::new(userdata)) as *mut std::ffi::c_void,
349333
call_func: Some(rust_callable_call_fn::<F, R>),
350334
free_func: Some(rust_callable_destroy::<FnWrapper<F>>),
351-
#[cfg(debug_assertions)]
335+
#[cfg(safeguards_balanced)]
352336
to_string_func: Some(rust_callable_to_string_named::<F>),
353337
is_valid_func: Some(rust_callable_is_valid),
354338
..Self::default_callable_custom_info()
@@ -617,21 +601,34 @@ mod custom_callable {
617601

618602
pub(crate) struct FnWrapper<F> {
619603
pub(super) rust_function: F,
620-
#[cfg(debug_assertions)]
604+
#[cfg(safeguards_balanced)]
621605
pub(super) name: GString,
622606

623607
/// `None` if the callable is multi-threaded ([`Callable::from_sync_fn`]).
624608
pub(super) thread_id: Option<ThreadId>,
625609
/// `None` if callable is not linked with any object.
626-
pub(super) linked_obj_id: Option<InstanceId>,
610+
pub(super) linked_object_id: Option<InstanceId>,
627611
}
628612

629613
impl<F> FnWrapper<F> {
630614
pub(crate) fn linked_object_id(&self) -> GDObjectInstanceID {
631-
self.linked_obj_id.map(InstanceId::to_u64).unwrap_or(0)
615+
self.linked_object_id.map(InstanceId::to_u64).unwrap_or(0)
632616
}
633617
}
634618

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+
635632
/// Represents a custom callable object defined in Rust.
636633
///
637634
/// This trait has a single method, `invoke`, which is called upon invocation.
@@ -668,14 +665,11 @@ mod custom_callable {
668665
) {
669666
let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize);
670667

671-
#[cfg(debug_assertions)]
672-
let name = &{
668+
let name = name_or_optimized! {
673669
let c: &C = CallableUserdata::inner_from_raw(callable_userdata);
674670
c.to_string()
675671
};
676-
#[cfg(not(debug_assertions))]
677-
let name = "<optimized out>";
678-
let ctx = meta::CallContext::custom_callable(name);
672+
let ctx = meta::CallContext::custom_callable(&name);
679673

680674
crate::private::handle_fallible_varcall(&ctx, &mut *r_error, move || {
681675
// Get the RustCallable again inside closure so it doesn't have to be UnwindSafe.
@@ -698,34 +692,25 @@ mod custom_callable {
698692
{
699693
let arg_refs: &[&Variant] = Variant::borrow_ref_slice(p_args, p_argument_count as usize);
700694

701-
#[cfg(debug_assertions)]
702-
let name = &{
695+
let name = name_or_optimized! {
703696
let w: &FnWrapper<F> = CallableUserdata::inner_from_raw(callable_userdata);
704697
w.name.to_string()
705698
};
706-
#[cfg(not(debug_assertions))]
707-
let name = "<optimized out>";
708-
let ctx = meta::CallContext::custom_callable(name);
699+
let ctx = meta::CallContext::custom_callable(&name);
709700

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

714-
if w.thread_id
715-
.is_some_and(|tid| tid != std::thread::current().id())
716-
{
717-
#[cfg(debug_assertions)]
718-
let name = &w.name;
719-
#[cfg(not(debug_assertions))]
720-
let name = "<optimized out>";
721-
// NOTE: this panic is currently not propagated to the caller, but results in an error message and Nil return.
722-
// See comments in itest callable_call() for details.
723-
panic!(
724-
"Callable '{}' created with from_fn() must be called from the same thread it was created in.\n\
725-
If you need to call it from any thread, use from_sync_fn() instead (requires `experimental-threads` feature).",
726-
name
727-
);
728-
}
705+
let name = name_or_optimized!(w.name.to_string());
706+
707+
// NOTE: this panic is currently not propagated to the caller, but results in an error message and Nil return.
708+
// See comments in itest callable_call() for details.
709+
sys::balanced_assert!(
710+
w.thread_id.is_none() || w.thread_id == Some(std::thread::current().id()),
711+
"Callable '{name}' created with from_fn() must be called from the same thread it was created in.\n\
712+
If you need to call it from any thread, use from_sync_fn() instead (requires `experimental-threads` feature)."
713+
);
729714

730715
let result = (w.rust_function)(arg_refs).to_variant();
731716
meta::varcall_return_checked(Ok(result), r_return, r_error);
@@ -769,7 +754,7 @@ mod custom_callable {
769754
*r_is_valid = sys::conv::SYS_TRUE;
770755
}
771756

772-
#[cfg(debug_assertions)]
757+
#[cfg(safeguards_balanced)]
773758
pub unsafe extern "C" fn rust_callable_to_string_named<F>(
774759
callable_userdata: *mut std::ffi::c_void,
775760
r_is_valid: *mut sys::GDExtensionBool,

0 commit comments

Comments
 (0)