Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass-by-ref for non-Copy builtins (frontend) #900

Merged
merged 10 commits into from
Sep 18, 2024
2 changes: 1 addition & 1 deletion examples/dodge-the-creeps/rust/src/player.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
15 changes: 13 additions & 2 deletions godot-codegen/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down Expand Up @@ -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)
}
Expand Down
102 changes: 78 additions & 24 deletions godot-codegen/src/conv/type_conversions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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),
};

Expand Down Expand Up @@ -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
}
}
Expand All @@ -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),
};
}
}

Expand All @@ -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);
Expand All @@ -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 };
Expand Down Expand Up @@ -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() }
}
Expand All @@ -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:?}"),
Expand All @@ -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)
Expand All @@ -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 => {
Expand All @@ -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) }
Expand Down Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -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]
Expand Down Expand Up @@ -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;
};

Expand Down
5 changes: 4 additions & 1 deletion godot-codegen/src/generator/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Loading
Loading