Merge pull request #408 from godot-rust/qol/indexed-method-tables

Array-based function pointer tables

Author: Bromeon

godot-codegen/src/central_generator.rs

@@ -11,12 +11,12 @@ use quote::{format_ident, quote, ToTokens};
 use std::path::Path;
 
 use crate::api_parser::*;
-use crate::central_generator::{collect_builtin_types, BuiltinTypeInfo};
+use crate::central_generator::collect_builtin_types;
 use crate::context::NotificationEnum;
 use crate::util::{
     ident, make_string_name, option_as_slice, parse_native_structures_format, safe_ident,
-    to_pascal_case, to_rust_expr, to_rust_type, to_rust_type_abi, to_snake_case,
-    NativeStructuresField,
+    to_pascal_case, to_rust_expr, to_rust_type, to_rust_type_abi, to_snake_case, ClassCodegenLevel,
+    MethodTableKey, NativeStructuresField,
 };
 use crate::{
     codegen_special_cases, special_cases, util, Context, GeneratedBuiltin, GeneratedBuiltinModule,
@@ -280,27 +280,26 @@ pub(crate) fn generate_builtin_class_files(
     let mut modules = vec![];
     for class in api.builtin_classes.iter() {
         let module_name = ModName::from_godot(&class.name);
-        let class_name = TyName::from_godot(&class.name);
-        let inner_class_name = TyName::from_godot(&format!("Inner{}", class.name));
+        let builtin_name = TyName::from_godot(&class.name);
+        let inner_builtin_name = TyName::from_godot(&format!("Inner{}", class.name));
 
-        if special_cases::is_builtin_type_deleted(&class_name) {
+        if special_cases::is_builtin_type_deleted(&builtin_name) {
             continue;
         }
 
-        let type_info = builtin_types_map
+        let _type_info = builtin_types_map
             .get(&class.name)
             .unwrap_or_else(|| panic!("builtin type not found: {}", class.name));
 
-        let generated_class =
-            make_builtin_class(class, &class_name, &inner_class_name, type_info, ctx);
+        let generated_class = make_builtin_class(class, &builtin_name, &inner_builtin_name, ctx);
         let file_contents = generated_class.code;
 
         let out_path = gen_path.join(format!("{}.rs", module_name.rust_mod));
 
         submit_fn(out_path, file_contents);
 
         modules.push(GeneratedBuiltinModule {
-            class_name: inner_class_name,
+            class_name: inner_builtin_name,
             module_name,
         });
     }
@@ -494,17 +493,12 @@ fn make_class(class: &Class, class_name: &TyName, ctx: &mut Context) -> Generate
     };
 
     let constructor = make_constructor(class, ctx);
-    let get_method_table = util::get_api_level(class).table_global_getter();
+    let api_level = util::get_api_level(class);
 
     let FnDefinitions {
         functions: methods,
         builders,
-    } = make_methods(
-        option_as_slice(&class.methods),
-        class_name,
-        &get_method_table,
-        ctx,
-    );
+    } = make_methods(option_as_slice(&class.methods), class_name, &api_level, ctx);
 
     let enums = make_enums(option_as_slice(&class.enums), class_name, ctx);
     let constants = make_constants(option_as_slice(&class.constants), class_name, ctx);
@@ -806,9 +800,8 @@ fn workaround_constant_collision(all_constants: &mut Vec<(Ident, i32)>) {
 
 fn make_builtin_class(
     class: &BuiltinClass,
-    class_name: &TyName,
+    builtin_name: &TyName,
     inner_class_name: &TyName,
-    type_info: &BuiltinTypeInfo,
     ctx: &mut Context,
 ) -> GeneratedBuiltin {
     let outer_class = if let RustTy::BuiltinIdent(ident) = to_rust_type(&class.name, None, ctx) {
@@ -830,14 +823,13 @@ fn make_builtin_class(
         builders,
     } = make_builtin_methods(
         option_as_slice(&class.methods),
-        class_name,
+        builtin_name,
         inner_class_name,
-        type_info,
         ctx,
     );
 
-    let enums = make_enums(&class_enums, class_name, ctx);
-    let special_constructors = make_special_builtin_methods(class_name, ctx);
+    let enums = make_enums(&class_enums, builtin_name, ctx);
+    let special_constructors = make_special_builtin_methods(builtin_name, ctx);
 
     // mod re_export needed, because class should not appear inside the file module, and we can't re-export private struct as pub
     let code = quote! {
@@ -1017,26 +1009,27 @@ fn make_builtin_module_file(classes_and_modules: Vec<GeneratedBuiltinModule>) ->
 fn make_methods(
     methods: &[ClassMethod],
     class_name: &TyName,
-    get_method_table: &Ident,
+    api_level: &ClassCodegenLevel,
     ctx: &mut Context,
 ) -> FnDefinitions {
-    let definitions = methods
-        .iter()
-        .map(|method| make_method_definition(method, class_name, get_method_table, ctx));
+    let get_method_table = api_level.table_global_getter();
+
+    let definitions = methods.iter().map(|method| {
+        make_method_definition(method, class_name, api_level, &get_method_table, ctx)
+    });
 
     FnDefinitions::expand(definitions)
 }
 
 fn make_builtin_methods(
     methods: &[BuiltinClassMethod],
-    class_name: &TyName,
+    builtin_name: &TyName,
     inner_class_name: &TyName,
-    type_info: &BuiltinTypeInfo,
     ctx: &mut Context,
 ) -> FnDefinitions {
-    let definitions = methods.iter().map(|method| {
-        make_builtin_method_definition(method, class_name, inner_class_name, type_info, ctx)
-    });
+    let definitions = methods
+        .iter()
+        .map(|method| make_builtin_method_definition(method, builtin_name, inner_class_name, ctx));
 
     FnDefinitions::expand(definitions)
 }
@@ -1082,12 +1075,11 @@ fn make_special_builtin_methods(class_name: &TyName, _ctx: &Context) -> TokenStr
 fn make_method_definition(
     method: &ClassMethod,
     class_name: &TyName,
+    api_level: &ClassCodegenLevel,
     get_method_table: &Ident,
     ctx: &mut Context,
 ) -> FnDefinition {
-    if codegen_special_cases::is_method_excluded(method, false, ctx)
-        || special_cases::is_deleted(class_name, &method.name)
-    {
+    if special_cases::is_deleted(class_name, method, ctx) {
         return FnDefinition::none();
     }
     /*if method.map_args(|args| args.is_empty()) {
@@ -1119,10 +1111,14 @@ fn make_method_definition(
         quote! { sys::interface_fn!(object_method_bind_ptrcall) }
     };
 
-    let fn_ptr = util::make_class_method_ptr_name(&class_name.godot_ty, method);
+    let table_index = ctx.get_table_index(&MethodTableKey::ClassMethod {
+        api_level: *api_level,
+        class_ty: class_name.clone(),
+        method_name: method.name.clone(),
+    });
 
     let init_code = quote! {
-        let __method_bind = sys::#get_method_table().#fn_ptr;
+        let __method_bind = sys::#get_method_table().fptr_by_index(#table_index);
         let __call_fn = #function_provider;
     };
 
@@ -1156,12 +1152,11 @@ fn make_method_definition(
 
 fn make_builtin_method_definition(
     method: &BuiltinClassMethod,
-    class_name: &TyName,
+    builtin_name: &TyName,
     inner_class_name: &TyName,
-    type_info: &BuiltinTypeInfo,
     ctx: &mut Context,
 ) -> FnDefinition {
-    if codegen_special_cases::is_builtin_method_excluded(method) {
+    if special_cases::is_builtin_deleted(builtin_name, method) {
         return FnDefinition::none();
     }
 
@@ -1175,10 +1170,13 @@ fn make_builtin_method_definition(
     let is_varcall = method.is_vararg;
     let variant_ffi = is_varcall.then(VariantFfi::type_ptr);
 
-    let fn_ptr = util::make_builtin_method_ptr_name(&type_info.type_names, method);
+    let table_index = ctx.get_table_index(&MethodTableKey::BuiltinMethod {
+        builtin_ty: builtin_name.clone(),
+        method_name: method.name.clone(),
+    });
 
     let init_code = quote! {
-        let __call_fn = sys::builtin_method_table().#fn_ptr;
+        let __call_fn = sys::builtin_method_table().fptr_by_index(#table_index);
     };
 
     let receiver = make_receiver(method.is_static, method.is_const, quote! { self.sys_ptr });
@@ -1191,7 +1189,7 @@ fn make_builtin_method_definition(
         &FnSignature {
             function_name: method_name_str,
             surrounding_class: Some(inner_class_name),
-            is_private: special_cases::is_private(class_name, &method.name),
+            is_private: special_cases::is_private(builtin_name, &method.name),
             is_virtual: false,
             qualifier: FnQualifier::for_method(method.is_const, method.is_static),
 

godot-codegen/src/class_generator.rs

@@ -4,8 +4,9 @@
  * file, You can obtain one at https://mozilla.org/MPL/2.0/.
  */
 
-use crate::api_parser::Class;
-use crate::{codegen_special_cases, util, ExtensionApi, GodotTy, RustTy, TyName};
+use crate::api_parser::{BuiltinClass, BuiltinClassMethod, Class, ClassConstant, ClassMethod};
+use crate::util::{option_as_slice, MethodTableKey};
+use crate::{codegen_special_cases, special_cases, util, ExtensionApi, GodotTy, RustTy, TyName};
 use proc_macro2::{Ident, TokenStream};
 use quote::{format_ident, ToTokens};
 use std::collections::{HashMap, HashSet};
@@ -20,6 +21,8 @@ pub(crate) struct Context<'a> {
     cached_rust_types: HashMap<GodotTy, RustTy>,
     notifications_by_class: HashMap<TyName, Vec<(Ident, i32)>>,
     notification_enum_names_by_class: HashMap<TyName, NotificationEnum>,
+    method_table_indices: HashMap<MethodTableKey, usize>,
+    method_table_next_index: HashMap<String, usize>,
 }
 
 impl<'a> Context<'a> {
@@ -34,6 +37,12 @@ impl<'a> Context<'a> {
         for builtin in api.builtin_classes.iter() {
             let ty_name = builtin.name.as_str();
             ctx.builtin_types.insert(ty_name);
+
+            Self::populate_builtin_class_table_indices(
+                builtin,
+                option_as_slice(&builtin.methods),
+                &mut ctx,
+            );
         }
 
         for structure in api.native_structures.iter() {
@@ -60,31 +69,17 @@ impl<'a> Context<'a> {
             }
 
             // Populate notification constants (first, only for classes that declare them themselves).
-            if let Some(constants) = class.constants.as_ref() {
-                let mut has_notifications = false;
-
-                for constant in constants.iter() {
-                    if let Some(rust_constant) = util::try_to_notification(constant) {
-                        // First time
-                        if !has_notifications {
-                            ctx.notifications_by_class
-                                .insert(class_name.clone(), Vec::new());
-
-                            ctx.notification_enum_names_by_class.insert(
-                                class_name.clone(),
-                                NotificationEnum::for_own_class(&class_name),
-                            );
-
-                            has_notifications = true;
-                        }
-
-                        ctx.notifications_by_class
-                            .get_mut(&class_name)
-                            .expect("just inserted constants; must be present")
-                            .push((rust_constant, constant.value));
-                    }
-                }
-            }
+            Self::populate_notification_constants(
+                &class_name,
+                option_as_slice(&class.constants),
+                &mut ctx,
+            );
+            Self::populate_class_table_indices(
+                class,
+                &class_name,
+                option_as_slice(&class.methods),
+                &mut ctx,
+            );
         }
 
         // Populate remaining notification enum names, by copying the one to nearest base class that has at least 1 notification.
@@ -129,13 +124,110 @@ impl<'a> Context<'a> {
         ctx
     }
 
+    fn populate_notification_constants(
+        class_name: &TyName,
+        constants: &[ClassConstant],
+        ctx: &mut Context,
+    ) {
+        let mut has_notifications = false;
+        for constant in constants.iter() {
+            if let Some(rust_constant) = util::try_to_notification(constant) {
+                // First time
+                if !has_notifications {
+                    ctx.notifications_by_class
+                        .insert(class_name.clone(), Vec::new());
+
+                    ctx.notification_enum_names_by_class.insert(
+                        class_name.clone(),
+                        NotificationEnum::for_own_class(class_name),
+                    );
+
+                    has_notifications = true;
+                }
+
+                ctx.notifications_by_class
+                    .get_mut(class_name)
+                    .expect("just inserted constants; must be present")
+                    .push((rust_constant, constant.value));
+            }
+        }
+    }
+
+    fn populate_class_table_indices(
+        class: &Class,
+        class_name: &TyName,
+        methods: &[ClassMethod],
+        ctx: &mut Context,
+    ) {
+        if special_cases::is_class_deleted(class_name) {
+            return;
+        }
+
+        for method in methods.iter() {
+            if special_cases::is_deleted(class_name, method, ctx) {
+                continue;
+            }
+
+            let key = MethodTableKey::ClassMethod {
+                api_level: util::get_api_level(class),
+                class_ty: class_name.clone(),
+                method_name: method.name.clone(),
+            };
+
+            ctx.register_table_index(key);
+        }
+    }
+
+    fn populate_builtin_class_table_indices(
+        builtin: &BuiltinClass,
+        methods: &[BuiltinClassMethod],
+        ctx: &mut Context,
+    ) {
+        let builtin_ty = TyName::from_godot(builtin.name.as_str());
+        if special_cases::is_builtin_type_deleted(&builtin_ty) {
+            return;
+        }
+
+        for method in methods.iter() {
+            if special_cases::is_builtin_deleted(&builtin_ty, method) {
+                continue;
+            }
+
+            let key = MethodTableKey::BuiltinMethod {
+                builtin_ty: builtin_ty.clone(),
+                method_name: method.name.clone(),
+            };
+
+            ctx.register_table_index(key);
+        }
+    }
+
     pub fn get_engine_class(&self, class_name: &TyName) -> &Class {
         self.engine_classes.get(class_name).unwrap()
     }
 
-    // pub fn is_engine_class(&self, class_name: &str) -> bool {
-    //     self.engine_classes.contains(class_name)
-    // }
+    // Private, because initialized in constructor. Ensures deterministic assignment.
+    fn register_table_index(&mut self, key: MethodTableKey) -> usize {
+        let key_category = key.category();
+
+        let next_index = self
+            .method_table_next_index
+            .entry(key_category)
+            .or_insert(0);
+
+        let prev = self.method_table_indices.insert(key, *next_index);
+        assert!(prev.is_none(), "table index already registered");
+
+        *next_index += 1;
+        *next_index
+    }
+
+    pub fn get_table_index(&self, key: &MethodTableKey) -> usize {
+        *self
+            .method_table_indices
+            .get(key)
+            .unwrap_or_else(|| panic!("did not register table index for key {:?}", key))
+    }
 
     /// Checks if this is a builtin type (not `Object`).
     ///