Implement edition-gated keywords

[Manishearth]

We want to reserve a bunch of keywords in the 2018 edition. There's a full list of potential keywords, but for now I'll probably just reserve async, await, and catch.

EDIT: Here's my preliminary review of potential keywords and stuff: https://hackmd.io/lu7wxNbRTuO3C0NITUum6Q?both# // @Centril

For edition hygiene reasons this will be done entirely at the lexer level. Keywords in Rust are all lexed as identifiers, however you can have "raw identifiers" which let you create idents treated explicitly as idents. We overload this, so that if the lexer comes across the identifier "async" on Rust 2015, it lexes it as if it were r#async. We may want to fix it up in the pretty printer as well.

[Manishearth]

An alternate fix is to have a second flag on Ident, which marks whether or not it's allowed to form a keyword. This has the additional benefit of being easier to work with for promoting contextual keywords to full keywords.

[Manishearth]

Actually, not doing catch yet, it's contextual.

[Manishearth]

Ah, catch is currently implemented in an unstable form as do catch. We can change this whenever we like so that it doesn't work for the 2015 epoch and in 2018 uses a non-contextual catch keyword.

[Manishearth]

#49611

[cramertj]

Why do async and await need to be non-contexutal keywords? AFAIK at least async could be added backwards-compatibly as a contextual keyword.

[Manishearth]

Because I picked the two keywords I knew we definitely wanted 😄 (most of the others are under discussion or otherwise iffy).

I wasn't aware we were fine with them as contextual keywords -- I thought we wanted to epochify anything that we may need as a keyword in the next epoch.

Personally I'd rather add things as full keywords. But thinking about it I guess it would be nice if Rust 2015 got async/await as well.

However, AFAICT await will have to be a real keyword? Or it will be prone to the same issue we have with catch (which causes do catch)? And if await is going to be a 2018-only keyword it seems like the entire feature should just be 2018-only, in which case, why not make async a full keyword.

idk 😄

[cramertj]

why not make async a full keyword

Because I have preexisting modules named async 😛 lol

[Manishearth]

r#async all the way 😛

[Manishearth]

@nikomatsakis thoughts on what we should be doing here?

As mentioned in Gitter, we can do this for dyn as well -- no dyn at all in 2015, and dyn becomes a full keyword in 2018. I kinda prefer that.

cc @Centril

[Centril]

@Manishearth

[..] we can do this for dyn as well -- no dyn at all in 2015, and dyn becomes a full keyword in 2018. I kinda prefer that.

cc @Centril

Yes, this ^. All the way. I think we should make switching to Rust 2018 as enticing as possible, and reduce technical debt if we can. So I'm 👍 for no dyn in edition 2015.

On async: @rpjohnst's pre-RFC has async { .. } blocks; and so this would conflict with struct async { .. } [..] async { .. } wherefore I think async needs to be a real keyword so that we can future proof things.

On catch, the keyword reservation does not seem settled, but whatever we go with (try, catch, trap, .. ) it should be a real keyword for the same problem that async blocks would have.

On await I have no idea.

[rpjohnst]

(I intentionally didn't think too hard about syntax. async { .. } was just shorthand for async || { .. } because the arguments were always empty- so unless we actually expect that syntax to stick around for some other reason it's not super relevant.)

[Manishearth]

@rpjohnst what about await? Are either of these in a situation where we want a non-contextual keyword?

[rpjohnst]

The proposal I wrote doesn't use the await keyword at all, but I have no idea if that will actually work out- like the async case it wasn't really about the syntax.

I think @withoutboats has thought a lot more about the syntax of async/await.

[nikomatsakis]

Tagging this as A-rust-2018-preview. The main work remaining here, afaik, is to issue a lint when people use identifiers that will become keywords in the future. I've been debating how to implement that. I can see three basic approaches so far:

• Walk the AST for definitions/references to identifiers with a suspicious name.
• Walk the HIR for definitions/references to identifiers with a suspicious name.
• Modify the parser so that when it sees a given identifier, it records the locations + spans.

Modifying the parser would be harder to tie in with a lint, in that we lose the "scope" information we typically use to decide if the lint is enabled, but we could say that you have to put the migration lint at the root of the crate for it to work, and I'd be ok with that.

The first two seem like they might be easier though. Have to find the right time to do it. We might be able to leverage the existing folders etc to visit every keyword, though, which is very nice. If we do it on the AST, I guess we might be able to intercept macro-related things, but I we actually don't have to do that, since AST matching (e.g., $t:ident etc) doesn't really care about this... right?

(Due in part to our behavior in #49520, which I was concerned about, but now I think I see why @petrochenkov preferred it.)

cc @petrochenkov @Manishearth — I know both of you have put some thought into this, would appreciate your feedback.

[Manishearth]

A tricky bit is handling keywords in macro definitions and invocations, but aside from that i think walking the tree is fine

[nikomatsakis]

@Manishearth

In what way is that tricky, say more =)

I guess the idea is if we use a macro to parse something as an expression and then drop it on the floor?

e.g.,

macro_rules! validate_expr {
  ($t:expr) => { }
}

macro_rules! gen_expr {
  () => { validate_expr! { { let try = 22; } }
}

Now gen_expr!, when invoked from the wrong Epoch or something, would likely misbehave?

(Though if we used the span of the stuff being parsed to decide the edition, that example would probably just work...)

[nikomatsakis]

(Note: in discussion today, we decided this wasn't a preview blocker per se, though it is an edition blocker.)

[petrochenkov]

@nikomatsakis
I think we can visit all idents in AST as soon as NodeIds are assigned and it will be good enough.

This visiting would include right sides of macro definitions which are possibly never expanded in the current crate, but will start generating errors in other crates once the edition of this crate is bumped.
They would be visited as token streams because we don't know anything more about them. Left sides don't need to be checked as you've mentioned above.

There can be false positives in corner cases, e.g. left sides of nested macros defined inside right side of a macro. Regarding the validate_expr example, I'm not even sure it would be a false positive - it certainly wouldn't hurt to rename try into something else in it, in case validate_expr stops dropping arguments.

[scottmcm]

FWIW, try is made an edition keyword by PR #52602.

[nikomatsakis]

Does anyone have any idea what work remains to be done here?

We are now linting for the use of 2018 keywords in Rust 2015, for example, at least afaik. Based on the code I see four keywords, and all of them appear to have at least rudimentary tests:

• async (test)
• dyn (test)
• try (test as ident, test as macro)
• await (migrate to r#await in 2105, error in 2018, non-error in 2018 with feature gate)

So maybe we can close this?

[Centril]

Yes I think we can close this; I filed #54774 for improved diagnostics, but that is tracked separately.