Split apart UnrecognizedEOF error variant from UnrecognizedToken (lalrpop#446)

* Split apart UnrecognizedEOF error variant from UnrecognizedToken

* Factor out fmt_expected function

* Initial attempt at fixing codegen

- Match on lookahead when returning error
- Use last fixed symbol as location of UnrecognizedEOF
- Use `Default::default()` when there are no fixed symbols (?)

* Initial implementation of finding latest optional location

* Fix UnrecognizedToken errors in `Some` test cases

* Fix UnrecognizedEOF test cases

* Fix parentheses around pattern error for 1.26 compatibility

* Add basic test for UnrecognizedEOF (and hopefully trigger Travis build)

* Fix emitted indentation and use travis_wait when testing

Author: nwtnni

.travis.yml

@@ -10,7 +10,7 @@ install:
 - bash tools/ci-install.sh
 script:
 - RUST_BACKTRACE=1 CARGO_INCREMENTAL=0 cargo build -p lalrpop
-- RUST_BACKTRACE=1 CARGO_INCREMENTAL=0 cargo test --all --all-features
+- RUST_BACKTRACE=1 CARGO_INCREMENTAL=0 travis_wait cargo test --all --all-features
 - bash tools/build-doc
 deploy:
   provider: pages

lalrpop-test/src/lib.rs

@@ -240,6 +240,18 @@ fn parse_error_map_err() {
     }
 }
 
+#[test]
+fn parse_error_eof_location() {
+    match expr_intern_tok::ExprParser::new().parse(1, "1 - ") {
+        Err(ParseError::UnrecognizedEOF { location, .. }) => {
+            assert_eq!(location, 3);
+        }
+        _ => {
+            panic!("Expected an UnrecognizedEOF error");
+        }
+    }
+}
+
 #[test]
 fn display_parse_error() {
     let expr = "(1+\n(2++3))";
@@ -483,8 +495,8 @@ fn error_recovery_eof() {
     assert_eq!(
         errors.borrow()[0],
         ErrorRecovery {
-            error: ParseError::UnrecognizedToken {
-                token: None,
+            error: ParseError::UnrecognizedEOF {
+                location: (),
                 expected: vec!["\"-\"".to_string()],
             },
             dropped_tokens: vec![],
@@ -499,8 +511,8 @@ fn error_recovery_eof_without_recovery() {
     let result = error_recovery::ItemParser::new().parse(&errors, tokens);
     assert_eq!(
         result,
-        Err(ParseError::UnrecognizedToken {
-            token: None,
+        Err(ParseError::UnrecognizedEOF {
+            location: (),
             expected: vec!["\"-\"".to_string()],
         })
     );
@@ -520,7 +532,7 @@ fn error_recovery_extra_token() {
         errors.borrow()[0],
         ErrorRecovery {
             error: ParseError::UnrecognizedToken {
-                token: Some(((), Tok::Plus, ())),
+                token: ((), Tok::Plus, ()),
                 expected: vec!["\")\"".to_string()],
             },
             dropped_tokens: vec![((), Tok::Plus, ())],
@@ -542,7 +554,7 @@ fn error_recovery_dont_drop_unrecognized_token() {
         errors.borrow()[0],
         ErrorRecovery {
             error: ParseError::UnrecognizedToken {
-                token: Some(((), Tok::RParen, ())),
+                token: ((), Tok::RParen, ()),
                 expected: vec!["\"-\"".to_string()],
             },
             dropped_tokens: vec![],
@@ -564,7 +576,7 @@ fn error_recovery_multiple_extra_tokens() {
         errors.borrow()[0],
         ErrorRecovery {
             error: ParseError::UnrecognizedToken {
-                token: Some(((), Tok::Plus, ())),
+                token: ((), Tok::Plus, ()),
                 expected: vec!["\")\"".to_string()],
             },
             dropped_tokens: vec![((), Tok::Plus, ()), ((), Tok::Plus, ())],
@@ -615,7 +627,7 @@ fn error_recovery_issue_240() {
         errors,
         vec![ErrorRecovery {
             error: ParseError::UnrecognizedToken {
-                token: Some((6, Tok::Div, 7)),
+                token: (6, Tok::Div, 7),
                 expected: vec!["\")\"".to_string()],
             },
             dropped_tokens: vec![(6, Tok::Div, 7)],

lalrpop-util/src/lib.rs

@@ -9,23 +9,29 @@ pub enum ParseError<L, T, E> {
     /// expect.
     InvalidToken { location: L },
 
-    /// Generated by the parser when it encounters a token (or EOF) it did not
-    /// expect.
+    /// Generated by the parser when it encounters an EOF it did not expect.
+    UnrecognizedEOF {
+        /// The end of the final token
+        location: L,
+
+        /// The set of expected tokens: these names are taken from the
+        /// grammar and hence may not necessarily be suitable for
+        /// presenting to the user.
+        expected: Vec<String>,
+    },
+
+    /// Generated by the parser when it encounters a token it did not expect.
     UnrecognizedToken {
-        /// If this is `Some`, then an unexpected token of type `T`
-        /// was observed, with a span given by the two `L` values. If
-        /// this is `None`, then EOF was observed when it was not
-        /// expected.
-        token: Option<(L, T, L)>,
+        /// The unexpected token of type `T` with a span given by the two `L` values.
+        token: (L, T, L),
 
         /// The set of expected tokens: these names are taken from the
         /// grammar and hence may not necessarily be suitable for
         /// presenting to the user.
         expected: Vec<String>,
     },
 
-    /// Generated by the parser when it encounters additional,
-    /// unexpected tokens.
+    /// Generated by the parser when it encounters additional, unexpected tokens.
     ExtraToken { token: (L, T, L) },
 
     /// Custom error type.
@@ -49,8 +55,12 @@ impl<L, T, E> ParseError<L, T, E> {
             ParseError::InvalidToken { location } => ParseError::InvalidToken {
                 location: loc_op(location),
             },
+            ParseError::UnrecognizedEOF { location, expected } => ParseError::UnrecognizedEOF {
+                location: loc_op(location),
+                expected: expected,
+            },
             ParseError::UnrecognizedToken { token, expected } => ParseError::UnrecognizedToken {
-                token: token.map(maptok),
+                token: maptok(token),
                 expected: expected,
             },
             ParseError::ExtraToken { token } => ParseError::ExtraToken {
@@ -84,6 +94,23 @@ impl<L, T, E> ParseError<L, T, E> {
     }
 }
 
+/// Format a list of expected tokens.
+fn fmt_expected(f: &mut fmt::Formatter, expected: &[String]) -> fmt::Result {
+    if !expected.is_empty() {
+        try!(writeln!(f, ""));
+        for (i, e) in expected.iter().enumerate() {
+            let sep = match i {
+                0 => "Expected one of",
+                _ if i < expected.len() - 1 => ",",
+                // Last expected message to be written
+                _ => " or",
+            };
+            try!(write!(f, "{} {}", sep, e));
+        }
+    }
+    Ok(())
+}
+
 impl<L, T, E> fmt::Display for ParseError<L, T, E>
 where
     L: fmt::Display,
@@ -93,37 +120,19 @@ where
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         use self::ParseError::*;
         match *self {
+            User { ref error } => write!(f, "{}", error),
             InvalidToken { ref location } => write!(f, "Invalid token at {}", location),
-            UnrecognizedToken {
-                ref token,
-                ref expected,
-            } => {
-                match *token {
-                    Some((ref start, ref token, ref end)) => try!(write!(
-                        f,
-                        "Unrecognized token `{}` found at {}:{}",
-                        token, start, end
-                    )),
-                    None => try!(write!(f, "Unrecognized EOF")),
-                }
-                if !expected.is_empty() {
-                    try!(writeln!(f, ""));
-                    for (i, e) in expected.iter().enumerate() {
-                        let sep = match i {
-                            0 => "Expected one of",
-                            _ if i < expected.len() - 1 => ",",
-                            // Last expected message to be written
-                            _ => " or",
-                        };
-                        try!(write!(f, "{} {}", sep, e));
-                    }
-                }
-                Ok(())
+            UnrecognizedEOF { ref location, ref expected } => {
+                try!(write!(f, "Unrecognized EOF found at {}", location));
+                fmt_expected(f, expected)
+            }
+            UnrecognizedToken { token: (ref start, ref token, ref end), ref expected } => {
+                try!(write!(f, "Unrecognized token `{}` found at {}:{}", token, start, end));
+                fmt_expected(f, expected)
+            }
+            ExtraToken { token: (ref start, ref token, ref end), } => {
+                write!(f, "Extra token {} found at {}:{}", token, start, end)
             }
-            ExtraToken {
-                token: (ref start, ref token, ref end),
-            } => write!(f, "Extra token {} found at {}:{}", token, start, end),
-            User { ref error } => write!(f, "{}", error),
         }
     }
 }
@@ -189,7 +198,7 @@ mod tests {
     #[test]
     fn test() {
         let err = ParseError::UnrecognizedToken::<i32, &str, &str> {
-            token: Some((1, "t0", 2)),
+            token: (1, "t0", 2),
             expected: vec!["t1", "t2", "t3"]
                 .into_iter()
                 .map(|s| s.to_string())

lalrpop-util/src/state_machine.rs

@@ -599,9 +599,19 @@ where
         token: Option<TokenTriple<D>>,
         top_state: D::StateIndex,
     ) -> ParseError<D> {
-        ::ParseError::UnrecognizedToken {
-            token: token,
-            expected: self.definition.expected_tokens(top_state),
+        match token {
+            Some(token) => {
+                ::ParseError::UnrecognizedToken {
+                    token: token,
+                    expected: self.definition.expected_tokens(top_state),
+                }
+            }
+            None => {
+                ::ParseError::UnrecognizedEOF {
+                    location: self.last_location.clone(),
+                    expected: self.definition.expected_tokens(top_state),
+                }
+            }
         }
     }
 

lalrpop/src/build/mod.rs

@@ -206,20 +206,19 @@ fn parse_and_normalize_grammar(session: &Session, file_text: &FileText) -> io::R
             );
         }
 
-        Err(ParseError::UnrecognizedToken {
-            token: None,
+        Err(ParseError::UnrecognizedEOF {
+            location,
             expected: _,
         }) => {
-            let len = file_text.text().len();
             report_error(
                 &file_text,
-                pt::Span(len, len),
+                pt::Span(location, location),
                 &format!("unexpected end of file"),
             );
         }
 
         Err(ParseError::UnrecognizedToken {
-            token: Some((lo, _, hi)),
+            token: (lo, _, hi),
             expected,
         }) => {
             let _ = expected; // didn't implement this yet :)

lalrpop/src/lr1/codegen/ascent.rs

@@ -374,20 +374,54 @@ impl<'ascent, 'grammar, W: Write>
                 .iter()
                 .any(|&(ref t, _)| t.contains(&Token::Terminal(terminal.clone())))
         });
-        rust!(
-            self.out,
-            "return Err({}lalrpop_util::ParseError::UnrecognizedToken {{",
-            self.prefix
-        );
-        rust!(self.out, "token: {}lookahead,", self.prefix);
-        rust!(self.out, "expected: vec![");
+
+        rust!(self.out, "let {}expected = vec![", self.prefix);
         for terminal in successful_terminals {
             rust!(self.out, "r###\"{}\"###.to_string(),", terminal);
         }
-        rust!(self.out, "]");
-        rust!(self.out, "}});");
+        rust!(self.out, "];");
+
+        // check if we've found an unrecognized token or EOF
+        rust!(self.out, "return Err(");
+        rust!(self.out, "match {}lookahead {{", self.prefix);
+
+        rust!(self.out, "Some({}token) => {{", self.prefix);
+        rust!(self.out, "{}lalrpop_util::ParseError::UnrecognizedToken {{", self.prefix);
+        rust!(self.out, "token: {}token,", self.prefix);
+        rust!(self.out, "expected: {}expected,", self.prefix);
+        rust!(self.out, "}}");
+        rust!(self.out, "}}");
+
+        rust!(self.out, "None => {{");
+
+        // find the location of the last symbol on stack
+        let (optional, fixed) = stack_suffix.optional_fixed_lens();
+        if fixed > 0 {
+            rust!(self.out, "let {}location = {}sym{}.2.clone();", self.prefix, self.prefix, stack_suffix.len() - 1);
+        } else if optional > 0 {
+            rust!(self.out, "let {}location = ", self.prefix);
+            for index in (0..optional).rev() {
+                rust!(self.out, "{}sym{}.as_ref().map(|sym| sym.2.clone()).unwrap_or_else(|| {{", self.prefix, index);
+            }
+            rust!(self.out, "Default::default()");
+            for _ in 0..optional {
+                rust!(self.out, "}})");
+            }
+            rust!(self.out, ";");
+        } else {
+            rust!(self.out, "let {}location = Default::default();", self.prefix);
+        }
+
+        rust!(self.out, "{}lalrpop_util::ParseError::UnrecognizedEOF {{", self.prefix);
+        rust!(self.out, "location: {}location,", self.prefix);
+        rust!(self.out, "expected: {}expected,", self.prefix);
         rust!(self.out, "}}");
+        rust!(self.out, "}}");
+
+        rust!(self.out, "}}"); // Error match
+        rust!(self.out, ")");
 
+        rust!(self.out, "}}"); // Wildcard match case
         rust!(self.out, "}}"); // match
 
         // finally, emit gotos (if relevant)