Handle malformed subscript expressions (#3064)

Closes #3063

---------

Co-authored-by: Richard Smith <richard@metafoo.co.uk>

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)

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);
-
-    SkipTo(tokens().GetMatchedClosingToken(expected_paren));
+    // 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_open));
     AddNode(close_kind, Consume(), state.subtree_start, /*has_error=*/true);
   }
 }

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

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

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

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);
 }

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,];