Merge pull request #900 from godot-rust/qol/engine-pass-by-ref

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

Author: Bromeon

examples/dodge-the-creeps/rust/src/player.rs

@@ -24,7 +24,7 @@ impl Player {
             .base()
             .get_node_as::<CollisionShape2D>("CollisionShape2D");
 
-        collision_shape.set_deferred("disabled".into(), true.to_variant());
+        collision_shape.set_deferred("disabled".into(), &true.to_variant());
     }
 
     #[func]

godot-codegen/src/context.rs

@@ -64,14 +64,19 @@ impl<'a> Context<'a> {
             }
 
             // Populate class lookup by name
-            println!("-- add engine class {}", class_name.description());
             engine_classes.insert(class_name.clone(), class);
 
             // Populate derived-to-base relations
             if let Some(base) = class.inherits.as_ref() {
                 let base_name = TyName::from_godot(base);
-                println!("  -- inherits {}", base_name.description());
+                println!(
+                    "* Add engine class {} <- inherits {}",
+                    class_name.description(),
+                    base_name.description()
+                );
                 ctx.inheritance_tree.insert(class_name.clone(), base_name);
+            } else {
+                println!("* Add engine class {}", class_name.description());
             }
 
             // Populate notification constants (first, only for classes that declare them themselves).
@@ -238,6 +243,12 @@ impl<'a> Context<'a> {
         self.builtin_types.contains(ty_name)
     }
 
+    pub fn is_builtin_copy(&self, ty_name: &str) -> bool {
+        debug_assert!(!ty_name.starts_with("Packed")); // Already handled separately.
+
+        !matches!(ty_name, "Variant" | "VariantArray" | "Dictionary")
+    }
+
     pub fn is_native_structure(&self, ty_name: &str) -> bool {
         self.native_structures_types.contains(ty_name)
     }

godot-codegen/src/conv/type_conversions.rs

@@ -20,6 +20,7 @@ use crate::util::ident;
 // ----------------------------------------------------------------------------------------------------------------------------------------------
 // Godot -> Rust types
 
+/// Returns `(identifier, is_copy)` for a hardcoded Rust type, if it exists.
 fn to_hardcoded_rust_ident(full_ty: &GodotTy) -> Option<&str> {
     let ty = full_ty.ty.as_str();
     let meta = full_ty.meta.as_deref();
@@ -95,13 +96,25 @@ pub(crate) fn to_rust_type_abi(ty: &str, ctx: &mut Context) -> (RustTy, bool) {
         "Object*" => {
             is_obj = true;
             RustTy::RawPointer {
-                inner: Box::new(RustTy::BuiltinIdent(ident("c_void"))),
+                inner: Box::new(RustTy::BuiltinIdent {
+                    ty: ident("c_void"),
+                    is_copy: true,
+                }),
                 is_const: false,
             }
         }
-        "int" => RustTy::BuiltinIdent(ident("i32")),
-        "float" => RustTy::BuiltinIdent(ident("f32")),
-        "double" => RustTy::BuiltinIdent(ident("f64")),
+        "int" => RustTy::BuiltinIdent {
+            ty: ident("i32"),
+            is_copy: true,
+        },
+        "float" => RustTy::BuiltinIdent {
+            ty: ident("f32"),
+            is_copy: true,
+        },
+        "double" => RustTy::BuiltinIdent {
+            ty: ident("f64"),
+            is_copy: true,
+        },
         _ => to_rust_type(ty, None, ctx),
     };
 
@@ -137,6 +150,7 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy {
         if is_builtin_type_scalar(ty) {
             ident(ty)
         } else {
+            // Convert as-is. Includes StringName and NodePath.
             TyName::from_godot(ty).rust_ty
         }
     }
@@ -163,7 +177,10 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy {
     // Only place where meta is relevant is here.
     if !ty.starts_with("typedarray::") {
         if let Some(hardcoded) = to_hardcoded_rust_ident(full_ty) {
-            return RustTy::BuiltinIdent(ident(hardcoded));
+            return RustTy::BuiltinIdent {
+                ty: ident(hardcoded),
+                is_copy: ctx.is_builtin_copy(hardcoded),
+            };
         }
     }
 
@@ -182,7 +199,10 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy {
     } else if let Some(packed_arr_ty) = ty.strip_prefix("Packed") {
         // Don't trigger on PackedScene ;P
         if packed_arr_ty.ends_with("Array") {
-            return RustTy::BuiltinIdent(rustify_ty(ty));
+            return RustTy::BuiltinIdent {
+                ty: rustify_ty(ty),
+                is_copy: false, // Packed arrays are not Copy.
+            };
         }
     } else if let Some(elem_ty) = ty.strip_prefix("typedarray::") {
         let rust_elem_ty = to_rust_type(elem_ty, full_ty.meta.as_ref(), ctx);
@@ -200,8 +220,12 @@ fn to_rust_type_uncached(full_ty: &GodotTy, ctx: &mut Context) -> RustTy {
 
     // Note: do not check if it's a known engine class, because that will not work in minimal mode (since not all classes are stored)
     if ctx.is_builtin(ty) || ctx.is_native_structure(ty) {
-        // Unchanged
-        RustTy::BuiltinIdent(rustify_ty(ty))
+        // Unchanged.
+        // Native structures might not all be Copy, but they should have value semantics.
+        RustTy::BuiltinIdent {
+            ty: rustify_ty(ty),
+            is_copy: ctx.is_builtin_copy(ty),
+        }
     } else {
         let ty = rustify_ty(ty);
         let qualified_class = quote! { crate::classes::#ty };
@@ -263,7 +287,9 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
         "{}" => return quote! { Dictionary::new() },
         "null" => {
             return match ty {
-                RustTy::BuiltinIdent(ident) if ident == "Variant" => quote! { Variant::nil() },
+                RustTy::BuiltinIdent { ty: ident, .. } if ident == "Variant" => {
+                    quote! { Variant::nil() }
+                }
                 RustTy::EngineClass { .. } => {
                     quote! { Gd::null_arg() }
                 }
@@ -273,8 +299,8 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
         // empty string appears only for Callable/Rid in 4.0; default ctor syntax in 4.1+
         "" | "RID()" | "Callable()" if !is_inner => {
             return match ty {
-                RustTy::BuiltinIdent(ident) if ident == "Rid" => quote! { Rid::Invalid },
-                RustTy::BuiltinIdent(ident) if ident == "Callable" => {
+                RustTy::BuiltinIdent { ty: ident, .. } if ident == "Rid" => quote! { Rid::Invalid },
+                RustTy::BuiltinIdent { ty: ident, .. } if ident == "Callable" => {
                     quote! { Callable::invalid() }
                 }
                 _ => panic!("empty string not representable in target type {ty:?}"),
@@ -295,9 +321,11 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
                 is_bitfield: false, ..
             } => quote! { crate::obj::EngineEnum::from_ord(#lit) },
 
-            RustTy::BuiltinIdent(ident) if ident == "Variant" => quote! { Variant::from(#lit) },
+            RustTy::BuiltinIdent { ty: ident, .. } if ident == "Variant" => {
+                quote! { Variant::from(#lit) }
+            }
 
-            RustTy::BuiltinIdent(ident)
+            RustTy::BuiltinIdent { ty: ident, .. }
                 if ident == "i64" || ident == "f64" || unmap_meta(ty).is_some() =>
             {
                 suffixed_lit(num, ident)
@@ -313,7 +341,9 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
     // Float literals (some floats already handled by integer literals)
     if let Ok(num) = expr.parse::<f64>() {
         return match ty {
-            RustTy::BuiltinIdent(ident) if ident == "f64" || unmap_meta(ty).is_some() => {
+            RustTy::BuiltinIdent { ty: ident, .. }
+                if ident == "f64" || unmap_meta(ty).is_some() =>
+            {
                 suffixed_lit(num, ident)
             }
             _ if is_inner => {
@@ -331,7 +361,7 @@ fn to_rust_expr_inner(expr: &str, ty: &RustTy, is_inner: bool) -> TokenStream {
             quote! { #expr }
         } else {
             match ty {
-                RustTy::BuiltinIdent(ident)
+                RustTy::BuiltinIdent { ty: ident, .. }
                     if ident == "GString" || ident == "StringName" || ident == "NodePath" =>
                 {
                     quote! { #ident::from(#expr) }
@@ -429,16 +459,28 @@ fn gdscript_to_rust_expr() {
     // The 'None' type is used to simulate absence of type information. Some tests are commented out, because this functionality is not
     // yet needed. If we ever want to reuse to_rust_expr() in other contexts, we could re-enable them.
 
-    let ty_int = RustTy::BuiltinIdent(ident("i64"));
+    let ty_int = RustTy::BuiltinIdent {
+        ty: ident("i64"),
+        is_copy: true,
+    };
     let ty_int = Some(&ty_int);
 
-    let ty_int_u16 = RustTy::BuiltinIdent(ident("u16"));
+    let ty_int_u16 = RustTy::BuiltinIdent {
+        ty: ident("u16"),
+        is_copy: true,
+    };
     let ty_int_u16 = Some(&ty_int_u16);
 
-    let ty_float = RustTy::BuiltinIdent(ident("f64"));
+    let ty_float = RustTy::BuiltinIdent {
+        ty: ident("f64"),
+        is_copy: true,
+    };
     let ty_float = Some(&ty_float);
 
-    let ty_float_f32 = RustTy::BuiltinIdent(ident("f32"));
+    let ty_float_f32 = RustTy::BuiltinIdent {
+        ty: ident("f32"),
+        is_copy: true,
+    };
     let ty_float_f32 = Some(&ty_float_f32);
 
     let ty_enum = RustTy::EngineEnum {
@@ -455,7 +497,10 @@ fn gdscript_to_rust_expr() {
     };
     let ty_bitfield = Some(&ty_bitfield);
 
-    let ty_variant = RustTy::BuiltinIdent(ident("Variant"));
+    let ty_variant = RustTy::BuiltinIdent {
+        ty: ident("Variant"),
+        is_copy: false,
+    };
     let ty_variant = Some(&ty_variant);
 
     // let ty_object = RustTy::EngineClass {
@@ -464,13 +509,22 @@ fn gdscript_to_rust_expr() {
     // };
     // let ty_object = Some(&ty_object);
 
-    let ty_string = RustTy::BuiltinIdent(ident("GString"));
+    let ty_string = RustTy::BuiltinIdent {
+        ty: ident("GString"),
+        is_copy: true,
+    };
     let ty_string = Some(&ty_string);
 
-    let ty_stringname = RustTy::BuiltinIdent(ident("StringName"));
+    let ty_stringname = RustTy::BuiltinIdent {
+        ty: ident("StringName"),
+        is_copy: true,
+    };
     let ty_stringname = Some(&ty_stringname);
 
-    let ty_nodepath = RustTy::BuiltinIdent(ident("NodePath"));
+    let ty_nodepath = RustTy::BuiltinIdent {
+        ty: ident("NodePath"),
+        is_copy: true,
+    };
     let ty_nodepath = Some(&ty_nodepath);
 
     #[rustfmt::skip]
@@ -581,7 +635,7 @@ fn gdscript_to_rust_expr() {
 ///
 /// Avoids dragging along the meta type through [`RustTy::BuiltinIdent`].
 pub(crate) fn unmap_meta(rust_ty: &RustTy) -> Option<Ident> {
-    let RustTy::BuiltinIdent(rust_ty) = rust_ty else {
+    let RustTy::BuiltinIdent { ty: rust_ty, .. } = rust_ty else {
         return None;
     };
 

godot-codegen/src/generator/builtins.rs

@@ -95,7 +95,10 @@ fn make_builtin_class(
 ) -> GeneratedBuiltin {
     let godot_name = &class.name().godot_ty;
 
-    let RustTy::BuiltinIdent(outer_class) = conv::to_rust_type(godot_name, None, ctx) else {
+    let RustTy::BuiltinIdent {
+        ty: outer_class, ..
+    } = conv::to_rust_type(godot_name, None, ctx)
+    else {
         panic!("Rust type `{}` categorized wrong", godot_name)
     };
     let inner_class = class.inner_name();