allow trailing commas in a struct pattern after an _ field (#8074)

Following a leads decision
(https://github.com/carbon-language/carbon-lang/issues/7404), this PR
adds support for trailing commas in a struct pattern where it was
previously only allowed after a full field.

docs/design/pattern_matching.md

@@ -414,8 +414,9 @@ matches if all of these sub-matches succeed.
 
 A struct can be matched with a struct pattern.
 
--   _struct-pattern_ ::= `{` [_field-pattern_ [`,` _field-pattern_ ]\* ] `}`
--   _struct-pattern_ ::= `{` [_field-pattern_ `,`]+ `_` `}`
+-   _struct-pattern_ ::= `{` [_field-pattern_ [`,` _field-pattern_ ]\* `,`? ]
+    `}`
+-   _struct-pattern_ ::= `{` [_field-pattern_ `,`]+ `_` `,`? `}`
 -   _field-pattern_ ::= _designator_ `=` _pattern_
 -   _field-pattern_ ::= _binding-pattern_
 -   _pattern_ ::= _struct-pattern_

toolchain/check/handle_pattern_list.cpp

@@ -167,10 +167,6 @@ auto HandleParseNode(Context& context, Parse::StructPatternId node_id) -> bool {
     return false;
   }
 
-  if (context.node_stack().PeekIs(Parse::NodeKind::UnderscoreName)) {
-    return context.TODO(node_id, "Struct pattern underscore field");
-  }
-
   if (!context.node_stack().PeekIs(Parse::NodeCategory::MemberName)) {
     // Remove the last parameter from the node stack before collecting names.
     context.param_and_arg_refs_stack().EndNoPop(
@@ -241,6 +237,11 @@ auto HandleParseNode(Context& context,
   return true;
 }
 
+auto HandleParseNode(Context& context,
+                     Parse::PatternListUnderscoreFieldId node_id) -> bool {
+  return context.TODO(node_id, "Struct pattern underscore field");
+}
+
 auto HandleParseNode(Context& context, Parse::PatternListCommaId /*node_id*/)
     -> bool {
   EndExprRegionForPattern(context, context.node_stack());

toolchain/check/node_stack.h

@@ -483,6 +483,7 @@ class NodeStack {
       case Parse::NodeKind::TemplateBindingName:
       case Parse::NodeKind::TupleLiteralStart:
       case Parse::NodeKind::TuplePatternStart:
+      case Parse::NodeKind::PatternListUnderscoreField:
       case Parse::NodeKind::VariableInitializer:
       case Parse::NodeKind::VariableIntroducer:
       case Parse::NodeKind::InlineImportBody:

toolchain/check/testdata/patterns/struct.carbon

@@ -135,9 +135,10 @@ let {a: i32, .b = c: bool} = {.b = true, .a = 1};
 library "[[@TEST_NAME]]";
 
 //@dump-sem-ir-begin
-// CHECK:STDERR: fail_todo_underscore.carbon:[[@LINE+2]]:5-20: error: semantics TODO: `Struct pattern underscore field` [SemanticsTodo]
+// CHECK:STDERR: fail_todo_underscore.carbon:[[@LINE+2]]:19: error: semantics TODO: `Struct pattern underscore field` [SemanticsTodo]
 // CHECK:STDERR:
 let {.a = x: i32, _} = {.a = 1, .b = ()};
+let {.a = y: i32, _,} = {.a = 1, .b = ()};
 //@dump-sem-ir-end
 
 // --- fail_no_matching_field.carbon

toolchain/parse/handle_pattern_list.cpp

@@ -2,6 +2,7 @@
 // Exceptions. See /LICENSE for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 
+#include "toolchain/diagnostics/format_providers.h"
 #include "toolchain/parse/context.h"
 #include "toolchain/parse/handle.h"
 
@@ -143,19 +144,37 @@ auto HandleStructPatternUnderscore(Context& context) -> void {
 
   auto underscore = context.ConsumeChecked(Lex::TokenKind::Underscore);
 
-  bool is_last = context.PositionIs(Lex::TokenKind::CloseCurlyBrace);
+  if (!context.PositionIs(Lex::TokenKind::CloseCurlyBrace)) {
+    bool has_error = true;
+    bool has_comma = context.PositionIs(Lex::TokenKind::Comma);
 
-  if (!is_last) {
-    CARBON_DIAGNOSTIC(
-        ExpectedCloseAfterUnderscore, Error,
-        "unexpected token `{0}` after `_` in struct pattern, expected `}`",
-        Lex::TokenKind);
-    context.emitter().Emit(*context.position(), ExpectedCloseAfterUnderscore,
-                           context.PositionKind());
-    state.has_error = true;
+    auto token = context.position();
+    auto token_kind = context.PositionKind();
+
+    if (has_comma) {
+      if (context.PositionIs(Lex::TokenKind::CloseCurlyBrace,
+                             Lookahead::NextToken)) {
+        // We have a trailing `,` followed by `}`, so there is no error
+        has_error = false;
+      } else {
+        token++;
+        token_kind = context.PositionKind(Lookahead::NextToken);
+      }
+    }
+
+    if (has_error) {
+      CARBON_DIAGNOSTIC(ExpectedCloseAfterUnderscore, Error,
+                        "unexpected token `{0}` after {1:`,`|`_`} in struct "
+                        "pattern{1: with `_` field|}, expected `}`",
+                        Lex::TokenKind, Diagnostics::BoolAsSelect);
+      context.emitter().Emit(*token, ExpectedCloseAfterUnderscore, token_kind,
+                             has_comma);
+      state.has_error = true;
+    }
   }
 
-  context.AddNode(NodeKind::UnderscoreName, underscore, state.has_error);
+  context.AddNode(NodeKind::PatternListUnderscoreField, underscore,
+                  state.has_error);
 
   if (state.has_error) {
     context.ReturnErrorOnState();

toolchain/parse/node_kind.def

@@ -150,6 +150,8 @@ CARBON_PARSE_NODE_KIND_DECLARATION(SelfTypeNameExpr)
 
 CARBON_PARSE_NODE_KIND_DECLARATION(UnderscoreName)
 
+CARBON_PARSE_NODE_KIND_DECLARATION(PatternListUnderscoreField)
+
 CARBON_PARSE_NODE_KIND_DECLARATION(BaseName)
 CARBON_PARSE_NODE_KIND_DECLARATION(PackageExpr)
 CARBON_PARSE_NODE_KIND_DECLARATION(CoreNameExpr)

toolchain/parse/testdata/struct/struct_pattern.carbon

@@ -32,6 +32,10 @@ let {.x = 1, .y = 2};
 
 let {.x = y: i32, _} = {};
 
+// --- struct_pattern_discard_with_comma.carbon
+
+let {.x = y: i32, _, } = {};
+
 // --- struct_pattern_unnamed.carbon
 
 let {.x = _: i32} = {};
@@ -136,13 +140,13 @@ let {.x x:i32} = {};
 
 // --- fail_struct_pattern_token_after_underscore.carbon
 
-// CHECK:STDERR: fail_struct_pattern_token_after_underscore.carbon:[[@LINE+2]]:20: error: unexpected token `,` after `_` in struct pattern, expected `}` [ExpectedCloseAfterUnderscore]
+// CHECK:STDERR: fail_struct_pattern_token_after_underscore.carbon:[[@LINE+2]]:20: error: unexpected token `.` after `_` in struct pattern, expected `}` [ExpectedCloseAfterUnderscore]
 // CHECK:STDERR:
-let {.x = x: i32, _,} = {};
+let {.x = x: i32, _. } = {};
 
 // --- fail_struct_pattern_underscore_not_last.carbon
 
-// CHECK:STDERR: fail_struct_pattern_underscore_not_last.carbon:[[@LINE+2]]:6: error: unexpected token `,` after `_` in struct pattern, expected `}` [ExpectedCloseAfterUnderscore]
+// CHECK:STDERR: fail_struct_pattern_underscore_not_last.carbon:[[@LINE+2]]:8: error: unexpected token `.` after `,` in struct pattern with `_` field, expected `}` [ExpectedCloseAfterUnderscore]
 // CHECK:STDERR:
 let{_, .x = x: i32} = {};
 
@@ -337,7 +341,28 @@ let {var _: i32} = {};
 // CHECK:STDOUT:   │ │ │ ├─LetBindingPattern ':'
 // CHECK:STDOUT:   │ │ ├─StructPatternDesignatedField '='
 // CHECK:STDOUT:   │ │ ├─PatternListComma ','
-// CHECK:STDOUT:   │ │ ├─UnderscoreName '_'
+// CHECK:STDOUT:   │ │ ├─PatternListUnderscoreField '_'
+// CHECK:STDOUT:   │ ├─StructPattern '}'
+// CHECK:STDOUT:   │ ├─LetInitializer '='
+// CHECK:STDOUT:   │ │ ╭─StructLiteralStart '{'
+// CHECK:STDOUT:   │ ├─StructLiteral '}'
+// CHECK:STDOUT:   ├─LetDecl ';'
+// CHECK:STDOUT:   ├─FileEnd ''
+// CHECK:STDOUT:   (root)
+// CHECK:STDOUT: - filename: struct_pattern_discard_with_comma.carbon
+// CHECK:STDOUT:   ╭─FileStart ''
+// CHECK:STDOUT:   │ ╭─LetIntroducer 'let'
+// CHECK:STDOUT:   │ │ ╭─StructPatternStart '{'
+// CHECK:STDOUT:   │ │ │   ╭─IdentifierNameNotBeforeSignature 'x'
+// CHECK:STDOUT:   │ │ │ ╭─StructFieldDesignator '.'
+// CHECK:STDOUT:   │ │ │ │ ╭─IdentifierNameNotBeforeSignature 'y'
+// CHECK:STDOUT:   │ │ │ │ ├─BindingPatternTypeStart ':'
+// CHECK:STDOUT:   │ │ │ │ ├─IntTypeLiteral 'i32'
+// CHECK:STDOUT:   │ │ │ ├─LetBindingPattern ':'
+// CHECK:STDOUT:   │ │ ├─StructPatternDesignatedField '='
+// CHECK:STDOUT:   │ │ ├─PatternListComma ','
+// CHECK:STDOUT:   │ │ ├─PatternListUnderscoreField '_'
+// CHECK:STDOUT:   │ │ ├─PatternListComma ','
 // CHECK:STDOUT:   │ ├─StructPattern '}'
 // CHECK:STDOUT:   │ ├─LetInitializer '='
 // CHECK:STDOUT:   │ │ ╭─StructLiteralStart '{'
@@ -715,8 +740,7 @@ let {var _: i32} = {};
 // CHECK:STDOUT:   │ │ │ ├─LetBindingPattern ':'
 // CHECK:STDOUT:   │ │ ├─StructPatternDesignatedField '='
 // CHECK:STDOUT:   │ │ ├─PatternListComma ','
-// CHECK:STDOUT:   │ │ ├─UnderscoreName '_' has_error
-// CHECK:STDOUT:   │ │ ├─PatternListComma ','
+// CHECK:STDOUT:   │ │ ├─PatternListUnderscoreField '_' has_error
 // CHECK:STDOUT:   │ ├─StructPattern '}' has_error
 // CHECK:STDOUT:   │ ├─LetInitializer '='
 // CHECK:STDOUT:   │ │ ╭─StructLiteralStart '{'
@@ -728,7 +752,7 @@ let {var _: i32} = {};
 // CHECK:STDOUT:   ╭─FileStart ''
 // CHECK:STDOUT:   │ ╭─LetIntroducer 'let'
 // CHECK:STDOUT:   │ │ ╭─StructPatternStart '{'
-// CHECK:STDOUT:   │ │ ├─UnderscoreName '_' has_error
+// CHECK:STDOUT:   │ │ ├─PatternListUnderscoreField '_' has_error
 // CHECK:STDOUT:   │ │ ├─PatternListComma ','
 // CHECK:STDOUT:   │ │ │   ╭─IdentifierNameNotBeforeSignature 'x'
 // CHECK:STDOUT:   │ │ │ ╭─StructFieldDesignator '.'

toolchain/parse/typed_nodes.h

@@ -169,6 +169,10 @@ using UnderscoreName =
     LeafNode<NodeKind::UnderscoreName, Lex::UnderscoreTokenIndex,
              NodeCategory::NonExprName>;
 
+// The `_` token, when used as the field in a struct pattern.
+using PatternListUnderscoreField =
+    LeafNode<NodeKind::PatternListUnderscoreField, Lex::UnderscoreTokenIndex>;
+
 // A name qualifier with parameters, such as `A(T: type).` or
 // `A[T: type](N: T).`.
 struct IdentifierNameQualifierWithParams {
@@ -1638,7 +1642,7 @@ using StructPatternFieldId =
     NodeIdOneOf<StructPatternDesignatedField,
                 StructPatternFieldLetBindingPattern, VariablePattern,
                 StructPatternFieldVarBindingPattern, UnusedPattern,
-                UnderscoreName, DefaultValuePattern>;
+                PatternListUnderscoreField, DefaultValuePattern>;
 
 struct StructPattern {
   static constexpr auto Kind = NodeKind::StructPattern.Define(