diff --git a/godot-codegen/src/generator/builtins.rs b/godot-codegen/src/generator/builtins.rs index 321fd0a8b..bfdcc9acf 100644 --- a/godot-codegen/src/generator/builtins.rs +++ b/godot-codegen/src/generator/builtins.rs @@ -197,14 +197,27 @@ fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStr /// Get the safety docs of an unsafe method, or `None` if it is safe. fn method_safety_doc(class_name: &TyName, method: &BuiltinMethod) -> Option { - if class_name.godot_ty == "Array" - && &method.return_value().type_tokens().to_string() == "VariantArray" - { - return Some(quote! { - /// # Safety - /// - /// You must ensure that the returned array fulfils the safety invariants of [`Array`](crate::builtin::Array). - }); + if class_name.godot_ty == "Array" { + if method.is_generic() { + return Some(quote! { + /// # Safety + /// You must ensure that the returned array fulfils the safety invariants of [`Array`](crate::builtin::Array), this being: + /// - Any values written to the array must match the runtime type of the array. + /// - Any values read from the array must be convertible to the type `T`. + /// + /// If the safety invariant of `Array` is intact, which it must be for any publicly accessible arrays, then `T` must match + /// the runtime type of the array. This then implies that both of the conditions above hold. This means that you only need + /// to keep the above conditions in mind if you are intentionally violating the safety invariant of `Array`. + /// + /// In the current implementation, both cases will produce a panic rather than undefined behavior, but this should not be relied upon. + }); + } else if &method.return_value().type_tokens().to_string() == "VariantArray" { + return Some(quote! { + /// # Safety + /// + /// You must ensure that the returned array fulfils the safety invariants of [`Array`](crate::builtin::Array). + }); + } } None @@ -252,11 +265,13 @@ fn make_builtin_method_definition( let receiver = functions_common::make_receiver(method.qualifier(), ffi_arg_in); let object_ptr = &receiver.ffi_arg; + let maybe_generic_params = method.return_value().generic_params(); + let ptrcall_invocation = quote! { let method_bind = sys::builtin_method_table().#fptr_access; - Signature::::out_builtin_ptrcall( + Signature::::out_builtin_ptrcall( method_bind, #builtin_name_str, #method_name_str, diff --git a/godot-codegen/src/generator/default_parameters.rs b/godot-codegen/src/generator/default_parameters.rs index 7f0ac3d82..8ec204cb2 100644 --- a/godot-codegen/src/generator/default_parameters.rs +++ b/godot-codegen/src/generator/default_parameters.rs @@ -11,7 +11,7 @@ use quote::{format_ident, quote}; use crate::generator::functions_common; use crate::generator::functions_common::{ - make_arg_expr, make_param_or_field_type, FnArgExpr, FnCode, FnKind, FnParamDecl, FnParamTokens, + make_arg_expr, make_param_or_field_type, FnArgExpr, FnCode, FnKind, FnParamDecl, }; use crate::models::domain::{FnParam, FnQualifier, Function, RustTy, TyName}; use crate::util::{ident, safe_ident}; @@ -59,15 +59,6 @@ pub fn make_function_definition_with_defaults( &default_fn_params, ); - // ExBuilder::new() constructor signature. - let FnParamTokens { - func_general_lifetime: simple_fn_lifetime, - .. - } = fns::make_params_exprs( - required_fn_params.iter().cloned(), - FnKind::ExBuilderConstructor, - ); - let return_decl = &sig.return_value().decl; // 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( // Lifetime is set if any parameter is a reference. #[doc = #default_parameter_usage] #[inline] - #vis fn #simple_fn_name #simple_fn_lifetime ( + #vis fn #simple_fn_name ( #simple_receiver_param #( #class_method_required_params, )* ) #return_decl { diff --git a/godot-codegen/src/generator/functions_common.rs b/godot-codegen/src/generator/functions_common.rs index aa14d177b..e1600587c 100644 --- a/godot-codegen/src/generator/functions_common.rs +++ b/godot-codegen/src/generator/functions_common.rs @@ -98,7 +98,6 @@ pub struct FnParamTokens { /// Generic argument list `<'a0, 'a1, ...>` after `type CallSig`, if available. pub callsig_lifetime_args: Option, pub arg_exprs: Vec, - pub func_general_lifetime: Option, } pub fn make_function_definition( @@ -142,7 +141,6 @@ pub fn make_function_definition( callsig_param_types: param_types, callsig_lifetime_args, arg_exprs: arg_names, - func_general_lifetime: fn_lifetime, } = if sig.is_virtual() { make_params_exprs_virtual(sig.params().iter(), sig) } else { @@ -175,11 +173,14 @@ pub fn make_function_definition( default_structs_code = TokenStream::new(); }; + let maybe_func_generic_params = sig.return_value().generic_params(); + let maybe_func_generic_bounds = sig.return_value().where_clause(); + let call_sig_decl = { let return_ty = &sig.return_value().type_tokens(); quote! { - type CallRet = #return_ty; + type CallRet #maybe_func_generic_params = #return_ty; type CallParams #callsig_lifetime_args = (#(#param_types,)*); } }; @@ -279,10 +280,12 @@ pub fn make_function_definition( quote! { #maybe_safety_doc - #vis #maybe_unsafe fn #primary_fn_name #fn_lifetime ( + #vis #maybe_unsafe fn #primary_fn_name #maybe_func_generic_params ( #receiver_param #( #params, )* - ) #return_decl { + ) #return_decl + #maybe_func_generic_bounds + { #call_sig_decl let args = (#( #arg_names, )*); @@ -357,10 +360,7 @@ pub(crate) enum FnKind { /// `call()` forwarding to `try_call()`. DelegateTry, - /// Default extender `new()` associated function -- optional receiver and required parameters. - ExBuilderConstructor, - - /// Same as [`ExBuilderConstructor`], but for a builder with an explicit lifetime. + /// Default extender `new()` associated function -- optional receiver and required parameters. Has an explicit lifetime. ExBuilderConstructorLifetimed, /// Default extender `new()` associated function -- only default parameters. @@ -489,6 +489,7 @@ pub(crate) fn make_param_or_field_type( .. } | RustTy::BuiltinArray { .. } + | RustTy::GenericArray | RustTy::EngineArray { .. } => { let lft = lifetimes.next(); special_ty = Some(quote! { RefArg<#lft, #ty> }); @@ -572,7 +573,6 @@ pub(crate) fn make_params_exprs<'a>( // Methods relevant in the context of default parameters. Flow in this order. // Note that for builder methods of Ex* structs, there's a direct call in default_parameters.rs to the parameter manipulation methods, // bypassing this method. So one case is missing here. - FnKind::ExBuilderConstructor => (FnParamDecl::FnPublic, FnArgExpr::StoreInField), FnKind::ExBuilderConstructorLifetimed => { (FnParamDecl::FnPublicLifetime, FnArgExpr::StoreInField) } diff --git a/godot-codegen/src/models/domain.rs b/godot-codegen/src/models/domain.rs index 7da3aa270..8ab808e01 100644 --- a/godot-codegen/src/models/domain.rs +++ b/godot-codegen/src/models/domain.rs @@ -300,28 +300,40 @@ pub trait Function: fmt::Display { fn name(&self) -> &str { &self.common().name } + /// Rust name as `Ident`. Might be cached in future. fn name_ident(&self) -> Ident { safe_ident(self.name()) } + fn godot_name(&self) -> &str { &self.common().godot_name } + fn params(&self) -> &[FnParam] { &self.common().parameters } + fn return_value(&self) -> &FnReturn { &self.common().return_value } + fn is_vararg(&self) -> bool { self.common().is_vararg } + fn is_private(&self) -> bool { self.common().is_private } + fn is_virtual(&self) -> bool { matches!(self.direction(), FnDirection::Virtual { .. }) } + + fn is_generic(&self) -> bool { + matches!(self.return_value().type_, Some(RustTy::GenericArray)) + } + fn direction(&self) -> FnDirection { self.common().direction } @@ -621,6 +633,13 @@ impl FnReturn { Self::with_enum_replacements(return_value, &[], ctx) } + pub fn with_generic_builtin(generic_type: RustTy) -> Self { + Self { + decl: generic_type.return_decl(), + type_: Some(generic_type), + } + } + pub fn with_enum_replacements( return_value: &Option, replacements: EnumReplacements, @@ -669,6 +688,14 @@ impl FnReturn { } } + pub fn generic_params(&self) -> Option { + self.type_.as_ref()?.generic_params() + } + + pub fn where_clause(&self) -> Option { + self.type_.as_ref()?.where_clause() + } + pub fn call_result_decl(&self) -> TokenStream { let ret = self.type_tokens(); quote! { -> Result<#ret, crate::meta::error::CallError> } @@ -705,6 +732,11 @@ pub enum RustTy { /// Note that untyped arrays are mapped as `BuiltinIdent("Array")`. BuiltinArray { elem_type: TokenStream }, + /// Will be included as `Array` in the generated source. + /// + /// Set by [`builtin_method_generic_ret`](crate::special_cases::builtin_method_generic_ret) + GenericArray, + /// C-style raw pointer to a `RustTy`. RawPointer { inner: Box, is_const: bool }, @@ -755,10 +787,30 @@ impl RustTy { pub fn return_decl(&self) -> TokenStream { match self { Self::EngineClass { tokens, .. } => quote! { -> Option<#tokens> }, + Self::GenericArray => quote! { -> Array }, other => quote! { -> #other }, } } + pub fn generic_params(&self) -> Option { + if matches!(self, Self::GenericArray) { + Some(quote! { < Ret > }) + } else { + None + } + } + + pub fn where_clause(&self) -> Option { + if matches!(self, Self::GenericArray) { + Some(quote! { + where + Ret: crate::meta::ArrayElement, + }) + } else { + None + } + } + pub fn is_integer(&self) -> bool { let RustTy::BuiltinIdent { ty, .. } = self else { return false; @@ -789,6 +841,7 @@ impl ToTokens for RustTy { RustTy::EngineEnum { tokens: path, .. } => path.to_tokens(tokens), RustTy::EngineClass { tokens: path, .. } => path.to_tokens(tokens), RustTy::ExtenderReceiver { tokens: path } => path.to_tokens(tokens), + RustTy::GenericArray => quote! { Array }.to_tokens(tokens), } } } diff --git a/godot-codegen/src/models/domain_mapping.rs b/godot-codegen/src/models/domain_mapping.rs index dbc89d0ee..477075028 100644 --- a/godot-codegen/src/models/domain_mapping.rs +++ b/godot-codegen/src/models/domain_mapping.rs @@ -366,10 +366,17 @@ impl BuiltinMethod { return None; } - let return_value = method - .return_type - .as_deref() - .map(JsonMethodReturn::from_type_no_meta); + let return_value = if let Some(generic) = + special_cases::builtin_method_generic_ret(builtin_name, method) + { + generic + } else { + let return_value = &method + .return_type + .as_deref() + .map(JsonMethodReturn::from_type_no_meta); + FnReturn::new(return_value, ctx) + }; Some(Self { common: FunctionCommon { @@ -381,7 +388,7 @@ impl BuiltinMethod { parameters: FnParam::builder() .no_defaults() .build_many(&method.arguments, ctx), - return_value: FnReturn::new(&return_value, ctx), + return_value, is_vararg: method.is_vararg, is_private: false, // See 'exposed' below. Could be special_cases::is_method_private(builtin_name, &method.name), is_virtual_required: false, diff --git a/godot-codegen/src/special_cases/codegen_special_cases.rs b/godot-codegen/src/special_cases/codegen_special_cases.rs index c045e4630..7ae19cbf6 100644 --- a/godot-codegen/src/special_cases/codegen_special_cases.rs +++ b/godot-codegen/src/special_cases/codegen_special_cases.rs @@ -48,6 +48,7 @@ fn is_type_excluded(ty: &str, ctx: &mut Context) -> bool { match ty { RustTy::BuiltinIdent { .. } => false, RustTy::BuiltinArray { .. } => false, + RustTy::GenericArray => false, RustTy::RawPointer { inner, .. } => is_rust_type_excluded(inner), RustTy::EngineArray { elem_class, .. } => is_class_excluded(elem_class.as_str()), RustTy::EngineEnum { diff --git a/godot-codegen/src/special_cases/special_cases.rs b/godot-codegen/src/special_cases/special_cases.rs index 518a5c1af..afa444f21 100644 --- a/godot-codegen/src/special_cases/special_cases.rs +++ b/godot-codegen/src/special_cases/special_cases.rs @@ -33,7 +33,7 @@ use proc_macro2::Ident; use crate::conv::to_enum_type_uncached; use crate::models::domain::{ - ClassCodegenLevel, Enum, EnumReplacements, RustTy, TyName, VirtualMethodPresence, + ClassCodegenLevel, Enum, EnumReplacements, FnReturn, RustTy, TyName, VirtualMethodPresence, }; use crate::models::json::{JsonBuiltinMethod, JsonClassMethod, JsonSignal, JsonUtilityFunction}; use crate::special_cases::codegen_special_cases; @@ -778,6 +778,24 @@ pub fn is_builtin_method_deleted(_class_name: &TyName, method: &JsonBuiltinMetho codegen_special_cases::is_builtin_method_excluded(method) } +/// Returns some generic type – such as `GenericArray` representing `Array` – if method is marked as generic, `None` otherwise. +/// +/// Usually required to initialize the return value and cache its type (see also https://github.com/godot-rust/gdext/pull/1357). +pub fn builtin_method_generic_ret( + class_name: &TyName, + method: &JsonBuiltinMethod, +) -> Option { + match ( + class_name.rust_ty.to_string().as_str(), + method.name.as_str(), + ) { + ("Array", "duplicate") | ("Array", "slice") => { + Some(FnReturn::with_generic_builtin(RustTy::GenericArray)) + } + _ => None, + } +} + /// True if signal is absent from codegen (only when surrounding class is excluded). pub fn is_signal_deleted(_class_name: &TyName, signal: &JsonSignal) -> bool { // If any argument type (a class) is excluded. diff --git a/godot-core/src/builtin/collections/array.rs b/godot-core/src/builtin/collections/array.rs index cdbd5ba5d..4d263e27b 100644 --- a/godot-core/src/builtin/collections/array.rs +++ b/godot-core/src/builtin/collections/array.rs @@ -533,12 +533,11 @@ impl Array { /// To create a deep copy, use [`duplicate_deep()`][Self::duplicate_deep] instead. /// To create a new reference to the same array data, use [`clone()`][Clone::clone]. pub fn duplicate_shallow(&self) -> Self { - // SAFETY: We never write to the duplicated array, and all values read are read as `Variant`. - let duplicate: VariantArray = unsafe { self.as_inner().duplicate(false) }; + // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type + let duplicate: Self = unsafe { self.as_inner().duplicate(false) }; - // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type. - let result = unsafe { duplicate.assume_type() }; - result.with_cache(self) + // Note: cache is being set while initializing the duplicate as a return value for above call. + duplicate } /// Returns a deep copy of the array. All nested arrays and dictionaries are duplicated and @@ -548,12 +547,11 @@ impl Array { /// To create a shallow copy, use [`duplicate_shallow()`][Self::duplicate_shallow] instead. /// To create a new reference to the same array data, use [`clone()`][Clone::clone]. pub fn duplicate_deep(&self) -> Self { - // SAFETY: We never write to the duplicated array, and all values read are read as `Variant`. - let duplicate: VariantArray = unsafe { self.as_inner().duplicate(true) }; + // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type + let duplicate: Self = unsafe { self.as_inner().duplicate(true) }; - // SAFETY: duplicate() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type. - let result = unsafe { duplicate.assume_type() }; - result.with_cache(self) + // Note: cache is being set while initializing the duplicate as a return value for above call. + duplicate } /// Returns a sub-range `begin..end` as a new `Array`. @@ -588,13 +586,10 @@ impl Array { let (begin, end) = range.signed(); let end = end.unwrap_or(i32::MAX as i64); - // SAFETY: The type of the array is `T` and we convert the returned array to an `Array` immediately. - let subarray: VariantArray = - unsafe { self.as_inner().slice(begin, end, step as i64, deep) }; + // SAFETY: slice() returns a typed array with the same type as Self, and all values are taken from `self` so have the right type. + let subarray: Self = unsafe { self.as_inner().slice(begin, end, step as i64, deep) }; - // SAFETY: slice() returns a typed array with the same type as Self. - let result = unsafe { subarray.assume_type() }; - result.with_cache(self) + subarray } /// Returns an iterator over the elements of the `Array`. Note that this takes the array @@ -950,8 +945,7 @@ impl Array { } } - /// Changes the generic type on this array, without changing its contents. Needed for API - /// functions that return a variant array even though we know its type, and for API functions + /// Changes the generic type on this array, without changing its contents. Needed for API functions /// that take a variant array even though we want to pass a typed one. /// /// # Safety @@ -966,13 +960,6 @@ impl Array { /// Note also that any `GodotType` can be written to a `Variant` array. /// /// In the current implementation, both cases will produce a panic rather than undefined behavior, but this should not be relied upon. - unsafe fn assume_type(self) -> Array { - // The memory layout of `Array` does not depend on `T`. - std::mem::transmute::, Array>(self) - } - - /// # Safety - /// See [`assume_type`](Self::assume_type). unsafe fn assume_type_ref(&self) -> &Array { // The memory layout of `Array` does not depend on `T`. std::mem::transmute::<&Array, &Array>(self) diff --git a/itest/rust/src/builtin_tests/containers/array_test.rs b/itest/rust/src/builtin_tests/containers/array_test.rs index ea8e3de98..ea8fc7a5a 100644 --- a/itest/rust/src/builtin_tests/containers/array_test.rs +++ b/itest/rust/src/builtin_tests/containers/array_test.rs @@ -677,6 +677,25 @@ func make_array() -> Array[CustomScriptForArrays]: assert_eq!(script.get_global_name(), "CustomScriptForArrays".into()); } +// Test that proper type has been set&cached while creating new Array. +// https://github.com/godot-rust/gdext/pull/1357 +#[itest] +fn array_inner_type() { + let primary = Array::::new(); + + let secondary = primary.duplicate_shallow(); + assert_eq!(secondary.element_type(), primary.element_type()); + + let secondary = primary.duplicate_deep(); + assert_eq!(secondary.element_type(), primary.element_type()); + + let subarray = primary.subarray_deep(.., None); + assert_eq!(subarray.element_type(), primary.element_type()); + + let subarray = primary.subarray_shallow(.., None); + assert_eq!(subarray.element_type(), primary.element_type()); +} + // ---------------------------------------------------------------------------------------------------------------------------------------------- // Class definitions