Skip to content

Commit f267dce

Browse files
committed
Make #[cfg] model for safeguards easier to understand
Use simple #[cfg]s: `safeguards_strict` + `safeguards_balanced`. This implies "at least this safety level". No more indirect `*_at_least = "string"` conditions and extra negation.
1 parent 3bffdf6 commit f267dce

File tree

16 files changed

+77
-71
lines changed

16 files changed

+77
-71
lines changed

godot-bindings/src/lib.rs

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -269,8 +269,13 @@ pub fn since_api(major_minor: &str) -> bool {
269269
}
270270

271271
pub fn emit_safeguard_levels() {
272-
let safeguard_modes = ["disengaged", "balanced", "strict"];
272+
// Determine safeguard level:
273+
// - Disengaged (0): no runtime checks, maximum performance
274+
// - Balanced (1): essential safety checks, good balance (default for release)
275+
// - Strict (2): all checks including expensive ones (default for debug)
273276
let mut safeguards_level = if cfg!(debug_assertions) { 2 } else { 1 };
277+
278+
// Features can override the default level
274279
#[cfg(debug_assertions)]
275280
if cfg!(feature = "safeguards-dev-balanced") {
276281
safeguards_level = 1;
@@ -280,10 +285,15 @@ pub fn emit_safeguard_levels() {
280285
safeguards_level = 0;
281286
}
282287

283-
for mode in safeguard_modes.iter() {
284-
println!(r#"cargo:rustc-check-cfg=cfg(safeguards_at_least, values("{mode}"))"#);
288+
// Emit cfg checks for rustc
289+
println!(r#"cargo:rustc-check-cfg=cfg(safeguards_balanced)"#);
290+
println!(r#"cargo:rustc-check-cfg=cfg(safeguards_strict)"#);
291+
292+
// Emit cfgs cumulatively: strict builds get both balanced and strict
293+
if safeguards_level >= 1 {
294+
println!(r#"cargo:rustc-cfg=safeguards_balanced"#);
285295
}
286-
for mode in safeguard_modes.iter().take(safeguards_level + 1) {
287-
println!(r#"cargo:rustc-cfg=safeguards_at_least="{mode}""#);
296+
if safeguards_level >= 2 {
297+
println!(r#"cargo:rustc-cfg=safeguards_strict"#);
288298
}
289299
}

godot-codegen/src/generator/classes.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
195195
fn __checked_id(&self) -> Option<crate::obj::InstanceId> {
196196
// SAFETY: only Option due to layout-compatibility with RawGd<T>; it is always Some because stored in Gd<T> which is non-null.
197197
let rtti = unsafe { self.rtti.as_ref().unwrap_unchecked() };
198-
#[cfg(safeguards_at_least = "strict")]
198+
#[cfg(safeguards_strict)]
199199
rtti.check_type::<Self>();
200200
Some(rtti.instance_id())
201201
}

godot-codegen/src/generator/native_structures.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,7 @@ fn make_native_structure_field_and_accessor(
219219

220220
let obj = #snake_field_name.upcast();
221221

222-
#[cfg(safeguards_at_least = "balanced")]
222+
#[cfg(safeguards_balanced)]
223223
assert!(obj.is_instance_valid(), "provided node is dead");
224224

225225
let id = obj.instance_id().to_u64();

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -968,7 +968,7 @@ impl<T: ArrayElement> Array<T> {
968968
}
969969

970970
/// Validates that all elements in this array can be converted to integers of type `T`.
971-
#[cfg(safeguards_at_least = "strict")]
971+
#[cfg(safeguards_strict)]
972972
pub(crate) fn debug_validate_int_elements(&self) -> Result<(), ConvertError> {
973973
// SAFETY: every element is internally represented as Variant.
974974
let canonical_array = unsafe { self.assume_type_ref::<Variant>() };
@@ -990,7 +990,7 @@ impl<T: ArrayElement> Array<T> {
990990
}
991991

992992
// No-op in Release. Avoids O(n) conversion checks, but still panics on access.
993-
#[cfg(not(safeguards_at_least = "strict"))]
993+
#[cfg(not(safeguards_strict))]
994994
pub(crate) fn debug_validate_int_elements(&self) -> Result<(), ConvertError> {
995995
Ok(())
996996
}
@@ -1233,7 +1233,7 @@ impl<T: ArrayElement> Clone for Array<T> {
12331233
let copy = unsafe { self.clone_unchecked() };
12341234

12351235
// Double-check copy's runtime type in Debug mode.
1236-
if cfg!(safeguards_at_least = "strict") {
1236+
if cfg!(safeguards_strict) {
12371237
copy.with_checked_type()
12381238
.expect("copied array should have same type as original array")
12391239
} else {

godot-core/src/classes/class_runtime.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,10 @@
88
//! Runtime checks and inspection of Godot classes.
99
1010
use crate::builtin::{GString, StringName, Variant, VariantType};
11-
#[cfg(safeguards_at_least = "strict")]
11+
#[cfg(safeguards_strict)]
1212
use crate::classes::{ClassDb, Object};
1313
use crate::meta::CallContext;
14-
#[cfg(safeguards_at_least = "strict")]
14+
#[cfg(safeguards_strict)]
1515
use crate::meta::ClassId;
1616
use crate::obj::{bounds, Bounds, Gd, GodotClass, InstanceId, RawGd, Singleton};
1717
use crate::sys;
@@ -191,7 +191,7 @@ where
191191
Gd::<T>::from_obj_sys(object_ptr)
192192
}
193193

194-
#[cfg(safeguards_at_least = "balanced")]
194+
#[cfg(safeguards_balanced)]
195195
pub(crate) fn ensure_object_alive(
196196
instance_id: InstanceId,
197197
old_object_ptr: sys::GDExtensionObjectPtr,
@@ -212,7 +212,7 @@ pub(crate) fn ensure_object_alive(
212212
);
213213
}
214214

215-
#[cfg(safeguards_at_least = "strict")]
215+
#[cfg(safeguards_strict)]
216216
pub(crate) fn ensure_object_inherits(derived: ClassId, base: ClassId, instance_id: InstanceId) {
217217
if derived == base
218218
|| base == Object::class_id() // for Object base, anything inherits by definition
@@ -227,7 +227,7 @@ pub(crate) fn ensure_object_inherits(derived: ClassId, base: ClassId, instance_i
227227
)
228228
}
229229

230-
#[cfg(safeguards_at_least = "strict")]
230+
#[cfg(safeguards_strict)]
231231
pub(crate) fn ensure_binding_not_null<T>(binding: sys::GDExtensionClassInstancePtr)
232232
where
233233
T: GodotClass + Bounds<Declarer = bounds::DeclUser>,
@@ -255,7 +255,7 @@ where
255255
// Implementation of this file
256256

257257
/// Checks if `derived` inherits from `base`, using a cache for _successful_ queries.
258-
#[cfg(safeguards_at_least = "strict")]
258+
#[cfg(safeguards_strict)]
259259
fn is_derived_base_cached(derived: ClassId, base: ClassId) -> bool {
260260
use std::collections::HashSet;
261261

godot-core/src/init/mod.rs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -535,14 +535,12 @@ unsafe fn ensure_godot_features_compatible() {
535535
out!("Check Godot precision setting...");
536536

537537
#[cfg(feature = "debug-log")] // Display safeguards level in debug log.
538-
let safeguards_level = if cfg!(safeguards_at_least = "strict") {
538+
let safeguards_level = if cfg!(safeguards_strict) {
539539
"strict"
540-
} else if cfg!(safeguards_at_least = "balanced") {
540+
} else if cfg!(safeguards_balanced) {
541541
"balanced"
542-
} else if cfg!(safeguards_at_least = "disengaged") {
543-
"disengaged"
544542
} else {
545-
"unknown"
543+
"disengaged"
546544
};
547545
out!("Safeguards: {safeguards_level}");
548546

godot-core/src/meta/error/convert_error.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ pub(crate) enum FromGodotError {
189189
},
190190

191191
/// Special case of `BadArrayType` where a custom int type such as `i8` cannot hold a dynamic `i64` value.
192-
#[cfg(safeguards_at_least = "strict")]
192+
#[cfg(safeguards_strict)]
193193
BadArrayTypeInt {
194194
expected_int_type: &'static str,
195195
value: i64,
@@ -234,7 +234,7 @@ impl fmt::Display for FromGodotError {
234234

235235
write!(f, "expected array of type {exp_class}, got {act_class}")
236236
}
237-
#[cfg(safeguards_at_least = "strict")]
237+
#[cfg(safeguards_strict)]
238238
Self::BadArrayTypeInt {
239239
expected_int_type,
240240
value,

godot-core/src/meta/signature.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ impl<Params: OutParamTuple, Ret: FromGodot> Signature<Params, Ret> {
145145

146146
// Note: varcalls are not safe from failing, if they happen through an object pointer -> validity check necessary.
147147
// paranoid since we already check the validity in check_rtti, this is unlikely to happen.
148-
#[cfg(safeguards_at_least = "strict")]
148+
#[cfg(safeguards_strict)]
149149
if let Some(instance_id) = maybe_instance_id {
150150
crate::classes::ensure_object_alive(instance_id, object_ptr, &call_ctx);
151151
}
@@ -307,7 +307,7 @@ impl<Params: OutParamTuple, Ret: FromGodot> Signature<Params, Ret> {
307307
// $crate::out!("out_class_ptrcall: {call_ctx}");
308308

309309
// paranoid since we already check the validity in check_rtti, this is unlikely to happen.
310-
#[cfg(safeguards_at_least = "strict")]
310+
#[cfg(safeguards_strict)]
311311
if let Some(instance_id) = maybe_instance_id {
312312
crate::classes::ensure_object_alive(instance_id, object_ptr, &call_ctx);
313313
}

godot-core/src/obj/base.rs

Lines changed: 18 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,14 @@
55
* file, You can obtain one at https://mozilla.org/MPL/2.0/.
66
*/
77

8-
#[cfg(safeguards_at_least = "strict")]
8+
#[cfg(safeguards_strict)]
99
use std::cell::Cell;
1010
use std::cell::RefCell;
1111
use std::collections::hash_map::Entry;
1212
use std::collections::HashMap;
1313
use std::fmt::{Debug, Display, Formatter, Result as FmtResult};
1414
use std::mem::ManuallyDrop;
15+
#[cfg(safeguards_strict)]
1516
use std::rc::Rc;
1617

1718
use crate::builtin::Callable;
@@ -27,7 +28,7 @@ thread_local! {
2728
}
2829

2930
/// Represents the initialization state of a `Base<T>` object.
30-
#[cfg(safeguards_at_least = "strict")]
31+
#[cfg(safeguards_strict)]
3132
#[derive(Debug, Copy, Clone, PartialEq, Eq)]
3233
enum InitState {
3334
/// Object is being constructed (inside `I*::init()` or `Gd::from_init_fn()`).
@@ -38,14 +39,14 @@ enum InitState {
3839
Script,
3940
}
4041

41-
#[cfg(safeguards_at_least = "strict")]
42+
#[cfg(safeguards_strict)]
4243
macro_rules! base_from_obj {
4344
($obj:expr, $state:expr) => {
4445
Base::from_obj($obj, $state)
4546
};
4647
}
4748

48-
#[cfg(not(safeguards_at_least = "strict"))]
49+
#[cfg(not(safeguards_strict))]
4950
macro_rules! base_from_obj {
5051
($obj:expr, $state:expr) => {
5152
Base::from_obj($obj)
@@ -82,7 +83,7 @@ pub struct Base<T: GodotClass> {
8283
/// Tracks the initialization state of this `Base<T>` in Debug mode.
8384
///
8485
/// Rc allows to "copy-construct" the base from an existing one, while still affecting the user-instance through the original `Base<T>`.
85-
#[cfg(safeguards_at_least = "strict")]
86+
#[cfg(safeguards_strict)]
8687
init_state: Rc<Cell<InitState>>,
8788
}
8889

@@ -95,14 +96,14 @@ impl<T: GodotClass> Base<T> {
9596
/// `base` must be alive at the time of invocation, i.e. user `init()` (which could technically destroy it) must not have run yet.
9697
/// If `base` is destroyed while the returned `Base<T>` is in use, that constitutes a logic error, not a safety issue.
9798
pub(crate) unsafe fn from_base(base: &Base<T>) -> Base<T> {
98-
#[cfg(safeguards_at_least = "strict")]
99+
#[cfg(safeguards_strict)]
99100
assert!(base.obj.is_instance_valid());
100101

101102
let obj = Gd::from_obj_sys_weak(base.obj.obj_sys());
102103

103104
Self {
104105
obj: ManuallyDrop::new(obj),
105-
#[cfg(safeguards_at_least = "strict")]
106+
#[cfg(safeguards_strict)]
106107
init_state: Rc::clone(&base.init_state),
107108
}
108109
}
@@ -115,7 +116,7 @@ impl<T: GodotClass> Base<T> {
115116
/// `gd` must be alive at the time of invocation. If it is destroyed while the returned `Base<T>` is in use, that constitutes a logic
116117
/// error, not a safety issue.
117118
pub(crate) unsafe fn from_script_gd(gd: &Gd<T>) -> Self {
118-
#[cfg(safeguards_at_least = "strict")]
119+
#[cfg(safeguards_strict)]
119120
assert!(gd.is_instance_valid());
120121

121122
let obj = Gd::from_obj_sys_weak(gd.obj_sys());
@@ -143,15 +144,15 @@ impl<T: GodotClass> Base<T> {
143144
base_from_obj!(obj, InitState::ObjectConstructing)
144145
}
145146

146-
#[cfg(safeguards_at_least = "strict")]
147+
#[cfg(safeguards_strict)]
147148
fn from_obj(obj: Gd<T>, init_state: InitState) -> Self {
148149
Self {
149150
obj: ManuallyDrop::new(obj),
150151
init_state: Rc::new(Cell::new(init_state)),
151152
}
152153
}
153154

154-
#[cfg(not(safeguards_at_least = "strict"))]
155+
#[cfg(not(safeguards_strict))]
155156
fn from_obj(obj: Gd<T>) -> Self {
156157
Self {
157158
obj: ManuallyDrop::new(obj),
@@ -178,7 +179,7 @@ impl<T: GodotClass> Base<T> {
178179
/// # Panics (Debug)
179180
/// If called outside an initialization function, or for ref-counted objects on a non-main thread.
180181
pub fn to_init_gd(&self) -> Gd<T> {
181-
#[cfg(safeguards_at_least = "strict")] // debug_assert! still checks existence of symbols.
182+
#[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols.
182183
assert!(
183184
self.is_initializing(),
184185
"Base::to_init_gd() can only be called during object initialization, inside I*::init() or Gd::from_init_fn()"
@@ -250,7 +251,7 @@ impl<T: GodotClass> Base<T> {
250251

251252
/// Finalizes the initialization of this `Base<T>` and returns whether
252253
pub(crate) fn mark_initialized(&mut self) {
253-
#[cfg(safeguards_at_least = "strict")]
254+
#[cfg(safeguards_strict)]
254255
{
255256
assert_eq!(
256257
self.init_state.get(),
@@ -267,7 +268,7 @@ impl<T: GodotClass> Base<T> {
267268
/// Returns a [`Gd`] referencing the base object, assuming the derived object is fully constructed.
268269
#[doc(hidden)]
269270
pub fn __fully_constructed_gd(&self) -> Gd<T> {
270-
#[cfg(safeguards_at_least = "strict")] // debug_assert! still checks existence of symbols.
271+
#[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols.
271272
assert!(
272273
!self.is_initializing(),
273274
"WithBaseField::to_gd(), base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()"
@@ -298,7 +299,7 @@ impl<T: GodotClass> Base<T> {
298299

299300
/// Returns a passive reference to the base object, for use in script contexts only.
300301
pub(crate) fn to_script_passive(&self) -> PassiveGd<T> {
301-
#[cfg(safeguards_at_least = "strict")]
302+
#[cfg(safeguards_strict)]
302303
assert_eq!(
303304
self.init_state.get(),
304305
InitState::Script,
@@ -310,15 +311,15 @@ impl<T: GodotClass> Base<T> {
310311
}
311312

312313
/// Returns `true` if this `Base<T>` is currently in the initializing state.
313-
#[cfg(safeguards_at_least = "strict")]
314+
#[cfg(safeguards_strict)]
314315
fn is_initializing(&self) -> bool {
315316
self.init_state.get() == InitState::ObjectConstructing
316317
}
317318

318319
/// Returns a [`Gd`] referencing the base object, assuming the derived object is fully constructed.
319320
#[doc(hidden)]
320321
pub fn __constructed_gd(&self) -> Gd<T> {
321-
#[cfg(safeguards_at_least = "strict")] // debug_assert! still checks existence of symbols.
322+
#[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols.
322323
assert!(
323324
!self.is_initializing(),
324325
"WithBaseField::to_gd(), base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()"
@@ -335,7 +336,7 @@ impl<T: GodotClass> Base<T> {
335336
/// # Safety
336337
/// Caller must ensure that the underlying object remains valid for the entire lifetime of the returned `PassiveGd`.
337338
pub(crate) unsafe fn constructed_passive(&self) -> PassiveGd<T> {
338-
#[cfg(safeguards_at_least = "strict")] // debug_assert! still checks existence of symbols.
339+
#[cfg(safeguards_strict)] // debug_assert! still checks existence of symbols.
339340
assert!(
340341
!self.is_initializing(),
341342
"WithBaseField::base(), base_mut() can only be called on fully-constructed objects, after I*::init() or Gd::from_init_fn()"

godot-core/src/obj/casts.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,15 @@ impl<T: GodotClass, U: GodotClass> CastSuccess<T, U> {
4848
}
4949

5050
/// Access shared reference to destination, without consuming object.
51-
#[cfg(safeguards_at_least = "strict")]
51+
#[cfg(safeguards_strict)]
5252
pub fn as_dest_ref(&self) -> &RawGd<U> {
5353
self.check_validity();
5454
&self.dest
5555
}
5656

5757
/// Access exclusive reference to destination, without consuming object.
5858
pub fn as_dest_mut(&mut self) -> &mut RawGd<U> {
59-
#[cfg(safeguards_at_least = "strict")]
59+
#[cfg(safeguards_strict)]
6060
self.check_validity();
6161
&mut self.dest
6262
}
@@ -71,14 +71,14 @@ impl<T: GodotClass, U: GodotClass> CastSuccess<T, U> {
7171
self.dest.instance_id_unchecked(),
7272
"traded_source must point to the same object as the destination"
7373
);
74-
#[cfg(safeguards_at_least = "strict")]
74+
#[cfg(safeguards_strict)]
7575
self.check_validity();
7676

7777
std::mem::forget(traded_source);
7878
ManuallyDrop::into_inner(self.dest)
7979
}
8080

81-
#[cfg(safeguards_at_least = "strict")]
81+
#[cfg(safeguards_strict)]
8282
fn check_validity(&self) {
8383
assert!(self.dest.is_null() || self.dest.is_instance_valid());
8484
}

0 commit comments

Comments
 (0)