Skip to content

Bugfix: maybe_placeholders pads shorter alternatives of an optional (#1078) - #1629

Open
gaoflow wants to merge 1 commit into
lark-parser:masterfrom
gaoflow:fix-maybe-placeholders-alternation-arity
Open

Bugfix: maybe_placeholders pads shorter alternatives of an optional (#1078)#1629
gaoflow wants to merge 1 commit into
lark-parser:masterfrom
gaoflow:fix-maybe-placeholders-alternation-arity

Conversation

@gaoflow

@gaoflow gaoflow commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

maybe_placeholders promises an optional [...] group a fixed number of children so consumers can index positionally. When the group's alternatives differ in width, the arity varied per input:

!start: "a" ["b" | "c" "d"]
"a"   -> ['a', None, None]
"ab"  -> ['a', 'b']          # arity 2 -> children[2] raises IndexError
"acd" -> ['a', 'c', 'd']

maybe() padded only the absent case up to the widest branch; a matched narrower branch was left short. This is the case flagged # Not implemented; current behavior is incorrect in test_maybe_placeholders, and issue #1078 ("the argument count must remain the same whether or not there is a match").

The fix pads each shorter alternative with trailing placeholders during EBNF expansion, so every derivation of the optional has the same arity — "ab" now yields ['a', 'b', None]. It also handles nested and non-leading alternations ([("a" | "b" "c") "d"], ["a" | "b" "c"] "z"), and leaves maybe_placeholders=False untouched. Same result on earley and lalr.

Re-enables the maintainer's own commented expectation and adds the sibling cases as regression tests.

…al (Issue lark-parser#1078)

An optional group [...] must yield a fixed number of children so consumers can
index positionally. When its alternatives differed in width (e.g. ["b" | "c" "d"]),
a matched narrower branch was left unpadded while the absent case padded to the
widest branch, so arity varied per input. Pad each shorter alternative with
trailing placeholders during EBNF expansion, covering nested and non-leading
alternations too. Same result on earley and lalr.
@gaoflow
gaoflow force-pushed the fix-maybe-placeholders-alternation-arity branch from eebf816 to 2a6381b Compare July 29, 2026 02:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant