Merge pull request #906 from godot-rust/feature/pass-by-ref-backend

Pass-by-ref for non-`Copy` builtins (backend)

Author: Bromeon

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

@@ -12,7 +12,7 @@ use crate::builtin::*;
 use crate::meta::error::{ConvertError, FromGodotError, FromVariantError};
 use crate::meta::{
     ArrayElement, ArrayTypeInfo, FromGodot, GodotConvert, GodotFfiVariant, GodotType,
-    PropertyHintInfo, ToGodot,
+    PropertyHintInfo, RefArg, ToGodot,
 };
 use crate::registry::property::{Export, Var};
 use godot_ffi as sys;
@@ -1047,10 +1047,11 @@ impl<T: ArrayElement> Drop for Array<T> {
 impl<T: ArrayElement> GodotType for Array<T> {
     type Ffi = Self;
 
-    fn to_ffi(&self) -> Self::Ffi {
-        // SAFETY: we may pass type-transmuted arrays to FFI (e.g. Array<T> as Array<Variant>). This would fail the regular
-        // type-check in clone(), so we disable it. Type invariants are upheld by the "front-end" APIs.
-        unsafe { self.clone_unchecked() }
+    type ToFfi<'f> = RefArg<'f, Array<T>>
+    where Self: 'f;
+
+    fn to_ffi(&self) -> Self::ToFfi<'_> {
+        RefArg::new(self)
     }
 
     fn into_ffi(self) -> Self::Ffi {

godot-core/src/builtin/variant/impls.rs

@@ -9,7 +9,9 @@ use super::*;
 use crate::builtin::*;
 use crate::global;
 use crate::meta::error::{ConvertError, FromVariantError};
-use crate::meta::{ArrayElement, GodotFfiVariant, GodotType, PropertyHintInfo, PropertyInfo};
+use crate::meta::{
+    ArrayElement, GodotFfiVariant, GodotType, PropertyHintInfo, PropertyInfo, RefArg,
+};
 use godot_ffi as sys;
 // For godot-cpp, see https://github.com/godotengine/godot-cpp/blob/master/include/godot_cpp/core/type_info.hpp.
 
@@ -22,7 +24,15 @@ use godot_ffi as sys;
 //
 // Therefore, we can use `init` to indicate when it must be initialized in 4.0.
 macro_rules! impl_ffi_variant {
-    ($T:ty, $from_fn:ident, $to_fn:ident $(; $godot_type_name:ident)?) => {
+    (ref $T:ty, $from_fn:ident, $to_fn:ident $(; $GodotTy:ident)?) => {
+        impl_ffi_variant!(@impls by_ref; $T, $from_fn, $to_fn $(; $GodotTy)?);
+    };
+    ($T:ty, $from_fn:ident, $to_fn:ident $(; $GodotTy:ident)?) => {
+        impl_ffi_variant!(@impls by_val; $T, $from_fn, $to_fn $(; $GodotTy)?);
+    };
+
+    // Implementations
+    (@impls $by_ref_or_val:ident; $T:ty, $from_fn:ident, $to_fn:ident $(; $GodotTy:ident)?) => {
         impl GodotFfiVariant for $T {
             fn ffi_to_variant(&self) -> Variant {
                 let variant = unsafe {
@@ -58,10 +68,7 @@ macro_rules! impl_ffi_variant {
 
         impl GodotType for $T {
             type Ffi = Self;
-
-            fn to_ffi(&self) -> Self::Ffi {
-                self.clone()
-            }
+            impl_ffi_variant!(@assoc_to_ffi $by_ref_or_val);
 
             fn into_ffi(self) -> Self::Ffi {
                 self
@@ -71,7 +78,7 @@ macro_rules! impl_ffi_variant {
                 Ok(ffi)
             }
 
-            impl_ffi_variant!(@godot_type_name $T $(, $godot_type_name)?);
+            impl_ffi_variant!(@godot_type_name $T $(, $GodotTy)?);
         }
 
         impl ArrayElement for $T {}
@@ -88,6 +95,22 @@ macro_rules! impl_ffi_variant {
             stringify!($godot_type_name).into()
         }
     };
+
+    (@assoc_to_ffi by_ref) => {
+        type ToFfi<'a> =  RefArg<'a, Self>;
+
+        fn to_ffi(&self) -> Self::ToFfi<'_> {
+            RefArg::new(self)
+        }
+    };
+
+    (@assoc_to_ffi by_val) => {
+        type ToFfi<'a> = Self;
+
+        fn to_ffi(&self) -> Self::ToFfi<'_> {
+            self.clone()
+        }
+    };
 }
 
 // ----------------------------------------------------------------------------------------------------------------------------------------------
@@ -116,17 +139,17 @@ mod impls {
     impl_ffi_variant!(GString, string_to_variant, string_from_variant; String);
     impl_ffi_variant!(StringName, string_name_to_variant, string_name_from_variant);
     impl_ffi_variant!(NodePath, node_path_to_variant, node_path_from_variant);
-    impl_ffi_variant!(PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant);
-    impl_ffi_variant!(PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant);
-    impl_ffi_variant!(PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant);
-    impl_ffi_variant!(PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant);
-    impl_ffi_variant!(PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant);
-    impl_ffi_variant!(PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant);
-    impl_ffi_variant!(PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant);
-    impl_ffi_variant!(PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant);
+    impl_ffi_variant!(ref PackedByteArray, packed_byte_array_to_variant, packed_byte_array_from_variant);
+    impl_ffi_variant!(ref PackedInt32Array, packed_int32_array_to_variant, packed_int32_array_from_variant);
+    impl_ffi_variant!(ref PackedInt64Array, packed_int64_array_to_variant, packed_int64_array_from_variant);
+    impl_ffi_variant!(ref PackedFloat32Array, packed_float32_array_to_variant, packed_float32_array_from_variant);
+    impl_ffi_variant!(ref PackedFloat64Array, packed_float64_array_to_variant, packed_float64_array_from_variant);
+    impl_ffi_variant!(ref PackedStringArray, packed_string_array_to_variant, packed_string_array_from_variant);
+    impl_ffi_variant!(ref PackedVector2Array, packed_vector2_array_to_variant, packed_vector2_array_from_variant);
+    impl_ffi_variant!(ref PackedVector3Array, packed_vector3_array_to_variant, packed_vector3_array_from_variant);
     #[cfg(since_api = "4.3")]
-    impl_ffi_variant!(PackedVector4Array, packed_vector4_array_to_variant, packed_vector4_array_from_variant);
-    impl_ffi_variant!(PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant);
+    impl_ffi_variant!(ref PackedVector4Array, packed_vector4_array_to_variant, packed_vector4_array_from_variant);
+    impl_ffi_variant!(ref PackedColorArray, packed_color_array_to_variant, packed_color_array_from_variant);
     impl_ffi_variant!(Plane, plane_to_variant, plane_from_variant);
     impl_ffi_variant!(Projection, projection_to_variant, projection_from_variant);
     impl_ffi_variant!(Rid, rid_to_variant, rid_from_variant; RID);
@@ -135,7 +158,7 @@ mod impls {
     impl_ffi_variant!(Signal, signal_to_variant, signal_from_variant);
     impl_ffi_variant!(Transform2D, transform_2d_to_variant, transform_2d_from_variant);
     impl_ffi_variant!(Transform3D, transform_3d_to_variant, transform_3d_from_variant);
-    impl_ffi_variant!(Dictionary, dictionary_to_variant, dictionary_from_variant);
+    impl_ffi_variant!(ref Dictionary, dictionary_to_variant, dictionary_from_variant);
 
 }
 
@@ -162,9 +185,10 @@ impl GodotFfiVariant for () {
 }
 
 impl GodotType for () {
-    type Ffi = Self;
+    type Ffi = ();
+    type ToFfi<'a> = ();
 
-    fn to_ffi(&self) -> Self::Ffi {}
+    fn to_ffi(&self) -> Self::ToFfi<'_> {}
 
     fn into_ffi(self) -> Self::Ffi {}
 
@@ -189,9 +213,10 @@ impl GodotFfiVariant for Variant {
 
 impl GodotType for Variant {
     type Ffi = Variant;
+    type ToFfi<'a> = RefArg<'a, Variant>;
 
-    fn to_ffi(&self) -> Self::Ffi {
-        self.clone()
+    fn to_ffi(&self) -> Self::ToFfi<'_> {
+        RefArg::new(self)
     }
 
     fn into_ffi(self) -> Self::Ffi {

godot-core/src/global/mod.rs

@@ -42,19 +42,19 @@ use crate::obj::Gd;
 // Reminder: remove #![allow(deprecated)] in utilities.test along with the below functions.
 
 #[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
-    For detailed reasons, see https://github.com/godot-rust/gdext/pull/892."]
+    For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
 pub fn instance_from_id(instance_id: i64) -> Option<Gd<crate::classes::Object>> {
     crate::gen::utilities::instance_from_id(instance_id)
 }
 
 #[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
-    For detailed reasons, see https://github.com/godot-rust/gdext/pull/892."]
+    For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
 pub fn is_instance_valid(instance: Variant) -> bool {
     crate::gen::utilities::is_instance_valid(&instance)
 }
 
 #[deprecated = "Instance utilities in `godot::global` will be removed. Use methods on `Gd` and `InstanceId` instead.\n\
-    For detailed reasons, see https://github.com/godot-rust/gdext/pull/892."]
+    For detailed reasons, see https://github.com/godot-rust/gdext/pull/901."]
 pub fn is_instance_id_valid(instance_id: i64) -> bool {
     crate::gen::utilities::is_instance_id_valid(instance_id)
 }

godot-core/src/meta/godot_convert/impls.rs

@@ -25,10 +25,13 @@ impl<T> GodotType for Option<T>
 where
     T: GodotType,
     T::Ffi: GodotNullableFfi,
+    for<'f> T::ToFfi<'f>: GodotNullableFfi,
 {
     type Ffi = T::Ffi;
 
-    fn to_ffi(&self) -> Self::Ffi {
+    type ToFfi<'f> = T::ToFfi<'f>;
+
+    fn to_ffi(&self) -> Self::ToFfi<'_> {
         GodotNullableFfi::flatten_option(self.as_ref().map(|t| t.to_ffi()))
     }
 
@@ -91,7 +94,11 @@ where
 impl<T: ToGodot> ToGodot for Option<T>
 where
     Option<T::Via>: GodotType,
-    for<'f> T::ToVia<'f>: GodotType<Ffi: GodotNullableFfi>,
+    for<'v, 'f> T::ToVia<'v>: GodotType<
+        // Associated types need to be nullable.
+        Ffi: GodotNullableFfi,
+        ToFfi<'f>: GodotNullableFfi,
+    >,
 {
     type ToVia<'v> = Option<T::ToVia<'v>>
     // type ToVia<'v> = Self::Via
@@ -155,8 +162,9 @@ macro_rules! impl_godot_scalar {
     ($T:ty as $Via:ty, $err:path, $param_metadata:expr) => {
         impl GodotType for $T {
             type Ffi = $Via;
+            type ToFfi<'f> = $Via;
 
-            fn to_ffi(&self) -> Self::Ffi {
+            fn to_ffi(&self) -> Self::ToFfi<'_> {
                 (*self).into()
             }
 
@@ -188,8 +196,9 @@ macro_rules! impl_godot_scalar {
     ($T:ty as $Via:ty, $param_metadata:expr; lossy) => {
         impl GodotType for $T {
             type Ffi = $Via;
+            type ToFfi<'f> = $Via;
 
-            fn to_ffi(&self) -> Self::Ffi {
+            fn to_ffi(&self) -> Self::ToFfi<'_> {
                 *self as $Via
             }
 
@@ -289,8 +298,9 @@ impl_godot_scalar!(
 
 impl GodotType for u64 {
     type Ffi = i64;
+    type ToFfi<'f> = i64;
 
-    fn to_ffi(&self) -> Self::Ffi {
+    fn to_ffi(&self) -> Self::ToFfi<'_> {
         *self as i64
     }
 

godot-core/src/meta/godot_convert/mod.rs

@@ -40,6 +40,7 @@ pub trait GodotConvert {
 ///
 /// Please read the [`godot::meta` module docs][crate::meta] for further information about conversions.
 pub trait ToGodot: Sized + GodotConvert {
+    /// Target type of [`to_godot()`](ToGodot::to_godot), which differs from [`Via`](GodotConvert::Via) for pass-by-reference types.
     type ToVia<'v>: GodotType
     where
         Self: 'v;
@@ -95,16 +96,10 @@ pub trait FromGodot: Sized + GodotConvert {
     }
 }
 
-// Note: removing the implicit lifetime (by taking value: T instead of &T) causes issues due to allegedly returning a lifetime
-// to a local variable, even though the result Ffi is 'static by definition.
-#[allow(clippy::needless_lifetimes)] // eliding causes error: missing generics for associated type `godot_convert::ToGodot::ToVia`
-pub(crate) fn into_ffi<'v, T: ToGodot>(value: &'v T) -> <T::ToVia<'v> as GodotType>::Ffi {
-    let by_ref = value.to_godot();
-    by_ref.to_ffi()
-}
-
 pub(crate) fn into_ffi_variant<T: ToGodot>(value: &T) -> Variant {
-    GodotFfiVariant::ffi_to_variant(&into_ffi(value))
+    let via = value.to_godot();
+    let ffi = via.to_ffi();
+    GodotFfiVariant::ffi_to_variant(&ffi)
 }
 
 pub(crate) fn try_from_ffi<T: FromGodot>(