Skip to content

Commit 695d71e

Browse files
Fix two parser generator bugs exposed by inlined error alternatives
Two groups with the same syntax but different actions or variable names were deduplicated into a single artificial rule, because the cache key was based on str(), which ignores actions and variable names. With the error alternatives inlined before the base alternatives, the artificial rule for the group in an error alternative (without an action) could shadow the artificial rule for the same-looking group in a base alternative (with an action), e.g. "class C(B): ..." lost its bases. Use repr() in the cache key. The action of an alternative inserted by a rule extension can return NULL without setting an error (a conditional action which only sometimes reports the error, e.g. in invalid_legacy_expression). The generated code treated this as a result of the whole rule, so the rule failed (and the failure was memoized) instead of trying the remaining alternatives. Generate a NULL check which continues with the next alternative, like a reference to an invalid_* rule returning NULL behaves. Also do not insert new alternatives before alternatives inserted by preceding extensions: they raise an error rather than succeed, so they cannot shadow the new alternative, and the order of extensions should define their relative order ("'in' expected after for-loop variables" must be reported before descending into the for targets). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0ef9162 commit 695d71e

6 files changed

Lines changed: 2387 additions & 1773 deletions

File tree

Grammar/python_errors.gram

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -429,14 +429,14 @@ strings (extend):
429429
dict (extend):
430430
| '{' invalid_double_starred_kvpairs '}'
431431

432-
(for_stmt | for_if_clause) (extend):
433-
| 'async'? 'for' a=star_expressions {
434-
RAISE_SYNTAX_ERROR_INVALID_TARGET(FOR_TARGETS, a) }
435-
436432
for_if_clause (extend):
437433
| 'async'? 'for' (bitwise_or (',' bitwise_or)* [',']) !'in' {
438434
RAISE_SYNTAX_ERROR("'in' expected after for-loop variables") }
439435

436+
(for_stmt | for_if_clause) (extend):
437+
| 'async'? 'for' a=star_expressions {
438+
RAISE_SYNTAX_ERROR_INVALID_TARGET(FOR_TARGETS, a) }
439+
440440
(listcomp | setcomp | genexp) (extend):
441441
| '[' a='**' b=expression for_if_clauses {
442442
RAISE_SYNTAX_ERROR_KNOWN_RANGE(a, b, "cannot use dict unpacking in list comprehension") }

Lib/test/test_peg_generator/test_pegen.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,18 @@ def test_merge_grammars_repeated_meta(self) -> None:
9898
with self.assertRaisesRegex(GrammarError, "Repeated meta 'header'"):
9999
merge_grammars([first_grammar, second_grammar])
100100

101+
def test_same_group_with_different_actions(self) -> None:
102+
# Two groups with the same syntax but different actions must
103+
# not share an artificial rule.
104+
grammar_source = """
105+
start: a=rule_a b=rule_b NEWLINE { (a, b) }
106+
rule_a: ('a' 'b' { 1 })
107+
rule_b: ('a' 'b' { 2 })
108+
"""
109+
parser_class = make_parser(grammar_source)
110+
node = parse_string("a b a b\n", parser_class)
111+
self.assertEqual(node[:2], (1, 2))
112+
101113
def test_extend_rule_append_and_prepend(self) -> None:
102114
base_grammar_source = """
103115
start: rule_a NEWLINE

0 commit comments

Comments
 (0)