Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 24 additions & 9 deletions godot-codegen/src/generator/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<TokenStream> {
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" {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is quite nice, but does anyone ever see these docs?
I guess the show up in IDE hints? 🤔

Copy link
Contributor Author

@Yarwin Yarwin Oct 8, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(currently) contributors are the only ones seeing it.

Still nice to have and might come in handy one day – I think it might be useful providing some context while debugging some UB (if any ever happens) too

screenshoot from previous version image

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
Expand Down Expand Up @@ -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::<CallParams, CallRet>::out_builtin_ptrcall(
Signature::<CallParams, CallRet #maybe_generic_params>::out_builtin_ptrcall(
method_bind,
#builtin_name_str,
#method_name_str,
Expand Down
13 changes: 2 additions & 11 deletions godot-codegen/src/generator/default_parameters.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand Down Expand Up @@ -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,
);
Comment on lines -62 to -69
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unrelated (the dead code you mentioned), right? Why exactly?

Could you extract it to its own commit so it's easier to isolate the changes in case of regressions?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why exactly?

func_general_lifetime was never set, thus it was always empty – effectively this code was doing nothing.


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,
Expand Down Expand Up @@ -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 {
Expand Down
20 changes: 10 additions & 10 deletions godot-codegen/src/generator/functions_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ pub struct FnParamTokens {
/// Generic argument list `<'a0, 'a1, ...>` after `type CallSig`, if available.
pub callsig_lifetime_args: Option<TokenStream>,
pub arg_exprs: Vec<TokenStream>,
pub func_general_lifetime: Option<TokenStream>,
}

pub fn make_function_definition(
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,)*);
}
};
Expand Down Expand Up @@ -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, )*);
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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> });
Expand Down Expand Up @@ -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)
}
Expand Down
53 changes: 53 additions & 0 deletions godot-codegen/src/models/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Comment on lines +333 to +335
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always empty lines between multi-line fn decls


fn direction(&self) -> FnDirection {
self.common().direction
}
Expand Down Expand Up @@ -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<JsonMethodReturn>,
replacements: EnumReplacements,
Expand Down Expand Up @@ -669,6 +688,14 @@ impl FnReturn {
}
}

pub fn generic_params(&self) -> Option<TokenStream> {
self.type_.as_ref()?.generic_params()
}

pub fn where_clause(&self) -> Option<TokenStream> {
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> }
Expand Down Expand Up @@ -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<T>` 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<RustTy>, is_const: bool },

Expand Down Expand Up @@ -755,10 +787,30 @@ impl RustTy {
pub fn return_decl(&self) -> TokenStream {
match self {
Self::EngineClass { tokens, .. } => quote! { -> Option<#tokens> },
Self::GenericArray => quote! { -> Array<Ret> },
other => quote! { -> #other },
}
}

pub fn generic_params(&self) -> Option<TokenStream> {
if matches!(self, Self::GenericArray) {
Some(quote! { < Ret > })
} else {
None
}
}

pub fn where_clause(&self) -> Option<TokenStream> {
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;
Expand Down Expand Up @@ -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<Ret> }.to_tokens(tokens),
}
}
}
Expand Down
17 changes: 12 additions & 5 deletions godot-codegen/src/models/domain_mapping.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions godot-codegen/src/special_cases/codegen_special_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
20 changes: 19 additions & 1 deletion godot-codegen/src/special_cases/special_cases.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<T>` – 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<FnReturn> {
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.
Expand Down
Loading
Loading