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

macros: Allow field path segments to be keywords #2925

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

svix-jplatte
Copy link

@svix-jplatte svix-jplatte commented Apr 3, 2024

Motivation

Currently, a keyword like type fails compilation as (a path segment of) a field name, for no clear reason. Trying to use r#type instead leads to the r# being part of the field name, which is unhelpful¹.

Solution

Don't require the field path to match a macro_rules! expr, use repeated tt instead. I can't tell why this was ever required: The internal stringify macro was introduced in 55091c9#diff-315c02cd05738da173861537577d159833f70f79cfda8cd7cf1a0d7a28ace31b with an expr matcher without any explanation, and no tests are failing from making it match upstream's stringify! input format.

Special thanks to whoever implemented the unstable macro-backtrace feature in rustc, otherwise this would have been nigh impossible to track down!

¹ this can likely be fixed too by some sort of "unraw" macro that turns r#foo into foo, but that's a separate change not made in this PR

Copy link
Contributor

@hds hds 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 tests! Could we please move them into the tracing crate, since that's where the macros that they're testing are defined.

tracing-attributes/tests/fields.rs Outdated Show resolved Hide resolved
@svix-jplatte
Copy link
Author

Rebased to fix CI (hopefully).

@svix-jplatte svix-jplatte requested a review from hds May 13, 2024 14:51
@svix-jplatte
Copy link
Author

I'm working on fixing the new CI failures ^^

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.

None yet

2 participants