Skip to content

Conversation

@mivort
Copy link
Contributor

@mivort mivort commented Nov 4, 2025

This PR adds a change which links together identifiers in quote! expansion. While in normal context, there's no issues with expansion of procedural macro #[func] attribute, Rust macro hygiene rules start to apply within the context of eager2 crate.

Context/use case

I'm using eager2 crate to implement makeshift mixin pattern for exported GDExtension classes. This allows to define 'interface' as macro once, and then apply the same #[func]/#[signal]/#[property] 'signature' to multiple exported classes, embedding it as tokens (so #[godot_api] is able to catch the #[func] attribute).

#[eager_macro]
macro_rules! common_method {
    () => {
        #[func]
        fn common_method(&self) { self.common_trait_method() }
    };
}

eager! {
    #[godot_api]
    impl Foo {
        common_method!()
    }
    #[godot_api]
    impl Bar {
        common_method!()
    }
}

Even beyond this use case, eager2 macro is overall infinitely useful and powerful, as it allows to define and perform eager macro expansion (similar to built-in ones, such as concat) - there's a chance that similar functionality will make it into 'declarative macros 2.0' proposal in Rust language some time later.

However, recent changes to the #[func] attribute expansion started causing this type of error:

error[E0425]: cannot find value `params` in this scope
  --> [...]:34:1
   |
34 | / eager! {
35 | |     #[godot_api]
36 | |     impl Foo {
37 | |         common_method!()
...  |
40 | | }
   | |_^ not found in this scope
   |
   = note: this error originates in the macro `interface_equipment` which comes from the expansion of the macr
o `eager` (in Nightly builds, run with -Z macro-backtrace for more info)

So during the expansion of #[godot_api] within eager context, hygiene rule for params trigger, preventing the compiler from matching the identifier referenced with #param_ident to params. Use of consistent identifier reference (#param_ident in both argument list and function body, with same span) prevents this issue and allows to keep using the eager expansion.

@GodotRust
Copy link

API docs are being generated and will be shortly available at: https://godot-rust.github.io/docs/gdext/pr-1397

@Bromeon Bromeon added bug c: register Register classes, functions and other symbols to GDScript labels Nov 4, 2025
@Bromeon
Copy link
Member

Bromeon commented Nov 4, 2025

Thanks a lot, great discovery and super nice that it's so simple to fix. Span improvements are always appreciated; we started with #1370 and #1373, but there is a lot left to do 🙂

Could you add a comment about macro hygiene (and maybe a link to this PR) above the changed lines, so that we don't risk accidentally re-introducing it again? Please squash commits with existing one.

Do you think there's a way to test this without depending on eager or another third-party crate?

@mivort mivort force-pushed the fix-eager-macro-hygiene-error branch from 401e0f3 to 5bcb419 Compare November 4, 2025 13:25
This PR adds a change which links together identifiers in `quote!`
expansion. While in normal context, there's no issues with expansion of
procedural macro `#[func]` attribute, Rust macro hygiene rules start to
apply within the context of `eager2` crate.
@mivort mivort force-pushed the fix-eager-macro-hygiene-error branch from 5bcb419 to 7b9b5cc Compare November 4, 2025 13:26
@mivort
Copy link
Contributor Author

mivort commented Nov 4, 2025

@Bromeon Added a comment about the requirement to provide spans to the identifiers with a link to this PR for context, is the explanation for it correct?

I haven't reproduce the issue outside of eager crate usage, but I believe a small subset of its functionality (a minimal #[proc_macro]) may be able to trigger the hygiene issue. I'll try with other macros and this change reverted to see if there's a simpler way to get this error.

Copy link
Member

@Bromeon Bromeon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Bromeon Bromeon added this pull request to the merge queue Nov 4, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 4, 2025
@Bromeon
Copy link
Member

Bromeon commented Nov 4, 2025

godot-itest (macos-x86-4.4)
This is a scheduled macos-13 brownout. The macOS-13 based runner images are being deprecated. For more details, see actions/runner-images#13046.

OK, that's probably the end of the line for our macOS Intel support 🥲

@Bromeon Bromeon added this pull request to the merge queue Nov 5, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Nov 5, 2025
@Bromeon Bromeon added this pull request to the merge queue Nov 8, 2025
Merged via the queue into godot-rust:master with commit 6ffe8a6 Nov 8, 2025
20 checks passed
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants