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

Add Span::mixed_site, Span::resolved_at, and Span::located_at behind hygiene feature #228

Merged
merged 2 commits into from May 20, 2020

Conversation

kevinmehall
Copy link
Contributor

This adds support for hygiene-related APIs recently stabilized for Rust 1.45 (currently nightly).

The Cargo feature is named following the overarching proc_macro_hygiene language feature also about to be stabilized, which itself doesn't add any API surface for proc_macro2.

Closes #210

src/lib.rs Outdated
Comment on lines 351 to 355
/// The span located at the invocation of the procedural macro, but with
/// local variables, labels, and `$crate` resolved at the definition site
/// of the macro.
///
/// `macro_rules` behaves like this in terms of hygiene.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a rephrasing of the proc_macro::Span::mixed_site() documentation, but I was unable to get $crate to work in a proc macro -- it just errors with "expected identifier, found $". Do you know if that is expected to work, or is that referring only to macro_rules?

@alexcrichton
Copy link
Contributor

Would it be possible to avoid the need to opt-in with a feature? Perhaps these APIs could be unconditionally defined on appropriate rustc versions and beyond?

Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Agreed with Alex; there shouldn't be a feature gate on these.

Additionally, it would be better to expose resolved_at and located_at unconditionally and have it return the span associated with the name resolution behavior (as opposed to the line/column information, which is cosmetic) when the compiler doesn't support the underlying API. This way they can be adopted more readily by downstream code.

@dtolnay dtolnay self-assigned this May 18, 2020
On compilers prior to 1.45 where this is not stable, these fall back to
returning the span associated with resolution behavior, since the source
location is only cosmetic. This is a reversal of the previous fallback
implementation, which preserved source location because it does not
track resolution location. The differnce is only observable with
`span_locations` enabled.

These methods were stabilized in Rust in
rust-lang/rust#69041
Copy link
Owner

@dtolnay dtolnay left a comment

Choose a reason for hiding this comment

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

Thanks, looks good.

I merged this with fc5678f to account for the possibility that any of the hygiene stabilizations are reverted before 1.45 ships. We can stabilize in proc-macro2 after the rustc release goes out.

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.

expose mixed_site macro_rules hygeine
3 participants