Skip to content

bpo-42218: Check for the error indicator when expecting a new token #23058

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
isidentical wants to merge 1 commit into from
Closed

bpo-42218: Check for the error indicator when expecting a new token #23058

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Lib/test/test_syntax.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,9 @@ def func2():
"""
self._check_error(code, "invalid syntax")

def test_unexpected_line_continuation(self):
self._check_error('A.\u018a\\ ', "unexpected character after line continuation character")

@serhiy-storchaka serhiy-storchaka Oct 31, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would test two cases: \\\n and \\ followed by non-\n character. They are two slightly different errors.

Instead \u018a you can use any non-ascii character.


def test_main():
support.run_unittest(SyntaxTestCase)
from test import test_syntax
Expand Down
2 changes: 1 addition & 1 deletion Parser/pegen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -835,7 +835,7 @@ expr_ty
_PyPegen_name_token(Parser *p)
{
Token *t = _PyPegen_expect_token(p, NAME);
if (t == NULL) {
if (t == NULL || p->error_indicator) {

@serhiy-storchaka serhiy-storchaka Oct 31, 2020

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can _PyPegen_expect_token() return non-NULL if error is set?

Either _PyPegen_name_token() is called when error is set (it should not, it should be checked earlier), or the problem is in _PyPegen_fill_token().

Add assert(!PyErr_Occurred()) at the top of _PyPegen_expect_token() and if it does not crash, search bug in _PyPegen_fill_token().

@isidentical isidentical Oct 31, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How can _PyPegen_expect_token() return non-NULL if error is set?

As far as I understand, _PyPegen_expect_token returns NULL on 2 different cases. If there is an error, it returns NULL and if the given type is not matched (e.g: NAME given but NUMBER is the next token) it also returns NULL. But on the latter, it returns NULL without setting an error. So when parser is trying alternatives and backtracking, it assumes it got a different type of token and continues.

One thing that I thought was adding

    if (p->error_indicator) {
        return NULL;
    }

at the begging of _PyPegen_expect_token but I failed to find another case where the error didn't propagate except the one that author suggested so this might be enough.

return NULL;
}
char* s = PyBytes_AsString(t->bytes);
Expand Down