Skip to content

gh-153437: Make pegen output error detection checks after loop body for '*' rules.#153441

Closed
stestagg wants to merge 1 commit into
python:mainfrom
stestagg:gh-153437-2
Closed

gh-153437: Make pegen output error detection checks after loop body for '*' rules.#153441
stestagg wants to merge 1 commit into
python:mainfrom
stestagg:gh-153437-2

Conversation

@stestagg

@stestagg stestagg commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Currently, the pegen output produces this for '+' rules after the children have been parsed:

    if (_n == 0 || p->error_indicator) {
        PyMem_Free(_children);
        p->level--;
        return NULL;
    }

But for '*' rules, no check is done at all, (skipping the _n == 0 makes sesnse of course)

For '*' it's possible for an error to be raised when parsing children, and currently the parser sets the error flag/exception but does not return NULL, so parent rules might not detect the error appropriately, as in this code:

    if (
        (a = _PyPegen_expect_token(p, FSTRING_START))  // token='FSTRING_START'
        &&
        (b = _loop0_79_rule(p))  // fstring_middle*
        &&
        (c = _PyPegen_expect_token(p, FSTRING_END))  // token='FSTRING_END'
    )

In this example, _PyPegen_expect_token can raise a SyntaxWarning which trips a debug assert.

This change outputs the same p->error_indicator check for '*' rules as for '+' rules.

Note, there are lots of rules affected by this change, but it seems to me that this is a latent bug for each of them, even though it might not be possible to trigger an error for all of them.

Currently, the pegen output produces this for '+' rules:

    if (_n == 0 || p->error_indicator) {
        PyMem_Free(_children);
        p->level--;
        return NULL;
    }

But for '*' rules, no check is done at all, (skipping the _n == 0 makes sesnse of course)

For '*' it's possible for an error to be raised when parsing children, and currently the parser
sets the error flag/exception but does not return NULL, so parent rules might not detect the
error appropriately, as in this code:

        if (
            (a = _PyPegen_expect_token(p, FSTRING_START))  // token='FSTRING_START'
            &&
            (b = _loop0_79_rule(p))  // fstring_middle*
            &&
            (c = _PyPegen_expect_token(p, FSTRING_END))  // token='FSTRING_END'
        )

In this example, _PyPegen_expect_token can raise a SyntaxWarning which trips a debug assert.

This change outputs the same p->error_indicator check for '*' rules as for '+' rules.
@stestagg

stestagg commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Duplicate of #150067

@stestagg stestagg closed this Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant