From 82f7d068554917f8c886b72f08f24bdd875ec33e Mon Sep 17 00:00:00 2001
From: Jon Ross-Perkins <jperkins@google.com>
Date: Mon, 23 Jan 2023 13:28:34 -0800
Subject: [PATCH] Fix function parameter parsing past 2 params. (#2542)

The comment on FunctionParameterFinish is actually correct (`1. FunctionParameter`), just a typo in the implementation. It specifically failed parsing at the comma after the 2nd param, regardless of whether there were more params.
---
 toolchain/parser/parser.cpp                   |  2 +-
 .../function/declaration/params_one.carbon    | 19 +++++++++
 .../params_one_suffix_comma.carbon            | 20 ++++++++++
 .../function/declaration/params_six.carbon    | 39 +++++++++++++++++++
 .../{with_params.carbon => params_two.carbon} |  6 +--
 .../params_two_suffix_comma.carbon            | 24 ++++++++++++
 6 files changed, 106 insertions(+), 4 deletions(-)
 create mode 100644 toolchain/parser/testdata/function/declaration/params_one.carbon
 create mode 100644 toolchain/parser/testdata/function/declaration/params_one_suffix_comma.carbon
 create mode 100644 toolchain/parser/testdata/function/declaration/params_six.carbon
 rename toolchain/parser/testdata/function/declaration/{with_params.carbon => params_two.carbon} (86%)
 create mode 100644 toolchain/parser/testdata/function/declaration/params_two_suffix_comma.carbon

diff --git a/toolchain/parser/parser.cpp b/toolchain/parser/parser.cpp
index d6109af1618d..bf8037c69e02 100644
--- a/toolchain/parser/parser.cpp
+++ b/toolchain/parser/parser.cpp
@@ -1090,7 +1090,7 @@ auto Parser::HandleFunctionParameterFinishState() -> void {
 
   if (ConsumeListToken(ParseNodeKind::ParameterListComma, TokenKind::CloseParen,
                        state.has_error) == ListTokenKind::Comma) {
-    PushState(ParserState::PatternAsFunctionParameter);
+    PushState(ParserState::FunctionParameter);
   }
 }
 
diff --git a/toolchain/parser/testdata/function/declaration/params_one.carbon b/toolchain/parser/testdata/function/declaration/params_one.carbon
new file mode 100644
index 000000000000..21dd3b90d47b
--- /dev/null
+++ b/toolchain/parser/testdata/function/declaration/params_one.carbon
@@ -0,0 +1,19 @@
+// 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
+// RUN: %{carbon-run-parser}
+// CHECK:STDOUT: [
+// CHECK:STDOUT:   {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:   {kind: 'DeclaredName', text: 'foo'},
+// CHECK:STDOUT:     {kind: 'ParameterListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'a'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:   {kind: 'ParameterList', text: ')', subtree_size: 5},
+// CHECK:STDOUT: {kind: 'FunctionDeclaration', text: ';', subtree_size: 8},
+// CHECK:STDOUT: {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT: ]
+
+fn foo(a: i32);
diff --git a/toolchain/parser/testdata/function/declaration/params_one_suffix_comma.carbon b/toolchain/parser/testdata/function/declaration/params_one_suffix_comma.carbon
new file mode 100644
index 000000000000..9f61a2e41323
--- /dev/null
+++ b/toolchain/parser/testdata/function/declaration/params_one_suffix_comma.carbon
@@ -0,0 +1,20 @@
+// 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
+// RUN: %{carbon-run-parser}
+// CHECK:STDOUT: [
+// CHECK:STDOUT:   {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:   {kind: 'DeclaredName', text: 'foo'},
+// CHECK:STDOUT:     {kind: 'ParameterListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'a'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:   {kind: 'ParameterList', text: ')', subtree_size: 6},
+// CHECK:STDOUT: {kind: 'FunctionDeclaration', text: ';', subtree_size: 9},
+// CHECK:STDOUT: {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT: ]
+
+fn foo(a: i32,);
diff --git a/toolchain/parser/testdata/function/declaration/params_six.carbon b/toolchain/parser/testdata/function/declaration/params_six.carbon
new file mode 100644
index 000000000000..672bfa408417
--- /dev/null
+++ b/toolchain/parser/testdata/function/declaration/params_six.carbon
@@ -0,0 +1,39 @@
+// 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
+// RUN: %{carbon-run-parser}
+// CHECK:STDOUT: [
+// CHECK:STDOUT:   {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:   {kind: 'DeclaredName', text: 'foo'},
+// CHECK:STDOUT:     {kind: 'ParameterListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'a'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'b'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'c'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'd'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'e'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'f'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:   {kind: 'ParameterList', text: ')', subtree_size: 25},
+// CHECK:STDOUT: {kind: 'FunctionDeclaration', text: ';', subtree_size: 28},
+// CHECK:STDOUT: {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT: ]
+
+fn foo(a: i32, b: i32, c: i32, d: i32, e: i32, f: i32);
diff --git a/toolchain/parser/testdata/function/declaration/with_params.carbon b/toolchain/parser/testdata/function/declaration/params_two.carbon
similarity index 86%
rename from toolchain/parser/testdata/function/declaration/with_params.carbon
rename to toolchain/parser/testdata/function/declaration/params_two.carbon
index c1e962d2a650..7cc94041f075 100644
--- a/toolchain/parser/testdata/function/declaration/with_params.carbon
+++ b/toolchain/parser/testdata/function/declaration/params_two.carbon
@@ -8,11 +8,11 @@
 // CHECK:STDOUT:   {kind: 'FunctionIntroducer', text: 'fn'},
 // CHECK:STDOUT:   {kind: 'DeclaredName', text: 'foo'},
 // CHECK:STDOUT:     {kind: 'ParameterListStart', text: '('},
-// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'bar'},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'a'},
 // CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
 // CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
 // CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
-// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'baz'},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'b'},
 // CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
 // CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
 // CHECK:STDOUT:   {kind: 'ParameterList', text: ')', subtree_size: 9},
@@ -20,4 +20,4 @@
 // CHECK:STDOUT: {kind: 'FileEnd', text: ''},
 // CHECK:STDOUT: ]
 
-fn foo(bar: i32, baz: i32);
+fn foo(a: i32, b: i32);
diff --git a/toolchain/parser/testdata/function/declaration/params_two_suffix_comma.carbon b/toolchain/parser/testdata/function/declaration/params_two_suffix_comma.carbon
new file mode 100644
index 000000000000..900f90cc4857
--- /dev/null
+++ b/toolchain/parser/testdata/function/declaration/params_two_suffix_comma.carbon
@@ -0,0 +1,24 @@
+// 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
+// RUN: %{carbon-run-parser}
+// CHECK:STDOUT: [
+// CHECK:STDOUT:   {kind: 'FunctionIntroducer', text: 'fn'},
+// CHECK:STDOUT:   {kind: 'DeclaredName', text: 'foo'},
+// CHECK:STDOUT:     {kind: 'ParameterListStart', text: '('},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'a'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:       {kind: 'DeclaredName', text: 'b'},
+// CHECK:STDOUT:       {kind: 'Literal', text: 'i32'},
+// CHECK:STDOUT:     {kind: 'PatternBinding', text: ':', subtree_size: 3},
+// CHECK:STDOUT:     {kind: 'ParameterListComma', text: ','},
+// CHECK:STDOUT:   {kind: 'ParameterList', text: ')', subtree_size: 10},
+// CHECK:STDOUT: {kind: 'FunctionDeclaration', text: ';', subtree_size: 13},
+// CHECK:STDOUT: {kind: 'FileEnd', text: ''},
+// CHECK:STDOUT: ]
+
+fn foo(a: i32, b: i32,);