Skip to content
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

syntax: Tweak path parsing logic #37290

Merged
merged 2 commits into from
Oct 21, 2016
Merged

syntax: Tweak path parsing logic #37290

merged 2 commits into from
Oct 21, 2016

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Oct 19, 2016

Associated paths starting with << are parsed in patterns.

Paths like self::foo::bar are interpreted as paths and not as self arguments in methods (cc @matklad).
Now, I believe, all paths are consistently parsed greedily in case of ambiguity.
Detection of &'a mut self:: requires pretty large (but still fixed) lookahead, so I had to increase the size of parser's lookahead buffer.
Curiously, if lookahead_distance >= lookahead_buffer_size was used previously, the parser hung forever, I fixed this as well, now it ICEs.

r? @jseyfried

Copy link
Contributor

@jseyfried jseyfried left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! r=me modulo union!();

impl LookaheadBuffer {
fn len(&self) -> usize {
let shift = if self.start <= self.end { 0 } else { LOOKAHEAD_BUFFER_CAPACITY };
shift + self.end - self.start
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I believe this can be slightly simplified to

(LOOKAHEAD_BUFFER_CAPACITY + self.end - self.start) % LOOKAHEAD_BUFFER_CAPACITY

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, looks simpler.

end: usize,
}

const LOOKAHEAD_BUFFER_CAPACITY: usize = 8;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason this is 8? I think 6 would suffice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

x % LOOKAHEAD_BUFFER_CAPACITY is faster.
Not much difference either way, though.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point -- I forgot about peephole optimizations.

@@ -5961,10 +5968,8 @@ impl<'a> Parser<'a> {
maybe_append(attrs, extra_attrs));
return Ok(Some(item));
}
if self.check_keyword(keywords::Union) &&
self.look_ahead(1, |t| t.is_ident() && !t.is_any_keyword()) {
if self.eat_keyword(keywords::Union) {
Copy link
Contributor

@jseyfried jseyfried Oct 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would break the macro invocation union!(); in an item position.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, right, will fix.

@petrochenkov
Copy link
Contributor Author

@bors r=jseyfried

@bors
Copy link
Contributor

bors commented Oct 20, 2016

📌 Commit fea630e has been approved by jseyfried

@bors
Copy link
Contributor

bors commented Oct 21, 2016

⌛ Testing commit fea630e with merge 5509ae3...

bors added a commit that referenced this pull request Oct 21, 2016
syntax: Tweak path parsing logic

Associated paths starting with `<<` are parsed in patterns.

Paths like `self::foo::bar` are interpreted as paths and not as `self` arguments in methods (cc @matklad).
Now, I believe, *all* paths are consistently parsed greedily in case of ambiguity.
Detection of `&'a mut self::` requires pretty large (but still fixed) lookahead, so I had to increase the size of parser's lookahead buffer.
Curiously, if `lookahead_distance >= lookahead_buffer_size` was used previously, the parser hung forever, I fixed this as well, now it ICEs.

r? @jseyfried
@bors bors merged commit fea630e into rust-lang:master Oct 21, 2016
@durka durka mentioned this pull request Nov 14, 2016
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 17, 2016
add test for rust-lang#37765

Adds a test for rust-lang#37765, a path parsing fix which removes the need for a parenthesis workaround.

Closes rust-lang#37765.
cc rust-lang#37290 @withoutboats
r? @petrochenkov
GuillaumeGomez added a commit to GuillaumeGomez/rust that referenced this pull request Nov 17, 2016
add test for rust-lang#37765

Adds a test for rust-lang#37765, a path parsing fix which removes the need for a parenthesis workaround.

Closes rust-lang#37765.
cc rust-lang#37290 @withoutboats
r? @petrochenkov
nikomatsakis added a commit to nikomatsakis/rust that referenced this pull request Dec 1, 2016
nikomatsakis added a commit to nikomatsakis/rust that referenced this pull request Dec 3, 2016
@petrochenkov petrochenkov deleted the pnp branch March 16, 2017 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants