-
-
Notifications
You must be signed in to change notification settings - Fork 222
Remove paste, simplify plugin macros #1069
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
Conversation
No longer mangle the macro name. Since there are no more proc-macros (paste) involved, this should also speed up compile times in case of hundreds of class or #[itest] registrations.
180cc06
to
fb0d61c
Compare
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1069 |
API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1069 |
// As such, instead we export a fn with a random name of predictable format to be used | ||
// by the embedder. | ||
$crate::paste::paste! { | ||
#[no_mangle] | ||
extern "C" fn [< rust_gdext_registrant_ $gensym >] () { | ||
__init(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was paste not still necessary here? We need a prefix with which to search for registered functions at init time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If needed, we can use pastey
instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good catch! This is the problem when the comments aren't expressive: "a random name of predictable format" didn't make me think that the concrete prefix mattered.
If needed, we can use
pastey
instead.
I don't simply trust random crates, and this one in particular had an incident where it just replaced all commits of the previous author. That after opening the RustSec advisory for paste, triggering the migration in the first place. Not a great start.
I'll see if I find another solution. Maybe a proc-macro, then we can get rid of gensym
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for checking. Let's do it ourselves then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in #1092.
Removes the
paste
crate, which is no longer maintained and started triggering cargo-deny alerts.The "plugin" macros are now simpler: they no longer mangle the registry name. Since there are no more proc-macros (
paste
) involved, this should also speed up compile times in case of hundreds of class or#[itest]
registrations.(I originally tried to do something very sophisticated with proc-macros, but abandoned ship as it grew too complex and I realized it not only wouldn't add value here, but actively make compilation slower. If ever we need those elsewhere, some efforts are preserved in e2aba76.)