From 310cf0d2f9c9ae12e1e346cf2f4f0644b3123a1b Mon Sep 17 00:00:00 2001 From: Jon Ross-Perkins Date: Tue, 15 Nov 2022 22:36:52 -0800 Subject: [PATCH] Refactor Pattern and FunctionParameter handling for Parser consistency (#2385) While the Parser has similar divergent states, lists of Expressions tend to be handled more like this. I'm keeping the divergent start state in order to continue support of a distinct error, but I think this organization of FunctionParameter/FunctionParameterFinish will be less surprising. --- toolchain/parser/parser.cpp | 69 +++++++++++++++---------------- toolchain/parser/parser.h | 13 +++--- toolchain/parser/parser_state.def | 47 ++++++++++----------- 3 files changed, 60 insertions(+), 69 deletions(-) diff --git a/toolchain/parser/parser.cpp b/toolchain/parser/parser.cpp index 45a7e76a6901..ea0dc4492e2d 100644 --- a/toolchain/parser/parser.cpp +++ b/toolchain/parser/parser.cpp @@ -1004,6 +1004,27 @@ auto Parser::HandleFunctionIntroducerState() -> void { // Advance past the open paren. ++position_; if (!PositionIs(TokenKind::CloseParen())) { + PushState(ParserState::FunctionParameter()); + } +} + +auto Parser::HandleFunctionParameterState() -> void { + PopAndDiscardState(); + + PushState(ParserState::FunctionParameterFinish()); + PushState(ParserState::PatternAsFunctionParameter()); +} + +auto Parser::HandleFunctionParameterFinishState() -> void { + auto state = PopState(); + + if (state.has_error) { + ReturnErrorOnState(); + } + + if (ConsumeListToken(ParseNodeKind::ParameterListComma(), + TokenKind::CloseParen(), + state.has_error) == ListTokenKind::Comma) { PushState(ParserState::PatternAsFunctionParameter()); } } @@ -1270,20 +1291,11 @@ auto Parser::HandleParenExpressionFinishAsTupleState() -> void { state.has_error); } -auto Parser::HandlePatternStart(PatternKind pattern_kind) -> void { +auto Parser::HandlePattern(PatternKind pattern_kind) -> void { auto state = PopState(); // Ensure the finish state always follows. - switch (pattern_kind) { - case PatternKind::Parameter: { - state.state = ParserState::PatternAsFunctionParameterFinish(); - break; - } - case PatternKind::Variable: { - state.state = ParserState::PatternAsVariableFinish(); - break; - } - } + state.state = ParserState::PatternFinish(); // Handle an invalid pattern introducer. if (!PositionIs(TokenKind::Identifier()) || @@ -1316,41 +1328,26 @@ auto Parser::HandlePatternStart(PatternKind pattern_kind) -> void { position_ += 2; } -auto Parser::HandlePatternFinish() -> bool { +auto Parser::HandlePatternAsFunctionParameterState() -> void { + HandlePattern(PatternKind::Parameter); +} + +auto Parser::HandlePatternAsVariableState() -> void { + HandlePattern(PatternKind::Variable); +} + +auto Parser::HandlePatternFinishState() -> void { auto state = PopState(); // If an error was encountered, propagate it without adding a node. if (state.has_error) { ReturnErrorOnState(); - return true; + return; } // TODO: may need to mark has_error if !type. AddNode(ParseNodeKind::PatternBinding(), state.token, state.subtree_start, /*has_error=*/false); - return false; -} - -auto Parser::HandlePatternAsFunctionParameterState() -> void { - HandlePatternStart(PatternKind::Parameter); -} - -auto Parser::HandlePatternAsFunctionParameterFinishState() -> void { - bool has_error = HandlePatternFinish(); - - if (ConsumeListToken(ParseNodeKind::ParameterListComma(), - TokenKind::CloseParen(), - has_error) == ListTokenKind::Comma) { - PushState(ParserState::PatternAsFunctionParameter()); - } -} - -auto Parser::HandlePatternAsVariableState() -> void { - HandlePatternStart(PatternKind::Variable); -} - -auto Parser::HandlePatternAsVariableFinishState() -> void { - HandlePatternFinish(); } auto Parser::HandleStatementState() -> void { diff --git a/toolchain/parser/parser.h b/toolchain/parser/parser.h index fb5361f67ec6..1fd201606f0b 100644 --- a/toolchain/parser/parser.h +++ b/toolchain/parser/parser.h @@ -16,6 +16,8 @@ namespace Carbon { +// This parser uses a stack for state transitions. See parser_state.def for +// state documentation. class Parser { public: // Parses the tokens into a parse tree, emitting any errors encountered. @@ -265,16 +267,11 @@ class Parser { auto HandleFunctionError(StateStackEntry state, bool skip_past_likely_end) -> void; - // Handles ParenExpressionParameterFinish(AsUnknown|AsTuple) + // Handles ParenExpressionParameterFinish(AsUnknown|AsTuple). auto HandleParenExpressionParameterFinish(bool as_tuple) -> void; - // Handles the start of a pattern. - // If the start of the pattern is invalid, it's the responsibility of the - // outside context to advance past the pattern. - auto HandlePatternStart(PatternKind pattern_kind) -> void; - - // Handles the end of a pattern. - auto HandlePatternFinish() -> bool; + // Handles PatternAs(FunctionParameter|Variable). + auto HandlePattern(PatternKind pattern_kind) -> void; // Handles the `;` after a keyword statement. auto HandleStatementKeywordFinish(TokenKind token_kind, diff --git a/toolchain/parser/parser_state.def b/toolchain/parser/parser_state.def index 0cdbefc8e862..6b4be058b3f7 100644 --- a/toolchain/parser/parser_state.def +++ b/toolchain/parser/parser_state.def @@ -218,6 +218,22 @@ CARBON_PARSER_STATE(ExpressionStatementFinish) // 2. FunctionAfterParameterList CARBON_PARSER_STATE(FunctionIntroducer) +// Starts function parameter processing. +// +// Always: +// 1. PatternAsFunctionParameter +// 2. FunctionParameterFinish +CARBON_PARSER_STATE(FunctionParameter) + +// Finishes function parameter processing, including `,`. If there are more +// parameters, enqueues another parameter processing state. +// +// If `Comma` without `CloseParen`: +// 1. FunctionParameter +// Else: +// (state done) +CARBON_PARSER_STATE(FunctionParameterFinish) + // Handles processing of a function's parameter list `)`. // // Always: @@ -312,41 +328,22 @@ CARBON_PARSER_STATE(ParenExpressionParameterFinishAsTuple) CARBON_PARSER_STATE(ParenExpressionFinish) CARBON_PARSER_STATE(ParenExpressionFinishAsTuple) -// Handles pattern parsing for a function parameter, enqueuing type expression -// processing. Proceeds to the matching Finish state when done. +// Handles pattern parsing for a pattern, enqueuing type expression processing. +// This covers function parameter and `var` support. // // If valid: // 1. Expression -// 2. PatternAsFunctionParameterFinish +// 2. PatternFinish // Else: -// 1. PatternAsFunctionParameterFinish +// 1. PatternFinish CARBON_PARSER_STATE(PatternAsFunctionParameter) - -// Finishes function parameter processing, including `,`. If there are more -// parameters, enqueues another parameter processing state. -// -// If `Comma` without `CloseParen`: -// 1. PatternAsFunctionParameter -// Else: -// (state done) -CARBON_PARSER_STATE(PatternAsFunctionParameterFinish) - -// Handles pattern parsing for a `var` statement, enqueuing type expression -// processing. Proceeds to the matching Finish state when done. -// -// -// If valid: -// 1. Expression -// 2. PatternAsVariableFinish -// Else: -// 1. PatternAsVariableFinish CARBON_PARSER_STATE(PatternAsVariable) -// Finishes `var` pattern processing. +// Finishes pattern processing. // // Always: // (state done) -CARBON_PARSER_STATE(PatternAsVariableFinish) +CARBON_PARSER_STATE(PatternFinish) // Handles a single statement. While typically within a statement block, this // can also be used for error recovery where we expect a statement block and