Merge pull request #603 from lilizoey/refactor/minor-cleanups

Some cleanup and more accurate tracking of safety invariants

Author: lilizoey

godot-codegen/src/context.rs

@@ -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
     }

godot-codegen/src/generator/builtins.rs

@@ -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,
@@ -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,
     )
 }

godot-codegen/src/generator/classes.rs

@@ -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);
@@ -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);
@@ -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! {
@@ -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
                 }
             }
         }
@@ -342,26 +346,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();
     }
@@ -484,5 +470,6 @@ fn make_class_method_definition(
             varcall_invocation,
             ptrcall_invocation,
         },
+        None,
     )
 }

godot-codegen/src/generator/functions_common.rs

@@ -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.
@@ -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! {

godot-codegen/src/generator/utility_functions.rs

@@ -78,6 +78,7 @@ pub(crate) fn make_utility_function_definition(function: &UtilityFunction) -> To
             varcall_invocation,
             ptrcall_invocation,
         },
+        None,
     );
 
     // Utility functions have no builders.

godot-codegen/src/generator/virtual_traits.rs

@@ -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.