Skip to content

Commit

Permalink
Auto merge of #32479 - eddyb:eof-not-even-twice, r=nikomatsakis
Browse files Browse the repository at this point in the history
Prevent bumping the parser past the EOF.

Makes `Parser::bump` after EOF into an ICE, forcing callers to avoid repeated EOF bumps.
This ICE is intended to break infinite loops where EOF wasn't stopping the loop.

For example, the handling of EOF in `parse_trait_items`' recovery loop fixes #32446.
But even without this specific fix, the ICE is triggered, which helps diagnosis and UX.

This is a `[breaking-change]` for plugins authors who eagerly eat multiple EOFs.
See docopt/docopt.rs#171 for such an example and the necessary fix.
  • Loading branch information
bors committed Mar 29, 2016
2 parents cad964a + 221d0fb commit a111297
Show file tree
Hide file tree
Showing 6 changed files with 49 additions and 14 deletions.
37 changes: 28 additions & 9 deletions src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,6 +254,7 @@ pub struct Parser<'a> {
/// the previous token or None (only stashed sometimes).
pub last_token: Option<Box<token::Token>>,
last_token_interpolated: bool,
last_token_eof: bool,
pub buffer: [TokenAndSpan; 4],
pub buffer_start: isize,
pub buffer_end: isize,
Expand Down Expand Up @@ -366,6 +367,7 @@ impl<'a> Parser<'a> {
last_span: span,
last_token: None,
last_token_interpolated: false,
last_token_eof: false,
buffer: [
placeholder.clone(),
placeholder.clone(),
Expand Down Expand Up @@ -955,7 +957,9 @@ impl<'a> Parser<'a> {
{
self.expect(bra)?;
let result = self.parse_seq_to_before_end(ket, sep, f);
self.bump();
if self.token == *ket {
self.bump();
}
Ok(result)
}

Expand Down Expand Up @@ -998,6 +1002,15 @@ impl<'a> Parser<'a> {

/// Advance the parser by one token
pub fn bump(&mut self) {
if self.last_token_eof {
// Bumping after EOF is a bad sign, usually an infinite loop.
self.bug("attempted to bump the parser past EOF (may be stuck in a loop)");
}

if self.token == token::Eof {
self.last_token_eof = true;
}

self.last_span = self.span;
// Stash token for error recovery (sometimes; clone is not necessarily cheap).
self.last_token = if self.token.is_ident() ||
Expand Down Expand Up @@ -1281,15 +1294,21 @@ impl<'a> Parser<'a> {
Ok(cua) => cua,
Err(e) => {
loop {
p.bump();
if p.token == token::Semi {
p.bump();
break;
}
match p.token {
token::Eof => break,

token::CloseDelim(token::Brace) |
token::Semi => {
p.bump();
break;
}

token::OpenDelim(token::Brace) => {
p.parse_token_tree()?;
break;
}

if p.token == token::OpenDelim(token::DelimToken::Brace) {
p.parse_token_tree()?;
break;
_ => p.bump()
}
}

Expand Down
3 changes: 1 addition & 2 deletions src/test/compile-fail/issue-10636-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,5 @@
pub fn trace_option(option: Option<isize>) {
option.map(|some| 42; //~ NOTE: unclosed delimiter
//~^ ERROR: expected one of
//~^^ ERROR: mismatched types
} //~ ERROR: incorrect close delimiter
//~^ ERROR: expected one of
//~^ ERROR: unexpected token
3 changes: 2 additions & 1 deletion src/test/compile-fail/token-error-correct-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ pub mod raw {
if !is_directory(path.as_ref()) { //~ ERROR: unresolved name `is_directory`
callback(path.as_ref(); //~ NOTE: unclosed delimiter
//~^ ERROR: expected one of
fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: expected one of
fs::create_dir_all(path.as_ref()).map(|()| true) //~ ERROR: mismatched types
} else { //~ ERROR: incorrect close delimiter: `}`
//~^ ERROR: expected one of
Ok(false);
}

Expand Down
16 changes: 16 additions & 0 deletions src/test/parse-fail/issue-32446.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
// file at the top-level directory of this distribution and at
// http://rust-lang.org/COPYRIGHT.
//
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
// option. This file may not be copied, modified, or distributed
// except according to those terms.

// compile-flags: -Z parse-only

fn main() {}

// This used to end up in an infite loop trying to bump past EOF.
trait T { ... } //~ ERROR
2 changes: 1 addition & 1 deletion src/test/parse-fail/pat-lt-bracket-6.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@

fn main() {
let Test(&desc[..]) = x; //~ error: expected one of `,` or `@`, found `[`
//~^ ERROR expected one of `:`, `;`, or `=`, found `..`
//~^ ERROR expected one of `:`, `;`, `=`, or `@`, found `[`
}
2 changes: 1 addition & 1 deletion src/test/parse-fail/pat-lt-bracket-7.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@

fn main() {
for thing(x[]) in foo {} //~ error: expected one of `,` or `@`, found `[`
//~^ ERROR: expected `in`, found `]`
//~^ ERROR expected one of `@` or `in`, found `[`
}

0 comments on commit a111297

Please sign in to comment.