syntax: Make `_` a reserved identifier by petrochenkov · Pull Request #48842 · rust-lang/rust

petrochenkov · 2018-03-08T13:44:36Z

Why:

Lexically _ is an identifier.
Internally it makes implementation of use Trait as _; (Tracking issue for RFC #2166: impl-only-use #48216) and some other things cleaner.
We prevent the externally observable effect of _ being accepted by macros expecting ident by treating _ specially in the ident matcher:

macro_rules! m {($i: ident) => {let $i = 10;}}m!(_);// Still an error

rust-highfive · 2018-03-08T13:44:39Z

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

petrochenkov · 2018-03-08T13:45:10Z

cc @rust-lang/lang
r? @nikomatsakis

petrochenkov · 2018-03-08T13:48:25Z

I can move it into the keyword list below, the only difference would be whether error messages say "keyword _" or "reserved identifier _".

I think "reserved identifier" is better than "keyword" as it doesn't really look/feel like a keyword.

hanna-kruppe · 2018-03-08T13:54:44Z

_ is different enough from identifiers that I think it's pedagogically harmful to treat it as one in any (edit: user-visible) contexts. One example where I've seen this come up is in patterns — let _ = <value with drop glue> dropping immediately rather than at the end of the block, &Foo(_) not causing a "cannot move out of borrowed content" error, etc.

nikomatsakis · 2018-03-08T16:27:05Z

I agree with @rkruppe. I don't want _ to be considered an identifer from the user's perspective -- I've although of it as a special token that has special behavior in places where it appears.

Centril · 2018-03-08T23:04:16Z

I also think this also conflicts with one of the alternatives to PhantomData<T> proposed here: rust-lang/rfcs#2357 (comment).

petrochenkov · 2018-03-08T23:39:06Z

@Centril
It's still a reserved identifier (a keyword, basically), so it can't be used as a general purpose identifier and in the future we can interpret it as we want in any position where it doesn't have existing meaning.

_ being an identifier and not a punctuation token should be an implementation detail except for the interaction with ident matcher (maybe it's also visible for procedural macros?).
Actually, it can be make it 100% implementation detail by tweaking ident matcher logic to keep the old behavior and not accept _.

nikomatsakis · 2018-03-09T22:54:23Z

@petrochenkov

Actually, it can be make it 100% implementation detail by tweaking ident matcher logic to keep the old behavior and not accept _.

👍 to that plan, this is what I was going to suggest.

petrochenkov · 2018-03-10T22:03:26Z

Updated with _ being rejected by ident matcher in macros

petrochenkov · 2018-03-12T20:04:55Z

[01:03:40] failures:
[01:03:40] [run-pass] run-pass/macro-first-set.rs

Ok, I guess I'll have to tweak the may_begin_with logic as well.

nikomatsakis

Well, the code seems fine modulo one nit -- but remind me, what is the motivation for this change again? =)

nikomatsakis · 2018-03-14T18:57:10Z

can we add some sort of helper for this check, like is_macro_ident?

nikomatsakis · 2018-03-14T18:57:21Z

we could then use the helper here too, rather than open-coding this

nikomatsakis · 2018-03-14T19:00:14Z

Wait: one other question .. is '_ also an ident? If so, can macro idents match against it?

nikomatsakis · 2018-03-14T19:01:09Z

Re-read motivation from the intro. r=me with nit addressed -- presuming '_ is not being matched by $x:ident.

petrochenkov · 2018-03-14T23:53:15Z

@nikomatsakis
'_ is matched by $lt: lifetime actually (even without this patch).

We can prohibit this for consistency (both '_ and lifetime matcher are unstable), but I'd rather eventually accept both '_/_ as lifetime/ident instead.

petrochenkov · 2018-03-15T00:05:02Z

what is the motivation for this change again

One more important thing that I didn't mention - _ in use Trait as _; actually needs hygiene data attached to it!
With _ being an identifier it happens automatically, but with _ being a non-identifier token we'd have to do some special tracking.

eddyb · 2018-03-15T11:26:45Z

@petrochenkov @jseyfried IMO we should just switch to relying on Span for hygiene everywhere.

petrochenkov · 2018-03-15T12:53:33Z

@eddyb

we should just switch to relying on Span for hygiene everywhere.

This reminded me of something - Spans are now u32.
So we can change Ident representation from { Symbol, SyntaxContext } to { Symbol, Span } thus giving all identifiers spans (this is really great for diagnostics) without increasing the size of Ident.
This would also eliminate SpannedIdent as duplicated data and would allow to meaningfully reintroduce Ident into HIR thus eliminating some horrible side tables.
Oh man, this is going to be great, I think I know what I'm going to spend this Saturday on.

(This is totally orthogonal to this PR though, it would still be more convenient for _ to be an Ident even with that scheme.)

petrochenkov · 2018-03-15T12:57:56Z

Question: given that #48942 is already in flight, should r#_ be supported as a way to create a normal identifier named "_" or not?

nikomatsakis · 2018-03-15T16:28:50Z

@petrochenkov

We can prohibit this for consistency

I believe that indeed '_ should not match $lt:lifetime. It is not a lifetime (in my mind) -- rather, it is a special token that is used to control the elision rules.

nikomatsakis · 2018-03-15T16:32:45Z

Well, I'm mildly reconsidering that -- see this comment

petrochenkov · 2018-03-17T19:09:18Z

@bors r=nikomatsakis

bors · 2018-03-17T19:09:18Z

📌 Commit ed5ea5c has been approved by nikomatsakis

bors · 2018-03-17T23:23:03Z

⌛ Testing commit ed5ea5c with merge ca6a984...

syntax: Make `_` a reserved identifier Why: - Lexically `_` is an identifier. - Internally it makes implementation of `use Trait as _;` (#48216) and some other things cleaner. - We prevent the externally observable effect of `_` being accepted by macros expecting `ident` by treating `_` specially in the `ident` matcher: ```rust macro_rules! m { ($i: ident) => { let $i = 10; } } m!(_); // Still an error ```

bors · 2018-03-18T01:50:46Z

☀️ Test successful - status-appveyor, status-travis
Approved by: nikomatsakis
Pushing ca6a984 to master...

AST: Give spans to all identifiers Change representation of `ast::Ident` from `{ name: Symbol, ctxt: SyntaxContext }` to `{ name: Symbol, span: Span }`. Syntax contexts still can be extracted from spans (`span.ctxt()`). Why this should not require more memory: - `Span` is `u32` just like `SyntaxContext`. - Despite keeping more spans in AST we don't actually *create* more spans, so the number of "outlined" spans kept in span interner shouldn't become larger. Why this may be slightly slower: - When we need to extract ctxt from an identifier instead of just field read we need to do bit field extraction possibly followed by and access by index into span interner's vector. Both operations should be fast (unless the span interner is under some synchronization) and we already do ctxt extraction from spans all the time during macro expansion, so the difference should be lost in noise. cc rust-lang#48842 (comment)

AST: Give spans to all identifiers Change representation of `ast::Ident` from `{ name: Symbol, ctxt: SyntaxContext }` to `{ name: Symbol, span: Span }`. Syntax contexts still can be extracted from spans (`span.ctxt()`). Why this should not require more memory: - `Span` is `u32` just like `SyntaxContext`. - Despite keeping more spans in AST we don't actually *create* more spans, so the number of "outlined" spans kept in span interner shouldn't become larger. Why this may be slightly slower: - When we need to extract ctxt from an identifier instead of just field read we need to do bit field extraction possibly followed by and access by index into span interner's vector. Both operations should be fast (unless the span interner is under some synchronization) and we already do ctxt extraction from spans all the time during macro expansion, so the difference should be lost in noise. cc #48842 (comment)

8122: Make bare underscore token an Ident rather than Punct in proc-macro r=edwin0cheng a=kevinmehall In rustc and proc-macro2, a bare `_` token is parsed for procedural macro purposes as `Ident` rather than `Punct` (see rust-lang/rust#48842). This changes rust-analyzer to match rustc's behavior and implementation by handling `_` as an Ident in token trees, but explicitly preventing `$x:ident` from matching it in MBE. proc macro crate: ```rust #[proc_macro] pub fn input(input: proc_macro::TokenStream) -> proc_macro::TokenStream { dbg!(input) } ``` test crate: ```rust test_proc_macro::input!(_); ``` output (rustc): ```rust [test-proc-macro/src/lib.rs:10] input = TokenStream [ Ident { ident: "_", span: #0 bytes(173..174), }, ] ``` output (rust-analyzer before this change): ```rust [test-proc-macro/src/lib.rs:10] input = TokenStream [ Punct { ch: '_', spacing: Joint, span: 4294967295, }, ] ``` output (rust-analyzer after this change): ```rust [test-proc-macro/src/lib.rs:10] input = TokenStream [ Ident { ident: "_", span: 4294967295, }, ] ``` Co-authored-by: Kevin Mehall <km@kevinmehall.net>

rust-highfive assigned aturon Mar 8, 2018

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 8, 2018

rust-highfive assigned nikomatsakis and unassigned aturon Mar 8, 2018

petrochenkov commented Mar 8, 2018
View reviewed changes

petrochenkov changed the title ~~syntax: Make _ an identifier~~syntax: Make _ a reserved identifierMar 8, 2018

petrochenkov force-pushed the under branch from e5b86ea to 85c0ae9Compare March 10, 2018 22:01

petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 12, 2018

petrochenkov force-pushed the under branch from 85c0ae9 to 3b31e68Compare March 12, 2018 22:07

petrochenkov added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 12, 2018

nikomatsakis requested changes Mar 14, 2018
View reviewed changes

nikomatsakis added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 14, 2018

nikomatsakis mentioned this pull request Mar 15, 2018
Tracking issue for adding a lifetime specifier to macro_rules!#34303
Closed

petrochenkov mentioned this pull request Mar 16, 2018
Tracking issue for RFC #2166: impl-only-use #48216
Closed
3 tasks

petrochenkov added 2 commits March 17, 2018 22:08

syntax: Make _ an identifier
5d06c89

Reject _ in ident matcher
ed5ea5c

petrochenkov force-pushed the under branch from 3b31e68 to ed5ea5cCompare March 17, 2018 19:08

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 17, 2018

bors merged commit ed5ea5c into rust-lang:masterMar 18, 2018

petrochenkov mentioned this pull request Mar 19, 2018
AST: Give spans to all identifiers #49154
Merged

dtolnay mentioned this pull request Apr 2, 2018
Accept _ in Ident::new dtolnay/syn#388
Closed

CAD97 mentioned this pull request Apr 26, 2018
Underscore Imports not expected intellij-rust/intellij-rust#2496
Closed

petrochenkov deleted the under branch June 5, 2019 16:01

kevinmehall mentioned this pull request Mar 20, 2021
Make bare underscore token an Ident rather than Punct in proc-macro rust-lang/rust-analyzer#8122
Merged

Uh oh!

Uh oh!

Conversation

petrochenkov commented Mar 8, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rust-highfive commented Mar 8, 2018

Uh oh!

petrochenkov commented Mar 8, 2018

Uh oh!

petrochenkovMar 8, 2018

Choose a reason for hiding this comment

Uh oh!

Mark-SimulacrumMar 8, 2018

Choose a reason for hiding this comment

Uh oh!

hanna-kruppe commented Mar 8, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikomatsakis commented Mar 8, 2018

Uh oh!

Centril commented Mar 8, 2018

Uh oh!

petrochenkov commented Mar 8, 2018

Uh oh!

nikomatsakis commented Mar 9, 2018

Uh oh!

petrochenkov commented Mar 10, 2018

Uh oh!

petrochenkov commented Mar 12, 2018

Uh oh!

nikomatsakis left a comment

Choose a reason for hiding this comment

Uh oh!

nikomatsakisMar 14, 2018

Choose a reason for hiding this comment

Uh oh!

nikomatsakisMar 14, 2018

Choose a reason for hiding this comment

Uh oh!

nikomatsakis commented Mar 14, 2018

Uh oh!

nikomatsakis commented Mar 14, 2018

Uh oh!

petrochenkov commented Mar 14, 2018

Uh oh!

petrochenkov commented Mar 15, 2018

Uh oh!

eddyb commented Mar 15, 2018

Uh oh!

petrochenkov commented Mar 15, 2018

Uh oh!

petrochenkov commented Mar 15, 2018

Uh oh!

nikomatsakis commented Mar 15, 2018

Uh oh!

nikomatsakis commented Mar 15, 2018

Uh oh!

petrochenkov commented Mar 17, 2018

Uh oh!

bors commented Mar 17, 2018

Uh oh!

bors commented Mar 17, 2018

Uh oh!

bors commented Mar 18, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

petrochenkov commented Mar 8, 2018•
edited
Loading

hanna-kruppe commented Mar 8, 2018•
edited
Loading