From 38de4a74198938b48b83f13cad8cbdd7da557435 Mon Sep 17 00:00:00 2001 From: dkanus Date: Sun, 3 May 2026 02:43:23 +0700 Subject: [PATCH] Clean up expression parsing module --- .../primary_expressions.rs | 7 ++- rottlib/src/parser/errors.rs | 4 +- rottlib/src/parser/grammar/class.rs | 6 +- .../parser/grammar/expression/identifier.rs | 19 ++++--- .../src/parser/grammar/expression/literals.rs | 24 +++++--- .../src/parser/grammar/expression/pratt.rs | 56 ++++++++++++------- .../parser/grammar/expression/primary/mod.rs | 4 +- .../src/parser/grammar/function/definition.rs | 6 +- rottlib/src/parser/mod.rs | 1 + 9 files changed, 79 insertions(+), 48 deletions(-) diff --git a/rottlib/src/diagnostics/parse_error_diagnostics/primary_expressions.rs b/rottlib/src/diagnostics/parse_error_diagnostics/primary_expressions.rs index ab4a28d..135d191 100644 --- a/rottlib/src/diagnostics/parse_error_diagnostics/primary_expressions.rs +++ b/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; diff --git a/rottlib/src/parser/errors.rs b/rottlib/src/parser/errors.rs index 343187f..76bd6f9 100644 --- a/rottlib/src/parser/errors.rs +++ b/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. diff --git a/rottlib/src/parser/grammar/class.rs b/rottlib/src/parser/grammar/class.rs index 57be640..9f2b3d9 100644 --- a/rottlib/src/parser/grammar/class.rs +++ b/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) } diff --git a/rottlib/src/parser/grammar/expression/identifier.rs b/rottlib/src/parser/grammar/expression/identifier.rs index 3e88f73..db03430 100644 --- a/rottlib/src/parser/grammar/expression/identifier.rs +++ b/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, - /// e.g. `KFChar.ZombieClot`. + /// Parses an identifier path, optionally followed by dot-separated + /// segments. /// - /// This is used for name paths where each segment must be - /// a valid identifier and segments are separated by `.` tokens. + /// Accepts both a single identifier, such as `KFChar`, and + /// a qualified path, such as `KFChar.ZombieClot`. /// - /// On failure produces an error of specified [`ParseErrorKind`] - /// `invalid_identifier_error_kind`. + /// Produces `invalid_identifier_error_kind` when the first identifier or + /// 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; diff --git a/rottlib/src/parser/grammar/expression/literals.rs b/rottlib/src/parser/grammar/expression/literals.rs index 0626b61..6381cbb 100644 --- a/rottlib/src/parser/grammar/expression/literals.rs +++ b/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) - .map_err(|_| self.make_error_at_last_consumed(ParseErrorKind::InvalidNumericLiteral)) + u128::from_str_radix(&digits_without_underscores, base).map_err(|_| { + 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 - .parse::() - .map_err(|_| self.make_error_at_last_consumed(ParseErrorKind::InvalidNumericLiteral)) + content.parse::().map_err(|_| { + self.make_error_at(ParseErrorKind::InvalidNumericLiteral, literal_position) + }) } /// Unescapes a tokenized string literal into an arena string. diff --git a/rottlib/src/parser/grammar/expression/pratt.rs b/rottlib/src/parser/grammar/expression/pratt.rs index 8c090e4..e6af351 100644 --- a/rottlib/src/parser/grammar/expression/pratt.rs +++ b/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 - /// token is known not to be a valid expression starter, this reports - /// `bad_start_error_kind`, consumes the bad token, and starts panic-mode - /// recovery until [`crate::parser::SyncLevel::ExpressionStart`]. + /// 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(super) fn parse_required_expression( + pub(crate) 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 { - return None; - }; + let operator = ast::PostfixOperator::try_from(token).ok()?; Some((operator, token_position)) } diff --git a/rottlib/src/parser/grammar/expression/primary/mod.rs b/rottlib/src/parser/grammar/expression/primary/mod.rs index b5d8824..3fe2949 100644 --- a/rottlib/src/parser/grammar/expression/primary/mod.rs +++ b/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) } diff --git a/rottlib/src/parser/grammar/function/definition.rs b/rottlib/src/parser/grammar/function/definition.rs index 3402781..b9a4af3 100644 --- a/rottlib/src/parser/grammar/function/definition.rs +++ b/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() { - Some((Token::IntegerLiteral, lex)) => { + let value = match self.peek_token_lexeme_and_position() { + 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); diff --git a/rottlib/src/parser/mod.rs b/rottlib/src/parser/mod.rs index dba56c3..fc48375 100644 --- a/rottlib/src/parser/mod.rs +++ b/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