From 049fbc1ee43fc2cf9bc57015ddfc759d1214b1f0 Mon Sep 17 00:00:00 2001 From: Geoff Romer Date: Mon, 7 Aug 2023 16:14:20 -0700 Subject: [PATCH] Handle malformed subscript expressions (#3064) Closes #3063 --------- Co-authored-by: Richard Smith --- toolchain/diagnostics/diagnostic_kind.def | 2 +- toolchain/parser/parser_context.cpp | 25 ++++++++++--------- toolchain/parser/parser_context.h | 11 +++++--- .../parser/parser_handle_index_expression.cpp | 6 ++--- .../parser/parser_handle_paren_condition.cpp | 8 ++++-- toolchain/parser/parser_handle_statement.cpp | 4 ++- .../testdata/index/fail_malformed_expr.carbon | 23 +++++++++++++++++ 7 files changed, 56 insertions(+), 23 deletions(-) create mode 100644 toolchain/parser/testdata/index/fail_malformed_expr.carbon diff --git a/toolchain/diagnostics/diagnostic_kind.def b/toolchain/diagnostics/diagnostic_kind.def index e48fe2d2910c..0333a64f0bf4 100644 --- a/toolchain/diagnostics/diagnostic_kind.def +++ b/toolchain/diagnostics/diagnostic_kind.def @@ -50,7 +50,7 @@ CARBON_DIAGNOSTIC_KIND(WrongRealLiteralExponent) // ============================================================================ CARBON_DIAGNOSTIC_KIND(BinaryOperatorRequiresWhitespace) -CARBON_DIAGNOSTIC_KIND(ExpectedCloseParen) +CARBON_DIAGNOSTIC_KIND(ExpectedCloseSymbol) CARBON_DIAGNOSTIC_KIND(ExpectedCodeBlock) CARBON_DIAGNOSTIC_KIND(ExpectedExpression) CARBON_DIAGNOSTIC_KIND(ExpectedIdentifierAfterDotOrArrow) diff --git a/toolchain/parser/parser_context.cpp b/toolchain/parser/parser_context.cpp index 2bf199442864..878df10bdae3 100644 --- a/toolchain/parser/parser_context.cpp +++ b/toolchain/parser/parser_context.cpp @@ -89,23 +89,24 @@ auto ParserContext::ConsumeAndAddOpenParen(TokenizedBuffer::Token default_token, } } -auto ParserContext::ConsumeAndAddCloseParen(StateStackEntry state, - ParseNodeKind close_kind) -> void { - // state.token should point at the introducer, with the paren one after the - // introducer. - auto expected_paren = *(TokenizedBuffer::TokenIterator(state.token) + 1); +auto ParserContext::ConsumeAndAddCloseSymbol( + TokenizedBuffer::Token expected_open, StateStackEntry state, + ParseNodeKind close_kind) -> void { + TokenKind open_token_kind = tokens().GetKind(expected_open); - if (tokens().GetKind(expected_paren) != TokenKind::OpenParen) { + if (!open_token_kind.is_opening_symbol()) { AddNode(close_kind, state.token, state.subtree_start, /*has_error=*/true); - } else if (auto close_token = ConsumeIf(TokenKind::CloseParen)) { + } else if (auto close_token = ConsumeIf(open_token_kind.closing_symbol())) { AddNode(close_kind, *close_token, state.subtree_start, state.has_error); } else { - // TODO: Include the location of the matching open_paren in the diagnostic. - CARBON_DIAGNOSTIC(ExpectedCloseParen, Error, - "Unexpected tokens before `)`."); - emitter_->Emit(*position_, ExpectedCloseParen); + // TODO: Include the location of the matching opening delimiter in the + // diagnostic. + CARBON_DIAGNOSTIC(ExpectedCloseSymbol, Error, + "Unexpected tokens before `{0}`.", llvm::StringRef); + emitter_->Emit(*position_, ExpectedCloseSymbol, + open_token_kind.closing_symbol().fixed_spelling()); - SkipTo(tokens().GetMatchedClosingToken(expected_paren)); + SkipTo(tokens().GetMatchedClosingToken(expected_open)); AddNode(close_kind, Consume(), state.subtree_start, /*has_error=*/true); } } diff --git a/toolchain/parser/parser_context.h b/toolchain/parser/parser_context.h index 20fe71a5a5ab..a75518707d95 100644 --- a/toolchain/parser/parser_context.h +++ b/toolchain/parser/parser_context.h @@ -112,10 +112,13 @@ class ParserContext { auto ConsumeAndAddOpenParen(TokenizedBuffer::Token default_token, ParseNodeKind start_kind) -> void; - // Parses a close paren token corresponding to the given open paren token, - // possibly skipping forward and diagnosing if necessary. Creates a parse node - // of the specified close kind. - auto ConsumeAndAddCloseParen(StateStackEntry state, ParseNodeKind close_kind) + // Parses a closing symbol corresponding to the opening symbol + // `expected_open`, possibly skipping forward and diagnosing if necessary. + // Creates a parse node of the specified close kind. If `expected_open` is not + // an opening symbol, the parse node will be associated with `state.token`, + // no input will be consumed, and no diagnostic will be emitted. + auto ConsumeAndAddCloseSymbol(TokenizedBuffer::Token expected_open, + StateStackEntry state, ParseNodeKind close_kind) -> void; // Composes `ConsumeIf` and `AddLeafNode`, returning false when ConsumeIf diff --git a/toolchain/parser/parser_handle_index_expression.cpp b/toolchain/parser/parser_handle_index_expression.cpp index 831ed14ab7cf..114663863553 100644 --- a/toolchain/parser/parser_handle_index_expression.cpp +++ b/toolchain/parser/parser_handle_index_expression.cpp @@ -19,9 +19,9 @@ auto ParserHandleIndexExpression(ParserContext& context) -> void { auto ParserHandleIndexExpressionFinish(ParserContext& context) -> void { auto state = context.PopState(); - context.AddNode(ParseNodeKind::IndexExpression, - context.ConsumeChecked(TokenKind::CloseSquareBracket), - state.subtree_start, state.has_error); + + context.ConsumeAndAddCloseSymbol(state.token, state, + ParseNodeKind::IndexExpression); } } // namespace Carbon diff --git a/toolchain/parser/parser_handle_paren_condition.cpp b/toolchain/parser/parser_handle_paren_condition.cpp index 1802174c858b..28942ff1344b 100644 --- a/toolchain/parser/parser_handle_paren_condition.cpp +++ b/toolchain/parser/parser_handle_paren_condition.cpp @@ -32,13 +32,17 @@ auto ParserHandleParenConditionAsWhile(ParserContext& context) -> void { auto ParserHandleParenConditionFinishAsIf(ParserContext& context) -> void { auto state = context.PopState(); - context.ConsumeAndAddCloseParen(state, ParseNodeKind::IfCondition); + context.ConsumeAndAddCloseSymbol( + *(TokenizedBuffer::TokenIterator(state.token) + 1), state, + ParseNodeKind::IfCondition); } auto ParserHandleParenConditionFinishAsWhile(ParserContext& context) -> void { auto state = context.PopState(); - context.ConsumeAndAddCloseParen(state, ParseNodeKind::WhileCondition); + context.ConsumeAndAddCloseSymbol( + *(TokenizedBuffer::TokenIterator(state.token) + 1), state, + ParseNodeKind::WhileCondition); } } // namespace Carbon diff --git a/toolchain/parser/parser_handle_statement.cpp b/toolchain/parser/parser_handle_statement.cpp index 60c1ee216db6..81a5423c5149 100644 --- a/toolchain/parser/parser_handle_statement.cpp +++ b/toolchain/parser/parser_handle_statement.cpp @@ -118,7 +118,9 @@ auto ParserHandleStatementForHeaderIn(ParserContext& context) -> void { auto ParserHandleStatementForHeaderFinish(ParserContext& context) -> void { auto state = context.PopState(); - context.ConsumeAndAddCloseParen(state, ParseNodeKind::ForHeader); + context.ConsumeAndAddCloseSymbol( + *(TokenizedBuffer::TokenIterator(state.token) + 1), state, + ParseNodeKind::ForHeader); context.PushState(ParserState::CodeBlock); } diff --git a/toolchain/parser/testdata/index/fail_malformed_expr.carbon b/toolchain/parser/testdata/index/fail_malformed_expr.carbon new file mode 100644 index 000000000000..c116fbcc0aa9 --- /dev/null +++ b/toolchain/parser/testdata/index/fail_malformed_expr.carbon @@ -0,0 +1,23 @@ +// Part of the Carbon Language project, under the Apache License v2.0 with LLVM +// Exceptions. See /LICENSE for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +// AUTOUPDATE +// CHECK:STDOUT: [ +// CHECK:STDOUT: {kind: 'VariableIntroducer', text: 'var'}, +// CHECK:STDOUT: {kind: 'Name', text: 'v'}, +// CHECK:STDOUT: {kind: 'Literal', text: 'i32'}, +// CHECK:STDOUT: {kind: 'PatternBinding', text: ':', subtree_size: 3}, +// CHECK:STDOUT: {kind: 'VariableInitializer', text: '='}, +// CHECK:STDOUT: {kind: 'NameExpression', text: 't'}, +// CHECK:STDOUT: {kind: 'IndexExpressionStart', text: '[', subtree_size: 2}, +// CHECK:STDOUT: {kind: 'Literal', text: '0'}, +// CHECK:STDOUT: {kind: 'IndexExpression', text: ']', has_error: yes, subtree_size: 4}, +// CHECK:STDOUT: {kind: 'VariableDeclaration', text: ';', subtree_size: 10}, +// CHECK:STDOUT: {kind: 'FileEnd', text: ''}, +// CHECK:STDOUT: ] + +// CHECK:STDERR: fail_malformed_expr.carbon:[[@LINE+3]]:17: Unexpected tokens before `]`. +// CHECK:STDERR: var v: i32 = t[0,]; +// CHECK:STDERR: ^ +var v: i32 = t[0,];