Skip to content

Add CallError, try_* varcalls and improve error diagnostics #634

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

Merged
merged 5 commits into from
Feb 29, 2024
Merged
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
20 changes: 14 additions & 6 deletions godot-codegen/src/generator/central_files.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,26 @@ pub fn make_core_central_code(api: &ExtensionApi, ctx: &mut Context) -> TokenStr
)*
}

#[cfg(FALSE)]
impl FromVariant for VariantDispatch {
fn try_from_variant(variant: &Variant) -> Result<Self, VariantConversionError> {
let dispatch = match variant.get_type() {
impl VariantDispatch {
pub fn from_variant(variant: &Variant) -> Self {
match variant.get_type() {
VariantType::Nil => Self::Nil,
#(
VariantType::#variant_ty_enumerators_pascal
=> Self::#variant_ty_enumerators_pascal(variant.to::<#variant_ty_enumerators_rust>()),
)*
};
}
}
}

Ok(dispatch)
impl std::fmt::Debug for VariantDispatch {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
Self::Nil => write!(f, "null"),
#(
Self::#variant_ty_enumerators_pascal(v) => write!(f, "{v:?}"),
)*
}
}
}

Expand Down
74 changes: 58 additions & 16 deletions godot-codegen/src/generator/functions_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,15 +101,16 @@ pub fn make_function_definition(
// to only use `unsafe` for pointers in parameters (for outbound calls), and in return values (for virtual calls). Or technically more
// correct, make the entire trait unsafe as soon as one function can return pointers, but that's very unergonomic and non-local.
// Thus, let's keep things simple and more conservative.
let (maybe_unsafe, safety_doc) = if let Some(safety_doc) = safety_doc {
let (maybe_unsafe, maybe_safety_doc) = if let Some(safety_doc) = safety_doc {
(quote! { unsafe }, safety_doc)
} else if function_uses_pointers(sig) {
(
quote! { unsafe },
quote! {
/// # Safety
///
/// Godot currently does not document safety requirements on this method. Make sure you understand the underlying semantics.
/// This method has automatically been marked `unsafe` because it accepts raw pointers as parameters.
/// If Godot does not document any safety requirements, make sure you understand the underlying semantics.
},
)
} else {
Expand Down Expand Up @@ -143,7 +144,7 @@ pub fn make_function_definition(
// Virtual functions

quote! {
#safety_doc
#maybe_safety_doc
#maybe_unsafe fn #primary_fn_name(
#receiver_param
#( #params, )*
Expand All @@ -157,20 +158,61 @@ pub fn make_function_definition(
// If the return type is not Variant, then convert to concrete target type
let varcall_invocation = &code.varcall_invocation;

// TODO use Result instead of panic on error
quote! {
#safety_doc
#vis #maybe_unsafe fn #primary_fn_name(
#receiver_param
#( #params, )*
varargs: &[Variant]
) #return_decl {
type CallSig = #call_sig;
// TODO Utility functions: update as well.
if code.receiver.param.is_empty() {
quote! {
#maybe_safety_doc
#vis #maybe_unsafe fn #primary_fn_name(
#receiver_param
#( #params, )*
varargs: &[Variant]
) #return_decl {
type CallSig = #call_sig;

let args = (#( #arg_names, )*);

unsafe {
#varcall_invocation
}
}
}
} else {
let try_return_decl = &sig.return_value().call_result_decl();
let try_fn_name = format_ident!("try_{}", rust_function_name_str);

let args = (#( #arg_names, )*);
// Note: all varargs functions are non-static, which is why there are some shortcuts in try_*() argument forwarding.
// This can be made more complex if ever necessary.

unsafe {
#varcall_invocation
quote! {
/// # Panics
/// This is a _varcall_ method, meaning parameters and return values are passed as `Variant`.
/// It can detect call failures and will panic in such a case.
#maybe_safety_doc
#vis #maybe_unsafe fn #primary_fn_name(
#receiver_param
#( #params, )*
varargs: &[Variant]
) #return_decl {
Self::#try_fn_name(self, #( #arg_names, )* varargs)
.unwrap_or_else(|e| panic!("{e}"))
}

/// # Return type
/// This is a _varcall_ method, meaning parameters and return values are passed as `Variant`.
/// It can detect call failures and will return `Err` in such a case.
#maybe_safety_doc
#vis #maybe_unsafe fn #try_fn_name(
#receiver_param
#( #params, )*
varargs: &[Variant]
) #try_return_decl {
type CallSig = #call_sig;

let args = (#( #arg_names, )*);

unsafe {
#varcall_invocation
}
}
}
}
Expand All @@ -189,7 +231,7 @@ pub fn make_function_definition(
};

quote! {
#safety_doc
#maybe_safety_doc
#vis #maybe_unsafe fn #primary_fn_name(
#receiver_param
#( #params, )*
Expand Down
5 changes: 5 additions & 0 deletions godot-codegen/src/models/domain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -594,6 +594,11 @@ impl FnReturn {
}
}
}

pub fn call_result_decl(&self) -> TokenStream {
let ret = self.type_tokens();
quote! { -> Result<#ret, crate::builtin::meta::CallError> }
}
}

// ----------------------------------------------------------------------------------------------------------------------------------------------
Expand Down
2 changes: 1 addition & 1 deletion godot-core/src/builtin/array.rs
Original file line number Diff line number Diff line change
Expand Up @@ -948,7 +948,7 @@ impl<T: GodotType> GodotFfiVariant for Array<T> {
if variant.get_type() != Self::variant_type() {
return Err(FromVariantError::BadType {
expected: Self::variant_type(),
got: variant.get_type(),
actual: variant.get_type(),
}
.into_error(variant));
}
Expand Down
Loading