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

fix: prefix macro calls with ::core to avoid clashing with local macros #3024

Merged
merged 3 commits into from
Jul 24, 2024

Conversation

joshka
Copy link
Contributor

@joshka joshka commented Jun 30, 2024

Motivation

This commit fixes a bug where a macro call to macros defined in the
standard lib from the tracing and tracing-core crates could be
resolved to a local macro with the same name, causing a compilation
error.

Solution

This commit prefixes these calls with ::core:: to ensure that
they are resolved to the standard library macros.

Fixes: #3023

While this fixes the specific issue at hand, there's likely many macro calls that are impacted by this problem - specifically every call to module_path (there are many). I haven't updated all of those places as I was unsure if that was a good idea as there's about 220 calls of that macro. I also haven't checked for other macros that would also be problematic.

This commit fixes a bug where a macro call to macros defined in the
standard lib from the `tracing` and `tracing-core` crates could be
resolved to a local macro with the same name, causing a compilation
error. This commit prefixes these calls with `::core::` to ensure that
they are resolved to the standard library macros.

Fixes: <tokio-rs#3023>
@joshka joshka requested review from hawkw, carllerche, davidbarsky and a team as code owners June 30, 2024 19:24
Copy link
Contributor

@kaffarell kaffarell left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!
This is not really optimal as some users have a crate named core. In that case this won't work. We fixed this a well in #2761. Basically it would be best to import these things in __macro_support and then use the full path $crate::__macro_support:: (As we did in #2762).

This ensures that the tracing lib correctly builds when a crate is named
`core`. See tokio-rs#2761 and
tokio-rs#2762 for more info.
@joshka
Copy link
Contributor Author

joshka commented Jul 2, 2024

Wow, that's an interesting problem -

This is not really optimal as some users have a crate named core. In that case this won't work. We fixed this a well in #2761. Basically it would be best to import these things in __macro_support and then use the full path $crate::__macro_support:: (As we did in #2762).

Addressed in 5960e0a by adding these to the __macro_support in tracing / tracing_core. I didn't change all the occurrences of module_path, as these only clash if someone has named a macro module_path. This is less likely than file / line which are small names that have many reasonable use cases.

Copy link
Contributor

@kaffarell kaffarell left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@joshka
Copy link
Contributor Author

joshka commented Jul 18, 2024

ping

@joshka
Copy link
Contributor Author

joshka commented Jul 23, 2024

Can this also be backported to the 0.1 branch please? (Happy to add another PR for that if needed)

@davidbarsky davidbarsky merged commit 1898311 into tokio-rs:master Jul 24, 2024
55 checks passed
@joshka joshka deleted the jm/fix-macro-disambiguation branch July 24, 2024 23:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unqualified line! macro usage inside info! causes issues with ratatui_macros
3 participants