Clean up expression parsing module

rottlib/src/diagnostics/parse_error_diagnostics/primary_expressions.rs

@@ -1,6 +1,6 @@
 use super::{Diagnostic, DiagnosticBuilder, FoundAt, collapse_span_to_end_on_same_line, found_at};
 use crate::lexer::{TokenSpan, TokenizedFile};
-use crate::parser::ParseError;
+use crate::parser::{ParseError, diagnostic_labels};
 
 pub(super) fn diagnostic_parenthesized_expression_invalid_start<'src>(
     error: ParseError,
@@ -164,7 +164,10 @@ pub(super) fn diagnostic_class_type_expected_qualified_type_name<'src>(
     error: ParseError,
     file: &TokenizedFile<'src>,
 ) -> Diagnostic {
-    let qualifier_dot_span = error.related_spans.get("qualifier_dot").copied();
+    let qualifier_dot_span = error
+        .related_spans
+        .get(diagnostic_labels::QUALIFIED_IDENTIFIER_DOT)
+        .copied();
     let class_span = error.related_spans.get("class_keyword").copied();
 
     let blame_pos = error.blame_span.end;

rottlib/src/parser/errors.rs

@@ -100,6 +100,8 @@ pub enum ParseErrorKind {
     SwitchCaseMissingExpression,
     /// P0042
     SwitchCaseExpressionInvalidStart,
+    /// P0043
+    InvalidNumericLiteral,
     // ================== Old errors to be thrown away! ==================
     /// Found an unexpected token while parsing an expression.
     ExpressionUnexpectedToken,
@@ -119,8 +121,6 @@ pub enum ParseErrorKind {
     BlockMissingSemicolonAfterStatement,
     /// Unexpected end of input while parsing.
     UnexpectedEndOfFile,
-    /// Token looked like a numeric literal but could not be parsed as one.
-    InvalidNumericLiteral,
     /// A bare expression appeared in a `switch` arm but was not the final arm.
     ///
     /// Such an expression must be terminated with `;` or be the final arm.

rottlib/src/parser/grammar/class.rs

@@ -744,7 +744,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
                 let is_negative = matches!(token, Token::Minus);
                 self.advance();
 
-                let (next_token, next_lexeme, _) =
+                let (next_token, next_lexeme, token_position) =
                     self.require_token_lexeme_and_position(ParseErrorKind::InvalidNumericLiteral)?;
 
                 match next_token {
@@ -761,7 +761,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
                         let mut signed_lexeme = String::with_capacity(1 + next_lexeme.len());
                         signed_lexeme.push(if is_negative { '-' } else { '+' });
                         signed_lexeme.push_str(next_lexeme);
-                        let value = self.decode_float_literal(&signed_lexeme)?;
+                        let value = self.decode_float_literal(&signed_lexeme, token_position)?;
                         self.advance();
                         DeclarationLiteral::Float(value)
                     }
@@ -778,7 +778,7 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
                 DeclarationLiteral::Integer(value)
             }
             Token::FloatLiteral => {
-                let value = self.decode_float_literal(lexeme)?;
+                let value = self.decode_float_literal(lexeme, token_position)?;
                 self.advance();
                 DeclarationLiteral::Float(value)
             }

rottlib/src/parser/grammar/expression/identifier.rs

@@ -6,7 +6,7 @@
 use crate::arena::{self, ArenaVec};
 use crate::ast::{IdentifierToken, QualifiedIdentifier, QualifiedIdentifierRef};
 use crate::lexer::{self, Token, TokenSpan};
-use crate::parser::{ParseErrorKind, ParseResult, Parser, ResultRecoveryExt};
+use crate::parser::{ParseErrorKind, ParseResult, Parser, ResultRecoveryExt, diagnostic_labels};
 
 impl<'src, 'arena> Parser<'src, 'arena> {
     /// Parses an identifier.
@@ -39,14 +39,14 @@ impl<'src, 'arena> Parser<'src, 'arena> {
             .then_some(IdentifierToken(token_position))
     }
 
-    /// Parses a qualified (dot-separated) identifier path,
+    /// Parses an identifier path, optionally followed by dot-separated
-    /// e.g. `KFChar.ZombieClot`.
+    /// segments.
     ///
-    /// This is used for name paths where each segment must be
+    /// Accepts both a single identifier, such as `KFChar`, and
-    /// a valid identifier and segments are separated by `.` tokens.
+    /// a qualified path, such as `KFChar.ZombieClot`.
     ///
-    /// On failure produces an error of specified [`ParseErrorKind`]
+    /// Produces `invalid_identifier_error_kind` when the first identifier or
-    /// `invalid_identifier_error_kind`.
+    /// a segment after `.` is missing or invalid.
     pub(crate) fn parse_qualified_identifier(
         &mut self,
         invalid_identifier_error_kind: ParseErrorKind,
@@ -63,7 +63,10 @@ impl<'src, 'arena> Parser<'src, 'arena> {
                 .widen_error_span_from(head.0)
             {
                 Ok(next_segment) => next_segment,
-                Err(error) => return Err(error.related_token("qualifier_dot", dot_position)),
+                Err(error) => {
+                    return Err(error
+                        .related_token(diagnostic_labels::QUALIFIED_IDENTIFIER_DOT, dot_position));
+                }
             };
             span_end = next_segment.0;
 

rottlib/src/parser/grammar/expression/literals.rs

@@ -7,6 +7,7 @@
 //! The rules implemented here intentionally mirror the quirks of
 //! Unreal Engine 2’s `UnrealScript`.
 
+use crate::lexer::TokenPosition;
 use crate::parser::{ParseErrorKind, ParseResult};
 
 impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
@@ -24,7 +25,11 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
     ///
     /// On failure, returns [`ParseErrorKind::InvalidNumericLiteral`] at
     /// the parser's current cursor position.
-    pub(crate) fn decode_integer_literal(&self, literal: &str) -> ParseResult<'src, 'arena, u128> {
+    pub(crate) fn decode_integer_literal(
+        &self,
+        literal: &str,
+        literal_position: TokenPosition,
+    ) -> ParseResult<'src, 'arena, u128> {
         let (base, content) = match literal.split_at_checked(2) {
             Some(("0b" | "0B", stripped)) => (2, stripped),
             Some(("0o" | "0O", stripped)) => (8, stripped),
@@ -32,8 +37,9 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
             _ => (10, literal),
         };
         let digits_without_underscores = content.replace('_', "");
-        u128::from_str_radix(&digits_without_underscores, base)
+        u128::from_str_radix(&digits_without_underscores, base).map_err(|_| {
-            .map_err(|_| self.make_error_at_last_consumed(ParseErrorKind::InvalidNumericLiteral))
+            self.make_error_at(ParseErrorKind::InvalidNumericLiteral, literal_position)
+        })
     }
 
     /// Decodes a float literal as `f64`, following the permissive and only
@@ -66,7 +72,11 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
     /// On failure, this function returns
     /// [`ParseErrorKind::InvalidNumericLiteral`] at the current parser
     /// position.
-    pub(crate) fn decode_float_literal(&self, literal: &str) -> ParseResult<'src, 'arena, f64> {
+    pub(crate) fn decode_float_literal(
+        &self,
+        literal: &str,
+        literal_position: TokenPosition,
+    ) -> ParseResult<'src, 'arena, f64> {
         let content = literal
             .strip_suffix('f')
             .or_else(|| literal.strip_suffix('F'))
@@ -77,9 +87,9 @@ impl<'src, 'arena> crate::parser::Parser<'src, 'arena> {
             .nth(1)
             .and_then(|(period_index, _)| content.get(..period_index))
             .unwrap_or(content);
-        content
+        content.parse::<f64>().map_err(|_| {
-            .parse::<f64>()
+            self.make_error_at(ParseErrorKind::InvalidNumericLiteral, literal_position)
-            .map_err(|_| self.make_error_at_last_consumed(ParseErrorKind::InvalidNumericLiteral))
+        })
     }
 
     /// Unescapes a tokenized string literal into an arena string.

rottlib/src/parser/grammar/expression/pratt.rs

@@ -35,7 +35,7 @@
 use crate::ast::{self, Expression, ExpressionRef};
 use crate::lexer::TokenPosition;
 use crate::parser::{
-    self, ParseErrorKind, ParseExpressionResult, Parser, ResultRecoveryExt, diagnostic_labels,
+    ParseErrorKind, ParseExpressionResult, Parser, ResultRecoveryExt, SyncLevel, diagnostic_labels,
 };
 
 pub use super::precedence::PrecedenceRank;
@@ -59,34 +59,30 @@ fn forbids_postfix_operators(expression: &ExpressionRef<'_, '_>) -> bool {
 }
 
 impl<'src, 'arena> Parser<'src, 'arena> {
-    // TODO: success here guaranees progress
     /// Parses an expression.
     ///
     /// Always returns some expression node; any syntax errors are reported
     /// through the parser's diagnostics.
+    /// Returning non-erroneous node guarantees that parser has advanced
+    /// at least one token.
     #[must_use]
     pub fn parse_expression(&mut self) -> ExpressionRef<'src, 'arena> {
         self.parse_expression_with_min_precedence_rank(PrecedenceRank::LOOSEST)
-            .sync_error_until(self, parser::SyncLevel::ExpressionStart)
+            .sync_error_until(self, SyncLevel::ExpressionStart)
             .unwrap_or_fallback(self)
     }
 
     /// Parses an expression in a grammar position where an expression is
     /// required.
     ///
-    /// This is the checked variant of [`Parser::parse_expression`]. If the next
+    /// If the next token cannot start an expression, this reports
-    /// token is known not to be a valid expression starter, this reports
+    /// `bad_start_error_kind` at that token, starts expression-start recovery
-    /// `bad_start_error_kind`, consumes the bad token, and starts panic-mode
+    /// and returns an error expression result.
-    /// recovery until [`crate::parser::SyncLevel::ExpressionStart`].
     ///
     /// `required_by_position` identifies the token or construct that created
     /// the requirement for an expression. It is attached to the diagnostic with
     /// the [`diagnostic_labels::EXPRESSION_REQUIRED_BY`] label.
-    ///
+    pub(crate) fn parse_required_expression(
-    /// `expression_context_position` identifies the local syntactic anchor after
-    /// which the expression was expected. It is attached to the diagnostic with
-    /// the [`diagnostic_labels::EXPRESSION_EXPECTED_AFTER`] label.
-    pub(super) fn parse_required_expression(
         &mut self,
         bad_start_error_kind: ParseErrorKind,
         required_by_position: TokenPosition,
@@ -96,7 +92,7 @@ impl<'src, 'arena> Parser<'src, 'arena> {
 
             return Err(self
                 .make_error_at(bad_start_error_kind, error_position)
-                .sync_error_until(self, crate::parser::SyncLevel::ExpressionStart)
+                .sync_error_until(self, SyncLevel::ExpressionStart)
                 .blame_token(error_position)
                 .related_token(
                     diagnostic_labels::EXPRESSION_REQUIRED_BY,
@@ -107,7 +103,22 @@ impl<'src, 'arena> Parser<'src, 'arena> {
         self.parse_expression_with_min_precedence_rank(PrecedenceRank::LOOSEST)
     }
 
-    pub(super) fn parse_required_expression_with_context(
+    /// Parses an expression in a grammar position where an expression is
+    /// required, and records the local syntactic context where it was expected.
+    ///
+    /// If the next token cannot start an expression, this reports
+    /// `bad_start_error_kind` at that token, starts expression-start recovery,
+    /// and returns an error expression result.
+    ///
+    /// `required_by_position` identifies the token or construct that created
+    /// the requirement for an expression. It is attached to the diagnostic with
+    /// the [`diagnostic_labels::EXPRESSION_REQUIRED_BY`] label.
+    ///
+    /// `expression_context_position` identifies the local syntactic anchor
+    /// after which the expression was expected. It is attached to
+    /// the diagnostic with the [`diagnostic_labels::EXPRESSION_EXPECTED_AFTER`]
+    /// label.
+    pub(crate) fn parse_required_expression_with_context(
         &mut self,
         bad_start_error_kind: ParseErrorKind,
         required_by_position: TokenPosition,
@@ -118,7 +129,7 @@ impl<'src, 'arena> Parser<'src, 'arena> {
 
             return Err(self
                 .make_error_at(bad_start_error_kind, error_position)
-                .sync_error_until(self, crate::parser::SyncLevel::ExpressionStart)
+                .sync_error_until(self, SyncLevel::ExpressionStart)
                 .blame_token(error_position)
                 .related_token(
                     diagnostic_labels::EXPRESSION_REQUIRED_BY,
@@ -133,6 +144,11 @@ impl<'src, 'arena> Parser<'src, 'arena> {
         self.parse_expression_with_min_precedence_rank(PrecedenceRank::LOOSEST)
     }
 
+    /// Creates an error expression at `position`.
+    ///
+    /// The returned node has a single-token span anchored at `position` and
+    /// can be used as a fallback expression when parsing cannot produce
+    /// a valid one.
     pub(super) fn make_error_expression_at(
         &self,
         position: TokenPosition,
@@ -149,7 +165,7 @@ impl<'src, 'arena> Parser<'src, 'arena> {
     fn parse_expression_with_min_precedence_rank(
         &mut self,
         min_precedence_rank: PrecedenceRank,
-    ) -> parser::ParseExpressionResult<'src, 'arena> {
+    ) -> ParseExpressionResult<'src, 'arena> {
         let mut left_hand_side = self.parse_prefix_or_primary()?;
         left_hand_side = self.parse_selectors_after(left_hand_side)?;
         // We disallow only postfix operators after expression forms that
@@ -174,7 +190,7 @@ impl<'src, 'arena> Parser<'src, 'arena> {
 
     /// Parses a prefix or primary expression (Pratt parser's "nud" or
     /// null denotation).
-    fn parse_prefix_or_primary(&mut self) -> parser::ParseExpressionResult<'src, 'arena> {
+    fn parse_prefix_or_primary(&mut self) -> ParseExpressionResult<'src, 'arena> {
         let (token, token_lexeme, token_position) =
             self.require_token_lexeme_and_position(ParseErrorKind::ExpressionExpected)?;
         // Avoid advancing over an obviously wrong token;
@@ -226,7 +242,7 @@ impl<'src, 'arena> Parser<'src, 'arena> {
         &mut self,
         mut left_hand_side: ExpressionRef<'src, 'arena>,
         min_precedence_rank: PrecedenceRank,
-    ) -> parser::ParseExpressionResult<'src, 'arena> {
+    ) -> ParseExpressionResult<'src, 'arena> {
         while let Some((operator, right_precedence_rank)) =
             self.peek_infix_with_min_precedence_rank(min_precedence_rank)
         {
@@ -249,9 +265,7 @@ impl<'src, 'arena> Parser<'src, 'arena> {
         &mut self,
     ) -> Option<(ast::PostfixOperator, crate::lexer::TokenPosition)> {
         let (token, token_position) = self.peek_token_and_position()?;
-        let Ok(operator) = ast::PostfixOperator::try_from(token) else {
+        let operator = ast::PostfixOperator::try_from(token).ok()?;
-            return None;
-        };
         Some((operator, token_position))
     }
 

rottlib/src/parser/grammar/expression/primary/mod.rs

@@ -84,12 +84,12 @@ impl<'src, 'arena> Parser<'src, 'arena> {
     ) -> ParseExpressionResult<'src, 'arena> {
         Ok(match token {
             Token::IntegerLiteral => {
-                let value = self.decode_integer_literal(token_lexeme)?;
+                let value = self.decode_integer_literal(token_lexeme, token_position)?;
                 self.arena
                     .alloc_node_at(Expression::Integer(value), token_position)
             }
             Token::FloatLiteral => {
-                let value = self.decode_float_literal(token_lexeme)?;
+                let value = self.decode_float_literal(token_lexeme, token_position)?;
                 self.arena
                     .alloc_node_at(Expression::Float(value), token_position)
             }

rottlib/src/parser/grammar/function/definition.rs

@@ -239,10 +239,10 @@ impl<'src, 'arena> Parser<'src, 'arena> {
             return None;
         }
 
-        let value = match self.peek_token_and_lexeme() {
+        let value = match self.peek_token_lexeme_and_position() {
-            Some((Token::IntegerLiteral, lex)) => {
+            Some((Token::IntegerLiteral, lex, token_position)) => {
                 self.advance();
-                self.decode_integer_literal(lex).ok_or_report(self)
+                self.decode_integer_literal(lex, token_position).ok_or_report(self)
             }
             Some(_) => {
                 self.report_error_here(ParseErrorKind::OperatorPrecedenceNotIntegerLiteral);

rottlib/src/parser/mod.rs

@@ -46,6 +46,7 @@ pub type ParseExpressionResult<'src, 'arena> =
 pub(crate) mod diagnostic_labels {
     pub(crate) const EXPRESSION_REQUIRED_BY: &str = "expression_required_by";
     pub(crate) const EXPRESSION_EXPECTED_AFTER: &str = "expression_expected_after";
+    pub(crate) const QUALIFIED_IDENTIFIER_DOT: &str = "qualifier_dot";
 }
 
 // TODO: add some kind of bailing for infinite loops