tamer: Add Display impl for each ParseState for generic ParseErrors

This is intended to describe, to the user, the state that the parser is
in.  This will be used to convey additional information for general parser
errors, but it should also probably be integrated into parsers' individual
errors as well when appropriate.

This is something I expected to add at some point, but I wanted to add them
because, when dealing with lowering errors, it can be difficult to tell
what parser the error originated from.

DEV-11864

tamer/src/diagnose.rs

@@ -163,6 +163,12 @@ impl<'a> From<String> for Label<'a> {
     }
 }
 
+impl<'a> From<&'a String> for Label<'a> {
+    fn from(s: &'a String) -> Self {
+        Self(Cow::Borrowed(s))
+    }
+}
+
 impl<'a> From<&'a str> for Label<'a> {
     fn from(s: &'a str) -> Self {
         Self(Cow::Borrowed(s))

tamer/src/obj/xmlo/reader.rs

@@ -17,6 +17,8 @@
 //  You should have received a copy of the GNU General Public License
 //  along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+use std::fmt::Display;
+
 use super::{SymAttrs, XmloError};
 use crate::{
     num::{Dim, Dtype},
@@ -254,6 +256,26 @@ impl<SS: XmloState, SD: XmloState, SF: XmloState> ParseState
     }
 }
 
+impl<SS: XmloState, SD: XmloState, SF: XmloState> Display
+    for XmloReader<SS, SD, SF>
+{
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use XmloReader::*;
+
+        match self {
+            Ready => write!(f, "awaiting xmlo input"),
+            Package => write!(f, "processing package attributes"),
+            Symtable(_, ss) => Display::fmt(ss, f),
+            SymDepsExpected => write!(f, "expecting symbol dependency list"),
+            SymDeps(_, sd) => Display::fmt(sd, f),
+            FragmentsExpected => write!(f, "expecting start of fragments"),
+            Fragments(_, sf) => Display::fmt(sf, f),
+            Eoh => write!(f, "finished parsing header"),
+            Done => write!(f, "finished parsing xmlo file"),
+        }
+    }
+}
+
 /// Symbol table parser operating within a delimited context.
 ///
 /// This parser expects a parent [`ParseState`] to indicate when symtable
@@ -511,6 +533,24 @@ impl SymtableState {
     }
 }
 
+impl Display for SymtableState {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use SymtableState::*;
+
+        match self {
+            Ready => write!(f, "expecting symbol declaration"),
+            Sym(_, Some(sym), _) => write!(f, "parsing symbol `{sym}`"),
+            Sym(_, None, _) => write!(f, "parsing a symbol"),
+            SymMapFrom(_, sym, _, _) => {
+                write!(f, "expecting map name for symbol `{sym}`")
+            }
+            SymRef(_, sym, _, _) => {
+                write!(f, "parsing refs for symbol `{sym}`")
+            }
+        }
+    }
+}
+
 impl From<(SymbolId, SymAttrs, Span)> for XmloToken {
     fn from(tup: (SymbolId, SymAttrs, Span)) -> Self {
         match tup {
@@ -610,6 +650,29 @@ impl ParseState for SymDepsState {
     }
 }
 
+impl Display for SymDepsState {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use SymDepsState::*;
+
+        match self {
+            Ready => write!(f, "expecting symbol table entry"),
+            SymUnnamed(_) => write!(f, "expecting name for symbol table entry"),
+            Sym(_, sym) => write!(
+                f,
+                "expecting dependencies for symbol table entry `{sym}`"
+            ),
+            SymRefUnnamed(_, sym, _) => write!(
+                f,
+                "expecting dependency name for symbol table entry `{sym}`"
+            ),
+            SymRefDone(_, sym, _) => write!(
+                f,
+                "expending end of dependency for symbol table entry `{sym}`"
+            ),
+        }
+    }
+}
+
 /// Text fragment (compiled code) parser for `preproc:fragments` children.
 ///
 /// This parser expects a parent [`ParseState`] to indicate when dependency
@@ -698,5 +761,24 @@ impl ParseState for FragmentsState {
     }
 }
 
+impl Display for FragmentsState {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use FragmentsState::*;
+
+        match self {
+            Ready => write!(f, "expecting fragment"),
+            FragmentUnnamed(_) => {
+                write!(f, "expecting fragment association id")
+            }
+            Fragment(_, sym) => {
+                write!(f, "expecting fragment text for symbol `{sym}`")
+            }
+            FragmentDone(_, sym) => {
+                write!(f, "expecting end of fragment for symbol `{sym}`")
+            }
+        }
+    }
+}
+
 #[cfg(test)]
 mod test;

tamer/src/parse.rs

@@ -113,7 +113,7 @@ where
 /// Whatever the underlying automaton,
 ///   a `(state, token, context)` triple must uniquely determine the next
 ///   parser action.
-pub trait ParseState: Default + PartialEq + Eq + Debug {
+pub trait ParseState: Default + PartialEq + Eq + Display + Debug {
     /// Input tokens to the parser.
     type Token: Token;
 
@@ -642,6 +642,7 @@ impl<S: ParseState, I: TokenStream<S::Token>> Parser<S, I> {
             let endpoints = self.last_span.endpoints();
             Err(ParseError::UnexpectedEof(
                 endpoints.1.unwrap_or(endpoints.0),
+                self.state.to_string(),
             ))
         }
     }
@@ -672,7 +673,10 @@ impl<S: ParseState, I: TokenStream<S::Token>> Parser<S, I> {
             // Nothing handled this dead state,
             //   and we cannot discard a lookahead token,
             //   so we have no choice but to produce an error.
-            Ok(Dead(invalid)) => Err(ParseError::UnexpectedToken(invalid)),
+            Ok(Dead(invalid)) => Err(ParseError::UnexpectedToken(
+                invalid,
+                self.state.to_string(),
+            )),
 
             Ok(parsed @ (Incomplete | Object(..))) => Ok(parsed.into()),
             Err(e) => Err(e.into()),
@@ -835,7 +839,12 @@ pub enum ParseError<T: Token, E: Diagnostic + PartialEq> {
     /// If this parser follows another,
     ///   then the combinator ought to substitute a missing span with
     ///   whatever span preceded this invocation.
-    UnexpectedEof(Span),
+    ///
+    /// The string is intended to describe what was expected to have been
+    ///   available based on the current [`ParseState`].
+    /// It is a heap-allocated string so that a copy of [`ParseState`]
+    ///   needn't be stored.
+    UnexpectedEof(Span, String),
 
     /// The parser reached an unhandled dead state.
     ///
@@ -844,11 +853,11 @@ pub enum ParseError<T: Token, E: Diagnostic + PartialEq> {
     /// If that does not occur,
     ///   [`Parser`] produces this error.
     ///
-    /// In the future,
+    /// The string is intended to describe what was expected to have been
-    ///   it may be desirable to be able to query [`ParseState`] for what
+    ///   available based on the current [`ParseState`].
-    ///   tokens are acceptable at this point,
+    /// It is a heap-allocated string so that a copy of [`ParseState`]
-    ///     to provide better error messages.
+    ///   needn't be stored.
-    UnexpectedToken(T),
+    UnexpectedToken(T, String),
 
     /// A parser-specific error associated with an inner
     ///   [`ParseState`].
@@ -864,8 +873,8 @@ impl<T: Token, EA: Diagnostic + PartialEq> ParseError<T, EA> {
     {
         use ParseError::*;
         match self {
-            UnexpectedEof(x) => UnexpectedEof(x),
+            UnexpectedEof(span, desc) => UnexpectedEof(span, desc),
-            UnexpectedToken(x) => UnexpectedToken(x),
+            UnexpectedToken(x, desc) => UnexpectedToken(x, desc),
             StateError(e) => StateError(e.into()),
         }
     }
@@ -880,11 +889,11 @@ impl<T: Token, E: Diagnostic + PartialEq> From<E> for ParseError<T, E> {
 impl<T: Token, E: Diagnostic + PartialEq> Display for ParseError<T, E> {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         match self {
-            Self::UnexpectedEof(_) => {
+            Self::UnexpectedEof(_, desc) => {
-                write!(f, "unexpected end of input")
+                write!(f, "unexpected end of input while {desc}")
             }
-            Self::UnexpectedToken(_tok) => {
+            Self::UnexpectedToken(_, desc) => {
-                write!(f, "unexpected input")
+                write!(f, "unexpected input while {desc}")
             }
             Self::StateError(e) => Display::fmt(e, f),
         }
@@ -907,14 +916,9 @@ impl<T: Token, E: Diagnostic + PartialEq + 'static> Diagnostic
         use ParseError::*;
 
         match self {
-            // TODO: More information from the underlying parser on what was expected.
+            UnexpectedEof(span, desc) => span.error(desc).into(),
-            UnexpectedEof(span) => {
-                span.error("unexpected end of input here").into()
-            }
 
-            UnexpectedToken(tok) => {
+            UnexpectedToken(tok, desc) => tok.span().error(desc).into(),
-                tok.span().error("this was unexpected").into()
-            }
 
             // TODO: Is there any additional useful context we can augment
             //   this with?
@@ -1121,6 +1125,12 @@ pub mod test {
         }
     }
 
+    impl Display for EchoState {
+        fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+            write!(f, "<EchoState as Display>::fmt")
+        }
+    }
+
     #[derive(Debug, PartialEq, Eq)]
     enum EchoStateError {
         InnerError(TestToken),
@@ -1183,7 +1193,12 @@ pub mod test {
         //     state,
         //   we must fail when we encounter the end of the stream.
         assert_eq!(
-            Some(Err(ParseError::UnexpectedEof(span.endpoints().1.unwrap()))),
+            Some(Err(ParseError::UnexpectedEof(
+                span.endpoints().1.unwrap(),
+                // All the states have the same string
+                //   (at time of writing).
+                EchoState::default().to_string(),
+            ))),
             sut.next()
         );
     }
@@ -1238,7 +1253,7 @@ pub mod test {
         let result = sut.finalize();
         assert_matches!(
             result,
-            Err((_, ParseError::UnexpectedEof(s))) if s == span.endpoints().1.unwrap()
+            Err((_, ParseError::UnexpectedEof(s, _))) if s == span.endpoints().1.unwrap()
         );
 
         // The sut should have been re-returned,
@@ -1266,7 +1281,13 @@ pub mod test {
         //   which is unhandled by any parent context
         //     (since we're not composing parsers),
         //     which causes an error due to an unhandled Dead state.
-        assert_eq!(sut.next(), Some(Err(ParseError::UnexpectedToken(tok))),);
+        assert_eq!(
+            sut.next(),
+            Some(Err(ParseError::UnexpectedToken(
+                tok,
+                EchoState::default().to_string()
+            ))),
+        );
     }
 
     // A context can be both retrieved from a finished parser and provided

tamer/src/xir/attr/parse.rs

@@ -87,6 +87,19 @@ impl Default for AttrParseState {
     }
 }
 
+impl Display for AttrParseState {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use AttrParseState::*;
+
+        match self {
+            Empty => write!(f, "expecting an attribute"),
+            Name(name, _) => {
+                write!(f, "expecting an attribute value for {name}")
+            }
+        }
+    }
+}
+
 /// Attribute parsing error.
 #[derive(Debug, PartialEq, Eq)]
 pub enum AttrParseError {

tamer/src/xir/flat.rs

@@ -255,6 +255,22 @@ where
     }
 }
 
+impl<const MAX_DEPTH: usize, SA> Display for State<MAX_DEPTH, SA>
+where
+    SA: FlatAttrParseState<MAX_DEPTH>,
+{
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use State::*;
+
+        match self {
+            PreRoot => write!(f, "expecting document root"),
+            NodeExpected => write!(f, "expecting a node"),
+            AttrExpected(sa) => Display::fmt(sa, f),
+            Done => write!(f, "done parsing"),
+        }
+    }
+}
+
 impl<const MAX_DEPTH: usize, SA> State<MAX_DEPTH, SA>
 where
     SA: FlatAttrParseState<MAX_DEPTH>,

tamer/src/xir/flat/test.rs

@@ -22,6 +22,8 @@
 //! These tests take place within the context of the XIR parsing framework,
 //!   so they are one layer of abstraction away from unit tests.
 
+use std::assert_matches::assert_matches;
+
 use super::*;
 use crate::convert::ExpectInto;
 use crate::parse::{ParseError, Parsed};
@@ -87,9 +89,12 @@ fn extra_closing_tag() {
 
     let sut = parse::<1>(toks);
 
-    assert_eq!(
+    assert_matches!(
-        Err(ParseError::UnexpectedToken(XirToken::Close(Some(name), S3),)),
+        sut.collect::<Result<Vec<Parsed<Object>>, _>>(),
-        sut.collect::<Result<Vec<Parsed<Object>>, _>>()
+        Err(ParseError::UnexpectedToken(
+            XirToken::Close(Some(given_name), given_span),
+            _
+        )) if given_name == name && given_span == S3
     );
 }
 
@@ -109,9 +114,10 @@ fn extra_self_closing_tag() {
 
     let sut = parse::<1>(toks);
 
-    assert_eq!(
+    assert_matches!(
-        Err(ParseError::UnexpectedToken(XirToken::Close(None, S3),)),
+        sut.collect::<Result<Vec<Parsed<Object>>, _>>(),
-        sut.collect::<Result<Vec<Parsed<Object>>, _>>()
+        Err(ParseError::UnexpectedToken(XirToken::Close(None, given_span), _))
+            if given_span == S3,
     );
 }
 
@@ -364,7 +370,7 @@ fn not_accepting_state_if_element_open() {
     );
 
     // Element was not closed.
-    assert_eq!(Some(Err(ParseError::UnexpectedEof(S))), sut.next());
+    assert_matches!(sut.next(), Some(Err(ParseError::UnexpectedEof(..))));
 }
 
 // XML permits comment nodes before and after the document root element.
@@ -417,11 +423,11 @@ fn content_after_root_close_error() {
 
     let sut = parse::<1>(toks);
 
-    assert_eq!(
+    assert_matches!(
+        sut.collect(),
         Result::<Vec<Parsed<Object>>, _>::Err(ParseError::UnexpectedToken(
-            XirToken::Open(name, S3)
+            XirToken::Open(given_name, given_span),
-        )),
+        _)) if given_name == name && given_span == S3
-        sut.collect()
     );
 }
 

tamer/src/xir/tree.rs

@@ -617,6 +617,27 @@ impl<SA: StackAttrParseState> ParseState for Stack<SA> {
     }
 }
 
+impl<SA: StackAttrParseState> Display for Stack<SA> {
+    fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+        use Stack::*;
+
+        // These are not very descriptive because XIRT is only used in tests
+        //   at the time of writing.
+        // If this results in user-facing errors in the future,
+        //   we should probably output more information.
+        match self {
+            Empty => write!(f, "waiting for a node"),
+            BuddingElement(_) => write!(f, "parsing an element"),
+            ClosedElement(_) => {
+                write!(f, "completing processing of an element")
+            }
+            BuddingAttrList(_, _) => write!(f, "parsing attribute list"),
+            AttrState(_, _, sa) => Display::fmt(sa, f),
+            Done => write!(f, "done parsing"),
+        }
+    }
+}
+
 impl<SA: StackAttrParseState> Stack<SA> {
     fn begin_attrs(
         name: QName,

tamer/src/xir/tree/test.rs

@@ -17,6 +17,8 @@
 //  You should have received a copy of the GNU General Public License
 //  along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+use std::assert_matches::assert_matches;
+
 use super::super::parse::ParseError;
 use super::*;
 use crate::convert::ExpectInto;
@@ -384,9 +386,10 @@ fn attr_parser_with_non_attr_token() {
 
     let mut sut = attr_parser_from(&mut toks);
 
-    assert_eq!(
+    assert_matches!(
         sut.next(),
-        Some(Err(ParseError::UnexpectedToken(Token::Open(name, *S))))
+        Some(Err(ParseError::UnexpectedToken(Token::Open(given_name, given_span), _)))
+            if given_name == name && given_span == *S
     );
 }
 
@@ -416,11 +419,11 @@ fn parser_attr_multiple() {
     //   after which some other parser can continue on the same token
     //   stream
     //     (using this token as a lookahead).
-    assert_eq!(
+    assert_matches!(
         sut.next(),
         Some(Err(ParseError::UnexpectedToken(Token::Text(
-            "nohit".into(),
+            given_name,
-            *S
+            given_span,
-        ))))
+        ), _))) if given_name == "nohit".into() && given_span == *S
     );
 }