syntax: Make parsing of paths whitespace agnostic + some other changes #33041

petrochenkov · 2016-04-16T20:26:46Z

Paths are mostly parsed without taking whitespaces into account, e.g. std :: vec :: Vec :: new () parses successfully, however, there are some special cases involving keywords super, self and Self. For example, self:: is considered a path start only if there are no spaces between self and ::. These restrictions probably made sense when self and friends weren't keywords, but now they are unnecessary.

The first two commits remove this special treatment of whitespaces by removing token::IdentStyle entirely and therefore fix #14109.
This change also affects naked self and super (which are not tightly followed by ::, obviously) they can now be parsed as paths, however they are still not resolved correctly in imports (cc @jseyfried, see compile-fail/use-keyword.rs), so #29036 is not completely fixed.

The third commit also makes super, self, Self and static keywords nominally (before this they acted as keywords for all purposes) and removes most of remaining "special idents".

The last commit (before tests) contains some small improvements - some qualified paths with type parameters are parsed correctly, parse_path is not used for parsing single identifiers, imports are sanity checked for absence of type parameters - such type parameters can be generated by syntax extensions or by macros when #10415 is fixed (~~soon!~~already!).

This patch changes some pretty basic things in libsyntax, like token::Token and the keyword list, so it's a plugin-[breaking-change].

r? @eddyb

eddyb · 2016-04-16T20:30:21Z

src/libsyntax/parse/token.rs

+        (2,                          __Unused2,      "<__unused2>");
+        (3,                          __Unused3,      "<__unused3>");
+        (4,                          __Unused4,      "<__unused4>");
+        (5,                          __Unused5,      "<__unused5>");


Can we remove all of these? And replace special_idents with "non-strict keywords"?

Yeah, I just wanted to leave some free space so the diff is not messed up on every new contextual keyword, but it's not so important I suppose.
Also, naming is hard, what is the short and obvious name to use instead of special_idents? weak_keywords maybe? Also, "" doesn't even look like a keyword!

Well, I wouldn't even have a name. I'd just put all of them in the same bag, and just auto-generate the STRICT_KEYWORD_{START,END} from the list, with 'strict still there.

bors · 2016-04-17T01:31:56Z

☔ The latest upstream changes (presumably #32875) made this pull request unmergeable. Please resolve the merge conflicts.

@eddyb

syntax: Parse import prefixes as paths Fixes rust-lang#10415 r? @eddyb (This partially intersects with rust-lang#33041)

petrochenkov · 2016-04-18T21:54:03Z

@eddyb
I've updated the PR.
Keywords and special idents are merged together and the keyword generating macro is simplified.
The "bounds" for strict/reserved keywords are written manually now, but I'd say it's less error prone than the previous macro shenanigans.
The second commit contains some other small changes, see the commit message for description.

eddyb · 2016-04-18T21:59:53Z

LGTM, very nice! @nrc @erickt @Manishearth, any objections?

nrc · 2016-04-18T22:06:53Z

Should be breaking-batch, rather than being landed directly. But I think we should try to land it soon so it doesn't rot. cc @Manishearth

nrc · 2016-04-18T22:08:14Z

LGTM

Manishearth · 2016-04-18T22:14:49Z

We don't have any other breaking-batch changes queued up, but this is large enough that it can be landed directly. Let's wait a few days for the stakeholders to prepare for this, and land it then.

(In the meantime, if you have any breaking changes planned, now is the time to make PRs!)

Manishearth · 2016-04-18T22:18:34Z

Left a comment on #31645, feel free to land in a few days

Manishearth · 2016-04-20T05:19:34Z

Please land this simultaneously with #31414 . I'll do it myself in a day or two if both get approved.

petrochenkov · 2016-04-20T23:46:50Z

I've pushed one more commit.
It makes is_path_start precise and fixes some error message regressions caused by that.

Manishearth · 2016-04-21T06:03:22Z

Is this ready to land? (two LGTMs, seems good) Just mention r=me, don't actually tell bors to approve it.

nrc · 2016-04-21T21:33:56Z

@Manishearth r=me

Lift some restrictions on type parameters in paths Sanity check import paths for type parameters

Simplify the macro used for generation of keywords Make `Keyword::ident` private

syntax: Merge PathParsingMode::NoTypesAllowed and PathParsingMode::ImportPrefix syntax: Rename PathParsingMode and its variants to better express their purpose syntax: Remove obsolete error message about 'self lifetime syntax: Remove ALLOW_MODULE_PATHS workaround syntax/resolve: Adjust some error messages resolve: Compare unhygienic (not renamed) names with keywords::Invalid, invalid identifiers may appear to be valid after renaming

…about unexpected tokens

petrochenkov · 2016-04-24T18:59:11Z

@Manishearth
I've rebased this PR, added #33157 to it and regenerated the failing keyword parsing tests.

@jseyfried

Paths are mostly parsed without taking whitespaces into account, e.g. `std :: vec :: Vec :: new ()` parses successfully, however, there are some special cases involving keywords `super`, `self` and `Self`. For example, `self::` is considered a path start only if there are no spaces between `self` and `::`. These restrictions probably made sense when `self` and friends weren't keywords, but now they are unnecessary. The first two commits remove this special treatment of whitespaces by removing `token::IdentStyle` entirely and therefore fix rust-lang#14109. This change also affects naked `self` and `super` (which are not tightly followed by `::`, obviously) they can now be parsed as paths, however they are still not resolved correctly in imports (cc @jseyfried, see `compile-fail/use-keyword.rs`), so rust-lang#29036 is not completely fixed. The third commit also makes `super`, `self`, `Self` and `static` keywords nominally (before this they acted as keywords for all purposes) and removes most of remaining \"special idents\". The last commit (before tests) contains some small improvements - some qualified paths with type parameters are parsed correctly, `parse_path` is not used for parsing single identifiers, imports are sanity checked for absence of type parameters - such type parameters can be generated by syntax extensions or by macros when rust-lang#10415 is fixed (~~soon!~~already!). This patch changes some pretty basic things in `libsyntax`, like `token::Token` and the keyword list, so it's a plugin-[breaking-change]. r? @eddyb

Batch up breaking libsyntax changes Contains: - #33125 - #33041 - #33157 cc #31645

rust-lang/rust#33041

rust-highfive assigned eddyb Apr 16, 2016

eddyb reviewed Apr 16, 2016
View reviewed changes

petrochenkov mentioned this pull request Apr 17, 2016

syntax: Parse import prefixes as paths #33044

Merged

Manishearth added a commit to Manishearth/rust that referenced this pull request Apr 17, 2016

Rollup merge of rust-lang#33044 - petrochenkov:prefix, r=eddyb

02e40d9

syntax: Parse import prefixes as paths Fixes rust-lang#10415 r? @eddyb (This partially intersects with rust-lang#33041)

petrochenkov force-pushed the path branch from bb4a451 to e9e09dd Compare April 18, 2016 21:42

Manishearth mentioned this pull request Apr 18, 2016

Tracking issue for libsyntax breaking changes #31645

Closed

Manishearth mentioned this pull request Apr 20, 2016

special-case #[derive(Copy, Clone)] with a shallow clone #31414

Merged

petrochenkov force-pushed the path branch from e9e09dd to f5972a6 Compare April 20, 2016 23:45

jseyfried mentioned this pull request Apr 23, 2016

Parse pub(restricted) visibilities on tuple struct fields #33161

Merged

Manishearth mentioned this pull request Apr 24, 2016

Batch up breaking libsyntax changes #33179

Merged

petrochenkov added 7 commits April 24, 2016 20:59

syntax: Don't rely on token::IdentStyle in the parser

8dbf8f5

syntax: Get rid of token::IdentStyle

546c052

syntax: Make static/super/self/Self keywords + special ident cleanup

e2c821d

syntax: Don't parse idents with parse_path

8dbab51

Lift some restrictions on type parameters in paths Sanity check import paths for type parameters

Add tests

923001e

syntax: Merge keywords and remaining special idents in one list

b32d7b5

Simplify the macro used for generation of keywords Make `Keyword::ident` private

petrochenkov added 3 commits April 24, 2016 20:59

syntax: Make is_path_start precise and improve some error messages …

a97f60e

…about unexpected tokens

Remove some old code from libsyntax

9108fb7

Fix keyword parsing tests

4bd44be

petrochenkov force-pushed the path branch from f5972a6 to 4bd44be Compare April 24, 2016 18:57

petrochenkov mentioned this pull request Apr 24, 2016

[breaking-batch] Remove some old code from libsyntax #33157

Closed

bors added a commit that referenced this pull request Apr 24, 2016

Auto merge of #33179 - Manishearth:breaking-batch, r=Manishearth

1930483

Batch up breaking libsyntax changes Contains: - #33125 - #33041 - #33157 cc #31645

bors merged commit 4bd44be into rust-lang:master Apr 24, 2016

lambda-fairy added a commit to lambda-fairy/maud that referenced this pull request Apr 27, 2016

Update Rust

00ec443

rust-lang/rust#33041

ivan mentioned this pull request May 1, 2016

Update for rust-lang/rust#33041 serde-deprecated/quasi#40

Closed

petrochenkov deleted the path branch September 21, 2016 19:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syntax: Make parsing of paths whitespace agnostic + some other changes #33041

syntax: Make parsing of paths whitespace agnostic + some other changes #33041

petrochenkov commented Apr 16, 2016 •

edited

Loading

eddyb Apr 16, 2016

petrochenkov Apr 16, 2016

eddyb Apr 16, 2016

bors commented Apr 17, 2016

petrochenkov commented Apr 18, 2016

eddyb commented Apr 18, 2016

nrc commented Apr 18, 2016

nrc commented Apr 18, 2016

Manishearth commented Apr 18, 2016

Manishearth commented Apr 18, 2016

Manishearth commented Apr 20, 2016

petrochenkov commented Apr 20, 2016 •

edited

Loading

Manishearth commented Apr 21, 2016

nrc commented Apr 21, 2016

petrochenkov commented Apr 24, 2016

syntax: Make parsing of paths whitespace agnostic + some other changes #33041

syntax: Make parsing of paths whitespace agnostic + some other changes #33041

Conversation

petrochenkov commented Apr 16, 2016 • edited Loading

eddyb Apr 16, 2016

Choose a reason for hiding this comment

petrochenkov Apr 16, 2016

Choose a reason for hiding this comment

eddyb Apr 16, 2016

Choose a reason for hiding this comment

bors commented Apr 17, 2016

petrochenkov commented Apr 18, 2016

eddyb commented Apr 18, 2016

nrc commented Apr 18, 2016

nrc commented Apr 18, 2016

Manishearth commented Apr 18, 2016

Manishearth commented Apr 18, 2016

Manishearth commented Apr 20, 2016

petrochenkov commented Apr 20, 2016 • edited Loading

Manishearth commented Apr 21, 2016

nrc commented Apr 21, 2016

petrochenkov commented Apr 24, 2016

petrochenkov commented Apr 16, 2016 •

edited

Loading

petrochenkov commented Apr 20, 2016 •

edited

Loading