Skip to content

await as a keyword and await! macro #53834

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

Closed
withoutboats opened this issue Aug 30, 2018 · 13 comments
Closed

await as a keyword and await! macro #53834

withoutboats opened this issue Aug 30, 2018 · 13 comments
Assignees
Labels
A-edition-2018 Area: The 2018 edition F-rust_2018_preview `#![feature(rust_2018_preview)]` P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management

Comments

@withoutboats
Copy link
Contributor

withoutboats commented Aug 30, 2018

We intend for await to be a keyword in the 2018 edition, this causes a tricky problem: currently, await! is a macro, not using "normal" control flow syntax, because we haven't decided exactly how the syntax should work (see the RFC unresolved questions for more context). But macros are not supposed to be able to have keywords as names.

It's now the case that not only does std have an await! macro, but other libraries are defining them also to handle compatibility between different versions of futures, specifically tokio-async-await.

I propose this solution to the problem:

  1. On 2018, await is a true keyword - a macro named await would be invalid.
  2. Under the await_macro feature, the await keyword is disabled, allowing users (including std) to use await as a macro name.
  3. Before we stabilize async/await, we resolve the syntactic question for await and make await a keyword again. This will break anyone using await as a macro name, but they were already on nightly, and expected this breakage.

This avoids special casing std await!, making the behavior on stable for 2018 simple and allowing tokio-async-await to keep its await! macro.

cc @carllerche @Centril @cramertj

@withoutboats withoutboats added WG-epoch Working group: Epoch (2018) management F-rust_2018_preview `#![feature(rust_2018_preview)]` P-high High priority labels Aug 30, 2018
@carllerche
Copy link
Member

I'm not sure that I follow. Isn't tokio-async-await using 2018? I also am not following all the details of the edition stuff.

@withoutboats
Copy link
Contributor Author

I'm not sure that I follow. Isn't tokio-async-await using 2018? I also am not following all the details of the edition stuff.

The proposal is that await will become a reserved keyword in 2018 unless you turn on the await_macro feature, which lets you define await as a macro, and also gates access to the await! macro defined in std. That is, we reverse the change in 2018 if you turn on this feature.

Eventually, we plan to settle on a permanent syntax for await, make it a real keyword, and remove the await_macro feature. But that's fine, because features are nightly only. We won't break any code on stable.

@Nemo157
Copy link
Member

Nemo157 commented Aug 30, 2018

I presume this will involve promoting the await_macro feature from being a lib feature to a lang feature? Currently it’s not possible to activate the await_macro feature in a no_std crate because it only exists with std. It seems that this plan should allow activating it to get the non-keywordness of await even without having the std await! macro available.

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Aug 30, 2018
@carllerche
Copy link
Member

@withoutboats ah, I see.

Either way, I think that I will have to switch to an alternate macro name. I don't expect the move off of futures 0.1 to be fast, so the transition plan will have to stretch into the 2018 edition.

@withoutboats
Copy link
Contributor Author

@carllerche the feature will exist until async/await is stabilized, which is well after the 2018 edition is released

@carllerche
Copy link
Member

@withoutboats Ah, that makes sense, thanks.

@crlf0710
Copy link
Member

crlf0710 commented Sep 5, 2018

A more straightforward solution is to always parse the identifier in the sequence
<ident> ! <lparen|lbracket|lbrace>

not as a keyword, but as a macro name. I think it is backwards compatible, though it does complicate things a bit.

@withoutboats
Copy link
Contributor Author

We discussed this in lang team today and we agreed to do this: on 2018 stable, await should be a reserved word, but the await_macro nightly feature will unreserve it so that these macros can be written.

@cramertj cramertj self-assigned this Sep 13, 2018
@cramertj
Copy link
Member

When I went to implement this I realized that we can't just specifically parse await as a non-keyword when in macro_rules or in a macro invocation (await!) because we also want to support use std::await;, which is neither a macro invocation nor definition. We could do name resolution for the macro and then succeed or fail based on whether or not the item pointed to is a macro, but this seems problematic for all kinds of reasons, not least of which being "what do we do if it would import a macro and struct both named await" (since macros and types don't share a namespace).

To clarify why we can't just change parsing based on the await_macro keyword, feature gates aren't available during parsing. One option would be not to treat await as a keyword at all, but instead gate its use in the AST based on edition=2018 XOR await_macro feature gate. However, this wouldn't give us the edition transition lint, so we'd have to manually craft the lint for await (which seems plausible). cc @petrochenkov who may have an idea how to best implement this.

@Centril
Copy link
Contributor

Centril commented Sep 14, 2018

As long as we retain the ability to eventually make it a true keyword in edition 2018, that sounds good to me.

@nikomatsakis
Copy link
Contributor

So I talked to @cramertj about how to implement this and we ran into the problem previously described. I had two ideas for possible ways to fix this:

Option 1. Never treat await as a keyword in the compiler. However, do something similar to the keyword edition lint found here:

declare_lint! {
pub KEYWORD_IDENTS,
Allow,
"detects edition keywords being used as an identifier"
}
/// Checks for uses of edtion keywords used as an identifier
#[derive(Clone)]
pub struct KeywordIdents;
impl LintPass for KeywordIdents {
fn get_lints(&self) -> LintArray {
lint_array!(KEYWORD_IDENTS)
}
}
impl KeywordIdents {
fn check_tokens(&mut self, cx: &EarlyContext, tokens: TokenStream) {
for tt in tokens.into_trees() {
match tt {
TokenTree::Token(span, tok) => match tok.ident() {
// only report non-raw idents
Some((ident, false)) => {
self.check_ident(cx, ast::Ident {
span: span.substitute_dummy(ident.span),
..ident
});
}
_ => {},
}
TokenTree::Delimited(_, ref delim) => {
self.check_tokens(cx, delim.tts.clone().into())
},
}
}
}
}
impl EarlyLintPass for KeywordIdents {
fn check_mac_def(&mut self, cx: &EarlyContext, mac_def: &ast::MacroDef, _id: ast::NodeId) {
self.check_tokens(cx, mac_def.stream());
}
fn check_mac(&mut self, cx: &EarlyContext, mac: &ast::Mac) {
self.check_tokens(cx, mac.node.tts.clone().into());
}
fn check_ident(&mut self, cx: &EarlyContext, ident: ast::Ident) {
let next_edition = match cx.sess.edition() {
Edition::Edition2015 => {
match &ident.as_str()[..] {
"async" |
"try" => Edition::Edition2018,
_ => return,
}
}
// no new keywords yet for 2018 edition and beyond
_ => return,
};
// don't lint `r#foo`
if cx.sess.parse_sess.raw_identifier_spans.borrow().contains(&ident.span) {
return;
}
let mut lint = cx.struct_span_lint(
KEYWORD_IDENTS,
ident.span,
&format!("`{}` is a keyword in the {} edition",
ident.as_str(),
next_edition),
);
lint.span_suggestion_with_applicability(
ident.span,
"you can use a raw identifier to stay compatible",
format!("r#{}", ident.as_str()),
Applicability::MachineApplicable,
);
lint.emit()
}
}

It would work mildly differently. I think it would work like this:

  • If the feature gate is enabled, we ignore await altogether.
  • Otherwise:
    • If in Rust 2018, we hard error when you use await anywhere.
    • If in Rust 2015, we warn, just like we would for any other keyword.

@nikomatsakis
Copy link
Contributor

OK, never mind, just Option 1. Option 2 just seemed like strictly more work so I leave it off. :)

@Centril Centril added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Sep 20, 2018
bors added a commit that referenced this issue Sep 25, 2018
Make "await" a pseudo-edition keyword

This change makes "await" ident an error in 2018 edition without async_await
feature and adds "await" to the 2018 edition keyword lint group that
suggest migration on the 2015 edition.

cc #53834

r? @nikomatsakis
@pnkfelix
Copy link
Member

visited for triage. As far as we can tell, this was fixed by PR #54411.

@fmease fmease added the A-edition-2018 Area: The 2018 edition label Dec 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-edition-2018 Area: The 2018 edition F-rust_2018_preview `#![feature(rust_2018_preview)]` P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. WG-epoch Working group: Epoch (2018) management
Projects
None yet
Development

No branches or pull requests

9 participants