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

Some cleanup and more accurate tracking of safety invariants #603

Merged
merged 3 commits into from
Feb 20, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 0 additions & 11 deletions godot-codegen/src/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -246,17 +246,6 @@ impl<'a> Context<'a> {
self.singletons.contains(class_name)
}

pub fn is_exportable(&self, class_name: &TyName) -> bool {
if class_name.godot_ty == "Resource" || class_name.godot_ty == "Node" {
return true;
}

self.inheritance_tree
.collect_all_bases(class_name)
.iter()
.any(|ty| ty.godot_ty == "Resource" || ty.godot_ty == "Node")
}

pub fn inheritance_tree(&self) -> &InheritanceTree {
&self.inheritance_tree
}
Expand Down
18 changes: 18 additions & 0 deletions godot-codegen/src/generator/builtins.rs
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,21 @@ 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).
});
}

None
}

fn make_builtin_method_definition(
builtin_class: &BuiltinClass,
method: &BuiltinMethod,
Expand Down Expand Up @@ -220,12 +235,15 @@ fn make_builtin_method_definition(
)*/
};

let safety_doc = method_safety_doc(builtin_class.name(), method);

functions_common::make_function_definition(
method,
&FnCode {
receiver,
varcall_invocation,
ptrcall_invocation,
},
safety_doc,
)
}
57 changes: 22 additions & 35 deletions godot-codegen/src/generator/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
let base = ident(&conv::to_pascal_case(base));
(quote! { crate::engine::#base }, Some(base))
}
None => (quote! { () }, None),
None => (quote! { crate::obj::NoBase }, None),
};

let (constructor, godot_default_impl) = make_constructor_and_default(class, ctx);
Expand All @@ -100,8 +100,7 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas

let enums = enums::make_enums(&class.enums);
let constants = constants::make_constants(&class.constants);
let inherits_macro = format_ident!("inherits_transitive_{}", class_name.rust_ty);
let (exportable_impl, exportable_macro_impl) = make_exportable_impl(class_name, ctx);
let inherits_macro = format_ident!("unsafe_inherits_transitive_{}", class_name.rust_ty);
let deref_impl = make_deref_impl(class_name, &base_ty);

let all_bases = ctx.inheritance_tree().collect_all_bases(class_name);
Expand Down Expand Up @@ -140,8 +139,18 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
let instance_id = rtti.check_type::<Self>();
Some(instance_id)
}

#[doc(hidden)]
pub fn __object_ptr(&self) -> sys::GDExtensionObjectPtr {
self.object_ptr
}
};

let inherits_macro_safety_doc = format!(
"The provided class must be a subclass of all the superclasses of [`{}`]",
class_name.rust_ty
);

// mod re_export needed, because class should not appear inside the file module, and we can't re-export private struct as pub.
let imports = util::make_imports();
let tokens = quote! {
Expand Down Expand Up @@ -187,31 +196,26 @@ fn make_class(class: &Class, ctx: &mut Context, view: &ApiView) -> GeneratedClas
type DynMemory = crate::obj::bounds::#assoc_dyn_memory;
type Declarer = crate::obj::bounds::DeclEngine;
}
impl crate::obj::EngineClass for #class_name {
fn as_object_ptr(&self) -> sys::GDExtensionObjectPtr {
self.object_ptr
}
fn as_type_ptr(&self) -> sys::GDExtensionTypePtr {
std::ptr::addr_of!(self.object_ptr) as sys::GDExtensionTypePtr
}
}

#(
impl crate::obj::Inherits<crate::engine::#all_bases> for #class_name {}
// SAFETY: #all_bases is a list of classes provided by Godot such that #class_name is guaranteed a subclass of all of them.
unsafe impl crate::obj::Inherits<crate::engine::#all_bases> for #class_name {}
)*

#exportable_impl
#godot_default_impl
#deref_impl

/// # Safety
///
#[doc = #inherits_macro_safety_doc]
#[macro_export]
#[allow(non_snake_case)]
macro_rules! #inherits_macro {
($Class:ident) => {
impl ::godot::obj::Inherits<::godot::engine::#class_name> for $Class {}
unsafe impl ::godot::obj::Inherits<::godot::engine::#class_name> for $Class {}
#(
impl ::godot::obj::Inherits<::godot::engine::#all_bases> for $Class {}
unsafe impl ::godot::obj::Inherits<::godot::engine::#all_bases> for $Class {}
)*
#exportable_macro_impl
}
}
}
Expand Down Expand Up @@ -351,26 +355,8 @@ fn make_constructor_and_default(class: &Class, ctx: &Context) -> (TokenStream, T
(constructor, godot_default_impl)
}

fn make_exportable_impl(class_name: &TyName, ctx: &mut Context) -> (TokenStream, TokenStream) {
let (exportable_impl, exportable_macro_impl);

if ctx.is_exportable(class_name) {
exportable_impl = quote! {
impl crate::obj::ExportableObject for #class_name {}
};
exportable_macro_impl = quote! {
impl ::godot::obj::ExportableObject for $Class {}
};
} else {
exportable_impl = TokenStream::new();
exportable_macro_impl = TokenStream::new();
};

(exportable_impl, exportable_macro_impl)
}

fn make_deref_impl(class_name: &TyName, base_ty: &TokenStream) -> TokenStream {
// The base_ty of `Object` is `()`, and we dont want every engine class to deref to `()`.
// The base_ty of `Object` is `NoBase`, and we dont want every engine class to deref to `NoBase`.
if class_name.rust_ty == "Object" {
return TokenStream::new();
}
Expand Down Expand Up @@ -493,5 +479,6 @@ fn make_class_method_definition(
varcall_invocation,
ptrcall_invocation,
},
None,
)
}
10 changes: 8 additions & 2 deletions godot-codegen/src/generator/functions_common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ impl FnDefinitions {
}
}

pub fn make_function_definition(sig: &dyn Function, code: &FnCode) -> FnDefinition {
pub fn make_function_definition(
sig: &dyn Function,
code: &FnCode,
safety_doc: Option<TokenStream>,
) -> FnDefinition {
let has_default_params = default_parameters::function_uses_default_params(sig);
let vis = if has_default_params {
// Public API mapped by separate function.
Expand All @@ -92,7 +96,9 @@ pub fn make_function_definition(sig: &dyn Function, code: &FnCode) -> FnDefiniti
make_vis(sig.is_private())
};

let (maybe_unsafe, safety_doc) = if function_uses_pointers(sig) {
let (maybe_unsafe, safety_doc) = if let Some(safety_doc) = safety_doc {
(quote! { unsafe }, safety_doc)
} else if function_uses_pointers(sig) {
(
quote! { unsafe },
quote! {
Expand Down
1 change: 1 addition & 0 deletions godot-codegen/src/generator/utility_functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ pub(crate) fn make_utility_function_definition(function: &UtilityFunction) -> To
varcall_invocation,
ptrcall_invocation,
},
None,
);

// Utility functions have no builders.
Expand Down
1 change: 1 addition & 0 deletions godot-codegen/src/generator/virtual_traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ fn make_virtual_method(method: &ClassMethod) -> Option<TokenStream> {
varcall_invocation: TokenStream::new(),
ptrcall_invocation: TokenStream::new(),
},
None,
);

// Virtual methods have no builders.
Expand Down