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

can't compile a macro_rules! macro that includes #[godot_api] proc macro with attempt to impl a godot node interface #996

Open
bcolloran opened this issue Jan 1, 2025 · 6 comments · May be fixed by #1008
Labels
bug c: register Register classes, functions and other symbols to GDScript status: upstream Depending on upstream fix (typically Godot)

Comments

@bcolloran
Copy link

Per discord thread here:
https://discord.com/channels/723850269347283004/1192118392660963338/threads/1323828276720046102

Here is a small repro case:

use godot::prelude::*;

trait Registerable {
    fn print_id(&self);
}

fn register_id_on_tree_entry<T: Registerable>(node: &mut T) {
    node.print_id()
}

#[derive(GodotClass)]
#[class(init, base=Node3D)]
pub struct Enemy {
    #[base]
    base: Base<Node3D>,
    id: u32,
}

impl Registerable for Enemy {
    fn print_id(&self) {
        println!("id: {}", self.id);
    }
}

#[macro_export]
macro_rules! impl_register_on_tree_enter_FAILING {
    ($inner_type:ty, $node_interface:path) => {
        #[godot_api]
        impl $node_interface for $inner_type {
            fn enter_tree(&mut self) {
                register_id_on_tree_entry(self);
            }
        }
    };
}

impl_register_on_tree_enter_FAILING!(Enemy, INode3D);

Which fails to compile with this error:

error: invalid Self type for #[godot_api] impl
  --> src/ideas/macro_bug_repro.rs:29:9
   |
29 | /         impl $node_interface for $inner_type {
30 | |             fn enter_tree(&mut self) {
31 | |                 register_id_on_tree_entry(self);
32 | |             }
33 | |         }
   | |_________^
...
37 |   impl_register_on_tree_enter_FAILING!(Enemy, INode3D);
   |   ---------------------------------------------------- in this macro invocation
   |
   = note: this error originates in the macro `impl_register_on_tree_enter_FAILING` (in Nightly builds, run with -Z macro-backtrace for more info)

the error message error: invalid Self type for #[godot_api] impl can be traced to here:

return bail!(decl, "invalid Self type for #[godot_api] impl");

If you expand this in VSCode, and paste it in place of the macro invocation, it compiles without issue. Here is that expanded macro for reference:

// Recursive expansion of impl_register_on_tree_enter_FAILING! macro
// ==================================================================

impl INode3D for Enemy {
    fn enter_tree(&mut self) {
        register_id_on_tree_entry(self);
    }
}
impl ::godot::private::You_forgot_the_attribute__godot_api for Enemy {}

impl ::godot::obj::cap::ImplementsGodotVirtual for Enemy {
    fn __virtual_call(name: &str) -> ::godot::sys::GDExtensionClassCallVirtual {
        use ::godot::obj::UserClass as _;
        match name {
            "_enter_tree" => {
                use ::godot::sys;
                type Sig = ((),);
                unsafe extern "C" fn virtual_fn(
                    instance_ptr: sys::GDExtensionClassInstancePtr,
                    args_ptr: *const sys::GDExtensionConstTypePtr,
                    ret: sys::GDExtensionTypePtr,
                ) {
                    let call_ctx = ::godot::meta::CallContext::func("Enemy", "enter_tree");
                    let _success = ::godot::private::handle_ptrcall_panic(&call_ctx, || {
                        <Sig as ::godot::meta::PtrcallSignatureTuple>::in_ptrcall(
                            instance_ptr,
                            &call_ctx,
                            args_ptr,
                            ret,
                            |instance_ptr, params| {
                                let () = params;
                                let storage =
                                    unsafe { ::godot::private::as_storage::<Enemy>(instance_ptr) };
                                let mut instance = ::godot::private::Storage::get_mut(storage);
                                instance.enter_tree()
                            },
                            sys::PtrcallType::Virtual,
                        )
                    });
                }
                Some(virtual_fn)
            }
            "_ready" => {
                use ::godot::sys;
                type Sig = ((),);
                unsafe extern "C" fn virtual_fn(
                    instance_ptr: sys::GDExtensionClassInstancePtr,
                    args_ptr: *const sys::GDExtensionConstTypePtr,
                    ret: sys::GDExtensionTypePtr,
                ) {
                    let call_ctx = ::godot::meta::CallContext::func("Enemy", "ready");
                    let _success = ::godot::private::handle_ptrcall_panic(&call_ctx, || {
                        <Sig as ::godot::meta::PtrcallSignatureTuple>::in_ptrcall(
                            instance_ptr,
                            &call_ctx,
                            args_ptr,
                            ret,
                            |instance_ptr, params| {
                                let () = params;
                                let storage =
                                    unsafe { ::godot::private::as_storage::<Enemy>(instance_ptr) };
                                let mut instance = ::godot::private::Storage::get_mut(storage);
                                instance.__before_ready();
                            },
                            sys::PtrcallType::Virtual,
                        )
                    });
                }
                Some(virtual_fn)
            }
            _ => None,
        }
    }
}
const _: () = {
    #[allow(non_upper_case_globals)]
    #[used]
    #[cfg_attr(target_os = "windows", link_section = ".CRT$XCU")]
    #[cfg_attr(target_os = "ios", link_section = "__DATA,__mod_init_func")]
    #[cfg_attr(target_os = "macos", link_section = "__DATA,__mod_init_func")]
    #[cfg_attr(target_os = "android", link_section = ".init_array")]
    #[cfg_attr(target_os = "dragonfly", link_section = ".init_array")]
    #[cfg_attr(target_os = "freebsd", link_section = ".init_array")]
    #[cfg_attr(target_os = "linux", link_section = ".init_array")]
    #[cfg_attr(target_os = "netbsd", link_section = ".init_array")]
    #[cfg_attr(target_os = "openbsd", link_section = ".init_array")]
    static __init: extern "C" fn() = {
        #[cfg_attr(target_os = "android", link_section = ".text.startup")]
        #[cfg_attr(target_os = "linux", link_section = ".text.startup")]
        extern "C" fn __inner_init() {
            {
                let mut guard = ::godot::private::__godot_rust_plugin___GODOT_PLUGIN_REGISTRY
                    .lock()
                    .unwrap();
                guard.push(
                    (::godot::private::ClassPlugin {
                        class_name: <Enemy as ::godot::obj::GodotClass>::class_name(),
                        item: ::godot::private::PluginItem::ITraitImpl {
                            user_register_fn: None,
                            user_create_fn: None,
                            user_recreate_fn: None,
                            user_to_string_fn: None,
                            user_on_notification_fn: None,
                            user_set_fn: None,
                            user_get_fn: None,
                            user_get_property_list_fn: None,
                            user_free_property_list_fn: None,
                            user_property_get_revert_fn: None,
                            user_property_can_revert_fn: None,
                            get_virtual_fn: ::godot::private::callbacks::get_virtual::<Enemy>,
                        },
                        init_level: <Enemy as ::godot::obj::GodotClass>::INIT_LEVEL,
                    }),
                );
            }
        }
        __inner_init
    };
    #[cfg(target_family = "wasm")]
    godot_ffi::gensym! {
        godot_ffi::plugin_execute_pre_main_wasm!()
    }
};

@Bromeon also recommended trying the macro without using :ty and :path, and indeed, using :ident instead does compile:

// (... other code from above ...)

#[macro_export]
macro_rules! impl_register_on_tree_enter_WORKING {
    ($inner_type:ident, $node_interface:ident) => {
        #[godot_api]
        impl $node_interface for $inner_type {
            fn enter_tree(&mut self) {
                register_id_on_tree_entry(self);
            }
        }
    };
}

impl_register_on_tree_enter_WORKING!(Enemy, INode3D);
@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Jan 1, 2025
@Bromeon
Copy link
Member

Bromeon commented Jan 1, 2025

Related: #379

@lilizoey
Copy link
Member

lilizoey commented Jan 8, 2025

pretty-printing out what self_ty is in that case i get this:

[
    Group {
        delimiter: None,
        stream: TokenStream [
            Ident {
                ident: "Enemy",
                span: #0 bytes(721..726),
            },
        ],
        span: #27 bytes(548..559),
    },
]

changing it to crate::Enemy i get:

[
    Group {
        delimiter: None,
        stream: TokenStream [
            Ident {
                ident: "crate",
                span: #0 bytes(721..726),
            },
            Punct {
                ch: ':',
                spacing: Joint,
                span: #0 bytes(726..727),
            },
            Punct {
                ch: ':',
                spacing: Alone,
                span: #0 bytes(727..728),
            },
            Ident {
                ident: "Enemy",
                span: #0 bytes(728..733),
            },
        ],
        span: #27 bytes(548..559),
    },
]

so it seems like this is set to a group containing the entire type path with no delimiter for ty macro expressions. which venial doesn't see as a path expression. i feel like this may be a venial bug?

@lilizoey
Copy link
Member

lilizoey commented Jan 8, 2025

yeah looking at syn it has a specific case for this situation: Type::Group. whereas venial doesn't seem to do any special casing here: https://github.com/PoignardAzur/venial/blob/178bb9d493d0f4589d92cb2815a75190f1a8c89d/src/parse_impl.rs#L245

it simply just takes the entire type expression and puts it into a TypeExpr.

@Bromeon
Copy link
Member

Bromeon commented Jan 9, 2025

Oh, so venial would need to special-case the "group with empty delimiter" and unpack its individual elements into a TypeExpr? 🤔

@lilizoey
Copy link
Member

lilizoey commented Jan 9, 2025

Oh, so venial would need to special-case the "group with empty delimiter" and unpack its individual elements into a TypeExpr? 🤔

i think so? im not 100% sure it's the right thing to do, but it definitely would make sense for me to have it work like that. since even though it's technically not a path, it's an invisible delimiter group, it still is a path.

@Bromeon
Copy link
Member

Bromeon commented Jan 11, 2025

Fixed upstream in PoignardAzur/venial#60.

Added test for this in #1008, which will only pass once venial is updated.

@Bromeon Bromeon added the status: upstream Depending on upstream fix (typically Godot) label Jan 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: register Register classes, functions and other symbols to GDScript status: upstream Depending on upstream fix (typically Godot)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants