Skip to content

fix(parser): accept a trailing comma after a match arm's condition list - #2283

Open
phcorp wants to merge 3 commits into
illegalstudio:mainfrom
phcorp:fix/match-arm-trailing-comma
Open

phcorp wants to merge 3 commits into
illegalstudio:mainfrom
phcorp:fix/match-arm-trailing-comma

Conversation

@phcorp

@phcorp phcorp commented Nov 8, 2026 •

Copy link
Copy Markdown

Summary

PHP lets a match arm end its condition list with a trailing comma, and allows the same comma after default. The PHP grammar rules are match_arm_cond_list possible_comma T_DOUBLE_ARROW expr and T_DEFAULT possible_comma T_DOUBLE_ARROW expr. elephc rejected both forms:

  • 'a', 'b', => … failed with Expected '=>' in match arm
  • default, => … failed with Expected '=>' after 'default'

Both forms now parse and run as they do in PHP. A comma that has no condition before it (, =>), and a doubled comma (1,, =>, default,, =>), are still rejected, as PHP rejects them.

Found while compiling a Symfony/TUI application: janalis/viz uses this form.

Repro

<?php
function f($s) { return match ($s) {
    'task',
    'round',
        => 'rect',
    'c', => 'c',
    default => 'd',
}; }
echo f('round'), f('c'), f('z'), "\n";
echo match (1) { default, => 1 }, "\n";
output
main error: Expected '=>' in match arm (and Expected '=>' after 'default' for the second form)
this branch rectcd / 1
php 8.5 rectcd / 1

Inputs that are still rejected, on this branch and in PHP:

input php 8.5 this branch
match (1) { , => 1, default => 2 } unexpected token "," Unexpected token: Comma
match (1) { 1,, => 1, default => 2 } unexpected token "," Unexpected token: Comma
match (1) { 1, default => 1 } unexpected token "default" Unexpected token: Default
match (1) { default,, => 1 } syntax error Expected '=>' after 'default'

How it works

parse_match_expr:

  • Condition list. Before the fix, the parser met , followed by =>, then moved the cursor back to the comma before leaving the condition loop. The => check that comes next therefore saw the comma. The parser now consumes the comma and leaves the loop with the cursor on =>:

    if *pos < tokens.len() && tokens[*pos].0 == Token::Comma {
        *pos += 1;
        // A trailing comma closes the condition list: `'a', 'b', => …`.
        if *pos < tokens.len() && tokens[*pos].0 == Token::DoubleArrow {
            break;
        }
  • default. One optional comma is consumed before the existing => check. A second comma still fails that check.

Tests

  • test_parse_match_arm_condition_list_trailing_comma (parser): a multi-line list, a single condition and default each take a trailing comma. Every condition stays on its arm.
  • test_match_arm_condition_list_trailing_comma (codegen): all three forms compile and dispatch the right arm (rect|rect|circle|none). The output matches PHP under ELEPHC_PHP_CHECK=1.
  • test_error_match_arm_lone_comma, test_error_match_arm_double_trailing_comma, test_error_match_default_double_trailing_comma (errors): stray commas are still rejected.

Run against main's parser, the parser test and the codegen test fail with Expected '=>' in match arm. Runs on this branch:

  • cargo test --test parser_tests match_arm: 1 passed
  • cargo test --test error_tests test_error_match: 5 passed
  • ELEPHC_PHP_CHECK=1 cargo test --test codegen_tests test_match_arm_condition_list_trailing_comma: 1 passed

Note

CI status: this PR comes from a fork, so the CI, PDO Live Databases and PHP extension host SDK workflows are waiting for maintainer approval (action_required). The focused tests above ran locally on macOS arm64, and the codegen tests were also compared with PHP 8.5.12 (ELEPHC_PHP_CHECK=1). This is a front-end change (parser/checker only) with no target-specific code.

Limits

  • This fixes the AOT parser only. The eval() parser in crates/elephc-magician is separate and still rejects the form: eval('return match (2) { 1, 2, => "a", default => "b" };') fails with Parse error: eval() fragment is invalid.

Follow-ups

  • Bring the same trailing-comma rule to the magician eval() parser (match arms and default).
  • Merge order with fix(parser): accept a trailing comma in a closure's use() list #2284: both PRs edit src/parser/expr/prefix_complex.rs, in different functions (parse_match_expr and parse_closure), and both add tests to tests/error_tests/syntax.rs at different places. A scratch merge of the two branches is clean, and the combined parser and error tests pass. The PRs can merge in either order.

@github-actions github-actions Bot added area:parser Touches parsing or AST construction. size:xs Very small pull request. type:fix Corrects broken or incompatible behavior. labels Nov 8, 2026
@github-actions github-actions Bot added size:s Small pull request. and removed size:xs Very small pull request. labels Nov 8, 2026
@phcorp
phcorp marked this pull request as ready for review November 8, 2026 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:parser Touches parsing or AST construction. size:s Small pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant