-
Notifications
You must be signed in to change notification settings - Fork 12.7k
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
Implement edition hygiene for keywords #50307
Conversation
r? @aturon (rust_highfive has picked a reviewer for you, use r? to override) |
r? @nikomatsakis |
We don't need keywords that are keywords on old editions but not new, we need it the other way around 😄 |
Haven't really gone through this completely, but could you include some form of the changes from 0eabb1b so that it's less fragile? (and liberally sprinkle comments that link things together) |
r#async = consumes_async!(async); // OK | ||
// r#async = consumes_async!(r#async); // ERROR, not a match | ||
// r#async = consumes_async_raw!(async); // ERROR, not a match | ||
r#async = consumes_async_raw!(r#async); // OK |
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.
I continue to be nervous about the idea that macros can observe the different here -- what happens if I have a macro from a 2018 Edition crate that accepts r#async
? Can a 2015 crate pass in async
and have it work?
Put another way:
If I have a macro that accepts a async
in its input, can I transition my crate to 2018 Edition without a breaking change?
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.
With scheme implemented in this PR keyword hygiene and raw identifiers are entirely orthogonal.
(So this PR doesn't make changes to any interactions between raw identifiers and macros.)
If macro accepts async
literally (not as $i: ident
or something) in its left hand side, then it will not appear in the macro output, so we are safe even without keyword hygiene (as consumes_async!(async)
shows, yeah).
let mut async = 1; // OK | ||
let mut r#async = 1; // OK | ||
|
||
r#async = consumes_async!(async); // OK |
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.
I guess the answers to my question are here, maybe?
This looks pretty good so far! (modulo my questions about the desired behavior of raw idents in macros) |
Update: I'll return to this once I'm done with proc macro 1.2 API review, likely this weekend. |
src/libsyntax_pos/symbol.rs
Outdated
@@ -359,16 +359,20 @@ declare_keywords! { | |||
(53, Virtual, "virtual") | |||
(54, Yield, "yield") | |||
|
|||
// Edition-specific keywords reserved for future use. | |||
(55, Async, "async") // >= 2018 Edition Only | |||
(56, Proc, "proc") // <= 2015 Edition Only |
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.
Why are we keeping proc reserved on 2015? We'll surely never use it for anything on the 2015 epoch.
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.
NVM saw the comment explaining this
☔ The latest upstream changes (presumably #50454) made this pull request unmergeable. Please resolve the merge conflicts. |
any updates |
@Manishearth |
Thanks!
…On Thu, May 10, 2018 at 2:24 PM Vadim Petrochenkov ***@***.***> wrote:
@Manishearth <https://github.com/Manishearth>
I still need plumbing mentioned in TODO (thread edition info from crate
metadata into libsyntax/ext/expand.rs where it's needed).
I'll work on this tomorrow.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#50307 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ABivSKf7mMm7fnLGeeYCOCaHUMivzEx1ks5txLAigaJpZM4TrnVO>
.
|
PR updated, all tests are now fixed. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
☔ The latest upstream changes (presumably #50235) made this pull request unmergeable. Please resolve the merge conflicts. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
1 similar comment
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Needs rebase. |
@bors r=nikomatsakis |
📌 Commit d8bbc1e has been approved by |
Implement edition hygiene for keywords Determine "keywordness" of an identifier in its hygienic context. cc #49611 I've resurrected `proc` as an Edition-2015-only keyword for testing purposes, but it should probably be buried again. EDIT: `proc` is removed again.
☀️ Test successful - status-appveyor, status-travis |
@petrochenkov Looking over this PR I'm unable to find a way to get the current edition while parsing (not necessarily in a macro). Should the edition be passed down into the parser, or is there something else I should be using to determine if something is a keyword in the current edition while parsing? |
|
For a given identifier |
`async` was added in rust-lang/rust#50307 and is not yet implemented `try` was added in rust-lang/rust#52602 and is not yet stable: rust-lang/rust#31436
Determine "keywordness" of an identifier in its hygienic context.
cc #49611
I've resurrected
proc
as an Edition-2015-only keyword for testing purposes, but it should probably be buried again. EDIT:proc
is removed again.