Skip to content

Commit 8f1b7e5

Browse files
jonbaldieclaude
andcommitted
gh-158847: Avoid enumerating cycles when choosing pegen recursion leaders
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent 9112dae commit 8f1b7e5

4 files changed

Lines changed: 84 additions & 44 deletions

File tree

‎Lib/test/test_peg_generator/test_pegen.py‎

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,9 @@
1212
with test_tools.imports_under_tool("peg_generator"):
1313
from pegen.grammar_parser import GeneratedParser as GrammarParser
1414
from pegen.testutil import parse_string, generate_parser, make_parser
15-
from pegen.grammar import GrammarVisitor, GrammarError, Grammar, RuleKind
15+
from pegen.grammar import (
16+
GrammarVisitor, GrammarError, Grammar, NameLeaf, RuleKind,
17+
)
1618
from pegen.grammar_visualizer import ASTGrammarPrinter
1719
from pegen.parser import Parser
1820
from pegen.parser_generator import compute_nullables, compute_left_recursives
@@ -751,6 +753,56 @@ def test_opt_sequence(self) -> None:
751753
# of a line in the generated source. See bpo-41044
752754
make_parser(grammar)
753755

756+
def test_left_recursion_leader_order(self) -> None:
757+
grammar = parse_string("""
758+
start: zeta NEWLINE
759+
zeta: alpha '+' | NUMBER
760+
alpha: zeta '-' | NUMBER
761+
""", GrammarParser)
762+
PythonParserGenerator(grammar, io.StringIO())
763+
self.assertTrue(grammar.rules["alpha"].leader)
764+
self.assertFalse(grammar.rules["zeta"].leader)
765+
766+
def test_large_left_recursive_grammar(self) -> None:
767+
size = 12
768+
lines = ["start: r0 NEWLINE ENDMARKER"]
769+
for i in range(size):
770+
children = list(range(i + 1, size))
771+
if i:
772+
children.append(0)
773+
alternatives = [f"r{j} '+'" for j in children] + ["NUMBER"]
774+
lines.append(f"r{i}: " + " | ".join(alternatives))
775+
parser_class = make_parser("\n".join(lines) + "\n")
776+
node = parse_string("1\n", parser_class)
777+
self.assertEqual(node[0].string, "1")
778+
779+
def test_left_recursion_analysis_work(self) -> None:
780+
class CountedName(str):
781+
comparisons = 0
782+
__hash__ = str.__hash__
783+
784+
def __eq__(self, other):
785+
type(self).comparisons += 1
786+
return super().__eq__(other)
787+
788+
size = 8
789+
lines = ["start: r0 NEWLINE ENDMARKER"]
790+
for i in range(size):
791+
alternatives = [
792+
f"r{j} '+'" for j in range(size) if j != i
793+
] + ["NUMBER"]
794+
lines.append(f"r{i}: " + " | ".join(alternatives))
795+
grammar = parse_string("\n".join(lines) + "\n", GrammarParser)
796+
for rule in grammar.rules.values():
797+
for alt in rule.rhs.alts:
798+
for item in alt.items:
799+
if isinstance(item.item, NameLeaf):
800+
item.item.value = CountedName(item.item.value)
801+
CountedName.comparisons = 0
802+
with self.assertRaisesRegex(ValueError, "no leadership candidate"):
803+
PythonParserGenerator(grammar, io.StringIO())
804+
self.assertLess(CountedName.comparisons, 2_000)
805+
754806
def test_left_recursion_too_complex(self) -> None:
755807
grammar = """
756808
start: foo
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Pegen now picks the leader of a left-recursive rule cycle in polynomial time,
2+
instead of enumerating every path through the cycle.

‎Tools/peg_generator/pegen/grammar_analysis.py‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,19 +115,15 @@ def compute_left_recursives(
115115
if len(scc) > 1:
116116
for name in scc:
117117
rules[name].left_recursive = True
118-
# Try to find a leader such that all cycles go through it.
119-
leaders = set(scc)
120-
for start in scc:
121-
for cycle in sccutils.find_cycles_in_scc(graph, scc, start):
122-
# print("Cycle:", " -> ".join(cycle))
123-
leaders -= scc - set(cycle)
124-
if not leaders:
125-
raise ValueError(
126-
f"SCC {scc} has no leadership candidate (no element is included in all cycles)"
127-
)
128-
# print("Leaders:", leaders)
129-
leader = min(leaders) # Pick an arbitrary leader from the candidates.
130-
rules[leader].leader = True
118+
# A leader lies in every cycle, so removing it must leave a DAG.
119+
for leader in sorted(scc):
120+
if sccutils.is_acyclic(graph, scc - {leader}):
121+
rules[leader].leader = True
122+
break
123+
else:
124+
raise ValueError(
125+
f"SCC {scc} has no leadership candidate (no element is included in all cycles)"
126+
)
131127
else:
132128
name = min(scc) # The only element.
133129
if name in graph[name]:

‎Tools/peg_generator/pegen/sccutils.py‎

Lines changed: 20 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
# Adapted from mypy (mypy/build.py) under the MIT license.
22

3-
from collections.abc import Iterable, Iterator, Set
3+
from collections.abc import Iterator, Set
44

55

66
def strongly_connected_components(
@@ -49,32 +49,22 @@ def dfs(v: str) -> Iterator[set[str]]:
4949
yield from dfs(v)
5050

5151

52-
def find_cycles_in_scc(
53-
graph: dict[str, Set[str]], scc: Set[str], start: str
54-
) -> Iterable[list[str]]:
55-
"""Find cycles in SCC emanating from start.
56-
57-
Yields lists of the form ['A', 'B', 'C', 'A'], which means there's
58-
a path from A -> B -> C -> A. The first item is always the start
59-
argument, but the last item may be another element, e.g. ['A',
60-
'B', 'C', 'B'] means there's a path from A to B and there's a
61-
cycle from B to C and back.
62-
"""
63-
# Basic input checks.
64-
assert start in scc, (start, scc)
65-
assert scc <= graph.keys(), scc - graph.keys()
66-
67-
# Reduce the graph to nodes in the SCC.
68-
graph = {src: {dst for dst in dsts if dst in scc} for src, dsts in graph.items() if src in scc}
69-
assert start in graph
70-
71-
# Recursive helper that yields cycles.
72-
def dfs(node: str, path: list[str]) -> Iterator[list[str]]:
73-
if node in path:
74-
yield path + [node]
75-
return
76-
path = path + [node] # TODO: Make this not quadratic.
77-
for child in graph[node]:
78-
yield from dfs(child, path)
79-
80-
yield from dfs(start, [])
52+
def is_acyclic(graph: dict[str, Set[str]], vertices: Set[str]) -> bool:
53+
"""Check the subgraph induced by vertices using a topological sort."""
54+
indegree = dict.fromkeys(vertices, 0)
55+
for src in vertices:
56+
for dst in graph[src]:
57+
if dst in vertices:
58+
indegree[dst] += 1
59+
60+
ready = [node for node, degree in indegree.items() if degree == 0]
61+
processed = 0
62+
while ready:
63+
src = ready.pop()
64+
processed += 1
65+
for dst in graph[src]:
66+
if dst in vertices:
67+
indegree[dst] -= 1
68+
if indegree[dst] == 0:
69+
ready.append(dst)
70+
return processed == len(vertices)

0 commit comments

Comments
 (0)