Skip to content

Commit 2c48248

Browse files
committed
Bugfix – initialize&cache proper return value for generic, typed array.
1 parent c3f705b commit 2c48248

File tree

9 files changed

+99
-30
lines changed

9 files changed

+99
-30
lines changed

godot-codegen/src/generator/builtins.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,8 @@ fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStr
198198
/// Get the safety docs of an unsafe method, or `None` if it is safe.
199199
fn method_safety_doc(class_name: &TyName, method: &BuiltinMethod) -> Option<TokenStream> {
200200
if class_name.godot_ty == "Array"
201-
&& &method.return_value().type_tokens().to_string() == "VariantArray"
201+
&& (&method.return_value().type_tokens().to_string() == "VariantArray"
202+
|| method.is_generic())
202203
{
203204
return Some(quote! {
204205
/// # Safety
@@ -252,11 +253,13 @@ fn make_builtin_method_definition(
252253
let receiver = functions_common::make_receiver(method.qualifier(), ffi_arg_in);
253254
let object_ptr = &receiver.ffi_arg;
254255

256+
let generic_params = method.return_value().generic_params();
257+
255258
let ptrcall_invocation = quote! {
256259
let method_bind = sys::builtin_method_table().#fptr_access;
257260

258261

259-
Signature::<CallParams, CallRet>::out_builtin_ptrcall(
262+
Signature::<CallParams, CallRet #generic_params>::out_builtin_ptrcall(
260263
method_bind,
261264
#builtin_name_str,
262265
#method_name_str,

godot-codegen/src/generator/default_parameters.rs

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use quote::{format_ident, quote};
1111

1212
use crate::generator::functions_common;
1313
use crate::generator::functions_common::{
14-
make_arg_expr, make_param_or_field_type, FnArgExpr, FnCode, FnKind, FnParamDecl, FnParamTokens,
14+
make_arg_expr, make_param_or_field_type, FnArgExpr, FnCode, FnKind, FnParamDecl,
1515
};
1616
use crate::models::domain::{FnParam, FnQualifier, Function, RustTy, TyName};
1717
use crate::util::{ident, safe_ident};
@@ -59,15 +59,6 @@ pub fn make_function_definition_with_defaults(
5959
&default_fn_params,
6060
);
6161

62-
// ExBuilder::new() constructor signature.
63-
let FnParamTokens {
64-
func_general_lifetime: simple_fn_lifetime,
65-
..
66-
} = fns::make_params_exprs(
67-
required_fn_params.iter().cloned(),
68-
FnKind::ExBuilderConstructor,
69-
);
70-
7162
let return_decl = &sig.return_value().decl;
7263

7364
// If either the builder has a lifetime (non-static/global method), or one of its parameters is a reference,
@@ -119,7 +110,7 @@ pub fn make_function_definition_with_defaults(
119110
// Lifetime is set if any parameter is a reference.
120111
#[doc = #default_parameter_usage]
121112
#[inline]
122-
#vis fn #simple_fn_name #simple_fn_lifetime (
113+
#vis fn #simple_fn_name (
123114
#simple_receiver_param
124115
#( #class_method_required_params, )*
125116
) #return_decl {

godot-codegen/src/generator/functions_common.rs

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,6 @@ pub struct FnParamTokens {
9898
/// Generic argument list `<'a0, 'a1, ...>` after `type CallSig`, if available.
9999
pub callsig_lifetime_args: Option<TokenStream>,
100100
pub arg_exprs: Vec<TokenStream>,
101-
pub func_general_lifetime: Option<TokenStream>,
102101
}
103102

104103
pub fn make_function_definition(
@@ -142,7 +141,6 @@ pub fn make_function_definition(
142141
callsig_param_types: param_types,
143142
callsig_lifetime_args,
144143
arg_exprs: arg_names,
145-
func_general_lifetime: fn_lifetime,
146144
} = if sig.is_virtual() {
147145
make_params_exprs_virtual(sig.params().iter(), sig)
148146
} else {
@@ -175,11 +173,14 @@ pub fn make_function_definition(
175173
default_structs_code = TokenStream::new();
176174
};
177175

176+
let func_generic_params = sig.return_value().generic_params();
177+
let func_generic_bounds = sig.return_value().generic_bounds();
178+
178179
let call_sig_decl = {
179180
let return_ty = &sig.return_value().type_tokens();
180181

181182
quote! {
182-
type CallRet = #return_ty;
183+
type CallRet #func_generic_params = #return_ty;
183184
type CallParams #callsig_lifetime_args = (#(#param_types,)*);
184185
}
185186
};
@@ -279,10 +280,12 @@ pub fn make_function_definition(
279280

280281
quote! {
281282
#maybe_safety_doc
282-
#vis #maybe_unsafe fn #primary_fn_name #fn_lifetime (
283+
#vis #maybe_unsafe fn #primary_fn_name #func_generic_params (
283284
#receiver_param
284285
#( #params, )*
285-
) #return_decl {
286+
) #return_decl
287+
#func_generic_bounds
288+
{
286289
#call_sig_decl
287290

288291
let args = (#( #arg_names, )*);
@@ -357,9 +360,9 @@ pub(crate) enum FnKind {
357360
/// `call()` forwarding to `try_call()`.
358361
DelegateTry,
359362

360-
/// Default extender `new()` associated function -- optional receiver and required parameters.
361-
ExBuilderConstructor,
362-
363+
// Currently not used – uncomment when needed again.
364+
// /// Default extender `new()` associated function -- optional receiver and required parameters.
365+
// ExBuilderConstructor,
363366
/// Same as [`ExBuilderConstructor`], but for a builder with an explicit lifetime.
364367
ExBuilderConstructorLifetimed,
365368

@@ -489,6 +492,7 @@ pub(crate) fn make_param_or_field_type(
489492
..
490493
}
491494
| RustTy::BuiltinArray { .. }
495+
| RustTy::GenericArray
492496
| RustTy::EngineArray { .. } => {
493497
let lft = lifetimes.next();
494498
special_ty = Some(quote! { RefArg<#lft, #ty> });
@@ -572,7 +576,7 @@ pub(crate) fn make_params_exprs<'a>(
572576
// Methods relevant in the context of default parameters. Flow in this order.
573577
// Note that for builder methods of Ex* structs, there's a direct call in default_parameters.rs to the parameter manipulation methods,
574578
// bypassing this method. So one case is missing here.
575-
FnKind::ExBuilderConstructor => (FnParamDecl::FnPublic, FnArgExpr::StoreInField),
579+
// FnKind::ExBuilderConstructor => (FnParamDecl::FnPublic, FnArgExpr::StoreInField),
576580
FnKind::ExBuilderConstructorLifetimed => {
577581
(FnParamDecl::FnPublicLifetime, FnArgExpr::StoreInField)
578582
}

godot-codegen/src/models/domain.rs

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -322,6 +322,9 @@ pub trait Function: fmt::Display {
322322
fn is_virtual(&self) -> bool {
323323
matches!(self.direction(), FnDirection::Virtual { .. })
324324
}
325+
fn is_generic(&self) -> bool {
326+
matches!(self.return_value().type_, Some(RustTy::GenericArray))
327+
}
325328
fn direction(&self) -> FnDirection {
326329
self.common().direction
327330
}
@@ -669,6 +672,14 @@ impl FnReturn {
669672
}
670673
}
671674

675+
pub fn generic_params(&self) -> Option<TokenStream> {
676+
self.type_.as_ref()?.generic_params()
677+
}
678+
679+
pub fn generic_bounds(&self) -> Option<TokenStream> {
680+
self.type_.as_ref()?.generic_bounds()
681+
}
682+
672683
pub fn call_result_decl(&self) -> TokenStream {
673684
let ret = self.type_tokens();
674685
quote! { -> Result<#ret, crate::meta::error::CallError> }
@@ -705,6 +716,11 @@ pub enum RustTy {
705716
/// Note that untyped arrays are mapped as `BuiltinIdent("Array")`.
706717
BuiltinArray { elem_type: TokenStream },
707718

719+
/// `Array<T>`
720+
///
721+
///
722+
GenericArray,
723+
708724
/// C-style raw pointer to a `RustTy`.
709725
RawPointer { inner: Box<RustTy>, is_const: bool },
710726

@@ -755,10 +771,30 @@ impl RustTy {
755771
pub fn return_decl(&self) -> TokenStream {
756772
match self {
757773
Self::EngineClass { tokens, .. } => quote! { -> Option<#tokens> },
774+
Self::GenericArray => quote! { -> Array<Ret> },
758775
other => quote! { -> #other },
759776
}
760777
}
761778

779+
pub fn generic_params(&self) -> Option<TokenStream> {
780+
if matches!(self, Self::GenericArray) {
781+
Some(quote! { < Ret > })
782+
} else {
783+
None
784+
}
785+
}
786+
787+
pub fn generic_bounds(&self) -> Option<TokenStream> {
788+
if matches!(self, Self::GenericArray) {
789+
Some(quote! {
790+
where
791+
Ret: crate::meta::ArrayElement,
792+
})
793+
} else {
794+
None
795+
}
796+
}
797+
762798
pub fn is_integer(&self) -> bool {
763799
let RustTy::BuiltinIdent { ty, .. } = self else {
764800
return false;
@@ -789,6 +825,7 @@ impl ToTokens for RustTy {
789825
RustTy::EngineEnum { tokens: path, .. } => path.to_tokens(tokens),
790826
RustTy::EngineClass { tokens: path, .. } => path.to_tokens(tokens),
791827
RustTy::ExtenderReceiver { tokens: path } => path.to_tokens(tokens),
828+
RustTy::GenericArray => quote! { Array<Ret> }.to_tokens(tokens),
792829
}
793830
}
794831
}

godot-codegen/src/models/domain_mapping.rs

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -366,10 +366,18 @@ impl BuiltinMethod {
366366
return None;
367367
}
368368

369-
let return_value = method
370-
.return_type
371-
.as_deref()
372-
.map(JsonMethodReturn::from_type_no_meta);
369+
let return_value =
370+
if let Some(generic) = special_cases::is_builtin_method_generic(builtin_name, method) {
371+
generic
372+
} else {
373+
FnReturn::new(
374+
&method
375+
.return_type
376+
.as_deref()
377+
.map(JsonMethodReturn::from_type_no_meta),
378+
ctx,
379+
)
380+
};
373381

374382
Some(Self {
375383
common: FunctionCommon {
@@ -381,7 +389,7 @@ impl BuiltinMethod {
381389
parameters: FnParam::builder()
382390
.no_defaults()
383391
.build_many(&method.arguments, ctx),
384-
return_value: FnReturn::new(&return_value, ctx),
392+
return_value,
385393
is_vararg: method.is_vararg,
386394
is_private: false, // See 'exposed' below. Could be special_cases::is_method_private(builtin_name, &method.name),
387395
is_virtual_required: false,

godot-codegen/src/special_cases/codegen_special_cases.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool {
4848
match ty {
4949
RustTy::BuiltinIdent { .. } => false,
5050
RustTy::BuiltinArray { .. } => false,
51+
RustTy::GenericArray => false,
5152
RustTy::RawPointer { inner, .. } => is_rust_type_excluded(inner),
5253
RustTy::EngineArray { elem_class, .. } => is_class_excluded(elem_class.as_str()),
5354
RustTy::EngineEnum {

godot-codegen/src/special_cases/special_cases.rs

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use proc_macro2::Ident;
3333

3434
use crate::conv::to_enum_type_uncached;
3535
use crate::models::domain::{
36-
ClassCodegenLevel, Enum, EnumReplacements, RustTy, TyName, VirtualMethodPresence,
36+
ClassCodegenLevel, Enum, EnumReplacements, FnReturn, RustTy, TyName, VirtualMethodPresence,
3737
};
3838
use crate::models::json::{JsonBuiltinMethod, JsonClassMethod, JsonSignal, JsonUtilityFunction};
3939
use crate::special_cases::codegen_special_cases;
@@ -778,6 +778,22 @@ pub fn is_builtin_method_deleted(_class_name: &TyName, method: &JsonBuiltinMetho
778778
codegen_special_cases::is_builtin_method_excluded(method)
779779
}
780780

781+
pub fn is_builtin_method_generic(
782+
class_name: &TyName,
783+
method: &JsonBuiltinMethod,
784+
) -> Option<FnReturn> {
785+
match (
786+
class_name.rust_ty.to_string().as_str(),
787+
method.name.as_str(),
788+
) {
789+
("Array", "duplicate") => Some(FnReturn {
790+
decl: RustTy::GenericArray.return_decl(),
791+
type_: Some(RustTy::GenericArray),
792+
}),
793+
_ => None,
794+
}
795+
}
796+
781797
/// True if signal is absent from codegen (only when surrounding class is excluded).
782798
pub fn is_signal_deleted(_class_name: &TyName, signal: &JsonSignal) -> bool {
783799
// If any argument type (a class) is excluded.

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -534,7 +534,7 @@ impl<T: ArrayElement> Array<T> {
534534
/// To create a new reference to the same array data, use [`clone()`][Clone::clone].
535535
pub fn duplicate_shallow(&self) -> Self {
536536
// SAFETY: We never write to the duplicated array, and all values read are read as `Variant`.
537-
let duplicate: VariantArray = unsafe { self.as_inner().duplicate(false) };
537+
let duplicate: Self = unsafe { self.as_inner().duplicate(false) };
538538

539539
// SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type.
540540
let result = unsafe { duplicate.assume_type() };
@@ -549,7 +549,7 @@ impl<T: ArrayElement> Array<T> {
549549
/// To create a new reference to the same array data, use [`clone()`][Clone::clone].
550550
pub fn duplicate_deep(&self) -> Self {
551551
// SAFETY: We never write to the duplicated array, and all values read are read as `Variant`.
552-
let duplicate: VariantArray = unsafe { self.as_inner().duplicate(true) };
552+
let duplicate: Self = unsafe { self.as_inner().duplicate(true) };
553553

554554
// SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type.
555555
let result = unsafe { duplicate.assume_type() };

itest/rust/src/builtin_tests/containers/array_test.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,15 @@ func make_array() -> Array[CustomScriptForArrays]:
677677
assert_eq!(script.get_global_name(), "CustomScriptForArrays".into());
678678
}
679679

680+
#[itest]
681+
fn array_inner_type() {
682+
let primary = Array::<Dictionary>::new();
683+
let secondary = primary.duplicate_shallow();
684+
assert_eq!(secondary.element_type(), primary.element_type());
685+
let secondary = primary.duplicate_deep();
686+
assert_eq!(secondary.element_type(), primary.element_type())
687+
}
688+
680689
// ----------------------------------------------------------------------------------------------------------------------------------------------
681690
// Class definitions
682691

0 commit comments

Comments
 (0)