Fix deeply nested quantifiers/alternations exceeding CPython CO_MAXBLOCKS - #396
piotr-oles wants to merge 1 commit into
Conversation
db6c650 to
b6fe6f4
Compare
|
Hey @renatahodovan , thanks for building this tool, I found it very useful for fuzz testing ANTLR parsers. Looking forward for your review 🙇🏻 |
renatahodovan
left a comment
There was a problem hiding this comment.
Thanks, the analysis is spot on. The approach (cutting deep sub-expressions into generated helper methods) is the right one. Before merging I'd like to shrink the implementation, see comments below.
b6fe6f4 to
7d8de64
Compare
|
Hey @renatahodovan ! I addressed your feedback, could you give it another look? |
akosthekiss
left a comment
There was a problem hiding this comment.
Let me chime in. Thanks for the update. I have some suggestions for simplifying the additions and to get them more aligned with existing code.
|
Hi @piotr-oles , I have pushed a commit into the PR, I hope you don't mind. This shows the idea behind by review better, hopefully. I have executed some tests locally, and this version generated the same code for DeepNesting.g4 as the original PR, at least. However, you surely have real use cases, so I'd like to ask you to check these changes on your side as well. Please, do edit the code further as necessary. (One more thing: once the PR gets close to completion, please squash all commits into one and rebase it on top of the master branch.) |
|
Hi @akosthekiss , thanks for review and changes! Sorry for the delay, I was on vacations :) I will test it against the real grammar we have, if everything works, I will rebase and squash, so we're ready for the merge. |
…OCKS CPython caps statically-nested blocks per code object at CO_MAXBLOCKS (20). Grammarinator renders each rule as one method, inlining quantifiers (3 blocks each) and alternations (1 block each) recursively, so deeply nested rules overflow the limit and make the generated module raise 'SyntaxError: too many statically nested blocks' at import.
2dfd08f to
5f9c517
Compare
|
I confirm this works after all the changes 👍 I squashed all commits and they're up to date with master. @renatahodovan , @akosthekiss could you take another look? 🙏🏻 |
|
Hey! Is there anything blocking from merging this? :) |
Summary
Grammarinator renders every grammar rule as a single Python method, inlining sub-structure recursively. Each quantifier opens 3 nested blocks (
with+while+with) and each alternation 1 (with). CPython caps statically-nested blocks per code object atCO_MAXBLOCKS(20), so a deeply nested rule overflows the limit and the generated module raisesSyntaxError: too many statically nested blocksat import time.This PR splits any over-budget rule (Python target only) into synthetic fragment helper methods: the deepest over-budget quantifier/alternation is cut out into its own
def(block depth 0) and a call is left behind. Extraction is safe becausecurrent == rule.currentat every sibling position.The work is in two commits:
modelmodule — pure refactor, no behaviour change. The node/graph representation moves out ofprocessor.pyintogrammarinator/tool/model.py;processor.pyandparser.pyimport from it.grammarinator/tool/splitter.py, theFragmentRuleNode/FragmentRefNodemodel nodes, template support, and tests.Before / After
Example grammar (
tests/grammars/DeepNesting.g4),deepquantrule — 11 nested optional quantifiers with a label (v=) deep inside, forcing alocal_ctx:Before — one method, nested to block depth 34 → fails at import
(
deepalt, with 23 nested alternations, reaches block depth 24 and fails the same way.)After — deepest over-budget node cut into a fragment; module imports cleanly
The
local_ctxdict is threaded into the fragment as a parameter, so labels/args/locals/returns still resolve after the cut.Notes
splitter._block_costmirrorsGeneratorTemplate.py.jinjaand must stay in sync with it; the test uses an independent AST-based oracle to guard against drift.@init,@after, etc.) block statements are not counted, so extremely deep action-heavy rules can still overflow.