Merge pull request #1406 from godot-rust/qol/opt-constrained-types

Add `GodotImmutable` to constrain `#[opt(default)]` types

Author: Bromeon

godot-core/src/meta/mod.rs

@@ -76,7 +76,7 @@ pub use signature::trace;
 #[doc(hidden)]
 pub use signature::*;
 pub use signed_range::{wrapped, SignedRange};
-pub use traits::{ArrayElement, GodotType, PackedArrayElement};
+pub use traits::{ArrayElement, GodotImmutable, GodotType, PackedArrayElement};
 pub use uniform_object_deref::UniformObjectDeref;
 
 // Public due to signals emit() needing it. Should be made pub(crate) again if that changes.

godot-core/src/meta/traits.rs

@@ -236,3 +236,88 @@ pub(crate) fn element_godot_type_name<T: ArrayElement>() -> String {
 // pub(crate)  fn element_godot_type_name<T: ArrayElement>() -> String {
 //     <T::Via as GodotType>::godot_type_name()
 // }
+
+// ----------------------------------------------------------------------------------------------------------------------------------------------
+
+/// Implemented for types that can be used as immutable default parameters in `#[func]` methods.
+///
+/// This trait ensures that default parameter values cannot be mutated by callers, preventing the Python "mutable default argument" problem
+/// where a single default value is shared across multiple calls.
+///
+/// Post-processes the default value in some cases, e.g. makes `Array<T>` read-only via `into_read_only()`.
+///
+/// At the moment, this trait is conservatively implemented for types where immutability can be statically guaranteed.
+/// Depending on usage, the API might be expanded in the future to allow defaults whose immutability is only determined
+/// at runtime (e.g. untyped arrays/dictionaries where all element types are immutable).
+///
+/// # Safety
+/// Allows to use the implementors in a limited `Sync` context. Implementing this trait asserts that `Self` is either:
+/// - `Copy`, i.e. each instance is truly independent.
+/// - Thread-safe in the sense that `clone()` is thread-safe. Individual clones must not offer a way to mutate the value or cause race conditions.
+#[diagnostic::on_unimplemented(
+    message = "#[opt(default = ...)] only supports a set of truly immutable types",
+    label = "this type is not immutable and thus not eligible for a default value"
+)]
+pub unsafe trait GodotImmutable: GodotConvert + Sized {
+    fn into_runtime_immutable(self) -> Self {
+        self
+    }
+}
+
+mod godot_immutable_impls {
+    use super::GodotImmutable;
+    use crate::builtin::*;
+    use crate::meta::ArrayElement;
+
+    unsafe impl GodotImmutable for bool {}
+    unsafe impl GodotImmutable for i8 {}
+    unsafe impl GodotImmutable for u8 {}
+    unsafe impl GodotImmutable for i16 {}
+    unsafe impl GodotImmutable for u16 {}
+    unsafe impl GodotImmutable for i32 {}
+    unsafe impl GodotImmutable for u32 {}
+    unsafe impl GodotImmutable for i64 {}
+    unsafe impl GodotImmutable for f32 {}
+    unsafe impl GodotImmutable for f64 {}
+
+    // No NodePath, Callable, Signal, Rid, Variant.
+    unsafe impl GodotImmutable for Aabb {}
+    unsafe impl GodotImmutable for Basis {}
+    unsafe impl GodotImmutable for Color {}
+    unsafe impl GodotImmutable for GString {}
+    unsafe impl GodotImmutable for Plane {}
+    unsafe impl GodotImmutable for Projection {}
+    unsafe impl GodotImmutable for Quaternion {}
+    unsafe impl GodotImmutable for Rect2 {}
+    unsafe impl GodotImmutable for Rect2i {}
+    unsafe impl GodotImmutable for StringName {}
+    unsafe impl GodotImmutable for Transform2D {}
+    unsafe impl GodotImmutable for Transform3D {}
+    unsafe impl GodotImmutable for Vector2 {}
+    unsafe impl GodotImmutable for Vector2i {}
+    unsafe impl GodotImmutable for Vector3 {}
+    unsafe impl GodotImmutable for Vector3i {}
+    unsafe impl GodotImmutable for Vector4 {}
+    unsafe impl GodotImmutable for Vector4i {}
+
+    unsafe impl GodotImmutable for PackedByteArray {}
+    unsafe impl GodotImmutable for PackedColorArray {}
+    unsafe impl GodotImmutable for PackedFloat32Array {}
+    unsafe impl GodotImmutable for PackedFloat64Array {}
+    unsafe impl GodotImmutable for PackedInt32Array {}
+    unsafe impl GodotImmutable for PackedInt64Array {}
+    unsafe impl GodotImmutable for PackedStringArray {}
+    unsafe impl GodotImmutable for PackedVector2Array {}
+    unsafe impl GodotImmutable for PackedVector3Array {}
+    #[cfg(since_api = "4.3")]
+    unsafe impl GodotImmutable for PackedVector4Array {}
+
+    unsafe impl<T> GodotImmutable for Array<T>
+    where
+        T: GodotImmutable + ArrayElement,
+    {
+        fn into_runtime_immutable(self) -> Self {
+            self.into_read_only()
+        }
+    }
+}

godot-core/src/private.rs

@@ -238,6 +238,25 @@ pub fn is_class_runtime(is_tool: bool) -> bool {
     global_config.tool_only_in_editor
 }
 
+/// Converts a default parameter value to a runtime-immutable `Variant`.
+///
+/// This function is used internally by the `#[opt(default)]` attribute to:
+/// 1. Convert the value using `AsArg` trait for argument conversions (e.g. `"str"` for `AsArg<GString>`).
+/// 2. Apply immutability transformation.
+/// 3. Convert to `Variant` for Godot's storage.
+pub fn opt_default_value<T>(value: impl crate::meta::AsArg<T>) -> crate::builtin::Variant
+where
+    T: crate::meta::GodotImmutable + crate::meta::ToGodot + Clone,
+{
+    // We currently need cow_into_owned() to create an owned value for the immutability transform. This may be revisited once `#[opt]`
+    // supports more types (e.g. `Gd<RefCounted>`, where `cow_into_owned()` would increment ref-counts).
+
+    let value = crate::meta::AsArg::<T>::into_arg(value);
+    let value = value.cow_into_owned();
+    let value = <T as crate::meta::GodotImmutable>::into_runtime_immutable(value);
+    crate::builtin::Variant::from(value)
+}
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Panic *hook* management
 

godot-macros/src/class/data_models/func.rs

@@ -217,9 +217,7 @@ fn make_default_argument_vec(
         .zip(optional_param_types)
         .map(|(value, param_type)| {
             quote! {
-                ::godot::builtin::Variant::from(
-                    ::godot::meta::AsArg::<#param_type>::into_arg(#value)
-                )
+                ::godot::private::opt_default_value::<#param_type>(#value)
             }
         });
 

itest/rust/src/register_tests/func_test.rs

@@ -66,6 +66,15 @@ impl FuncObj {
         varray![required, string, integer]
     }
 
+    #[func]
+    fn method_with_immutable_array_default(
+        &self,
+        #[opt(default = &array![1, 2, 3])] arr: Array<i64>,
+    ) -> Array<i64> {
+        arr
+    }
+
+    /* For now, Gd<T> types cannot be used as default parameters due to immutability requirement.
     #[func]
     fn static_with_defaults(
         #[opt(default = &RefCounted::new_gd())] mut required: Gd<RefCounted>,
@@ -79,6 +88,7 @@ impl FuncObj {
         required.set_meta("nullable_id", &id.to_variant());
         required
     }
+    */
 }
 
 impl FuncObj {
@@ -345,6 +355,7 @@ fn func_default_parameters() {
     let c = obj.call("method_with_defaults", vslice![2, "Another string", 456]);
     assert_eq!(c.to::<VariantArray>(), varray![2, "Another string", 456]);
 
+    /* For now, Gd<T> defaults are disabled due to immutability.
     // Test that object is passed through, and that Option<Gd> with default Gd::null_arg() works.
     let first = RefCounted::new_gd();
     let d = obj
@@ -360,8 +371,10 @@ fn func_default_parameters() {
         .to::<Gd<RefCounted>>();
     assert_eq!(e.instance_id(), first.instance_id());
     assert_eq!(e.get_meta("nullable_id"), second.instance_id().to_variant());
+    */
 }
 
+/* For now, Gd<T> defaults are disabled due to immutability.
 #[itest]
 fn func_defaults_re_evaluate_expr() {
     // ClassDb::class_call_static() added in Godot 4.4, but non-static dispatch works even before.
@@ -386,8 +399,23 @@ fn func_defaults_re_evaluate_expr() {
         "#[opt = EXPR] should create evaluate EXPR on each call"
     );
 }
+*/
 
-// No test for Gd::from_object(), as that simply moves the existing object without running user code.
+#[itest]
+fn func_immutable_defaults() {
+    let mut obj = FuncObj::new_gd();
+
+    // Test Array<T> default parameter.
+    let arr = obj
+        .call("method_with_immutable_array_default", &[])
+        .to::<Array<i64>>();
+    assert_eq!(arr, array![1, 2, 3]);
+
+    assert!(
+        arr.is_read_only(),
+        "GodotImmutable trait did its job to make array read-only"
+    );
+}
 
 #[itest]
 fn cfg_doesnt_interfere_with_valid_method_impls() {
@@ -431,6 +459,8 @@ fn cfg_removes_or_keeps_signals() {
     assert!(!class_has_signal::<GdSelfObj>("cfg_removes_signal"));
 }
 
+// No test for Gd::from_object(), as that simply moves the existing object without running user code.
+
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Helpers