Skip to content
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.

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

Handle invalid expressions #54

Merged
merged 1 commit into from
Mar 27, 2023
Merged

Handle invalid expressions #54

merged 1 commit into from
Mar 27, 2023

Conversation

sunmy2019
Copy link

@sunmy2019 sunmy2019 commented Mar 25, 2023

A new test case failure can be added.

f"{6 0}"

The old parser will try to send "6 0" into the parser for f-string, thus will raise an SyntaxError like

    (6 0)
     ^^^
SyntaxError: f-string: invalid syntax. Perhaps you forgot a comma?

The new parser will (force) except } right after 6. Thus the error message would be

    f"{6 0}"
         ^
SyntaxError: expected '}'

The new parser cannot match the invalid_* at the first round, but p->error_indicator was set to 1, thus it cannot perform the second round.

https://github.com/python/cpython/blob/0708437ad043657f992cb985fd5c37e1ac052f93/Parser/pegen.c#L835-L840

So I made some changes to the grammar file to avoid setting p->error_indicator in that case.


As for the original test case,

Even with this change, the new parser will output

    f"{6 0="
           ^
SyntaxError: f-string: expecting '}'

rather than the old parser's

    (6 0)
     ^^^
SyntaxError: f-string: invalid syntax. Perhaps you forgot a comma?

After some digging, this error was improperly hijacked by the tokenizer:
https://github.com/python/cpython/blob/0708437ad043657f992cb985fd5c37e1ac052f93/Parser/pegen_errors.c#L406-L416

I think we should just leave it here, it is acceptable.

@sunmy2019 sunmy2019 changed the base branch from main to fstring-grammar-rebased-after-sprint March 25, 2023 18:20
@pablogsal
Copy link
Owner

This PR needs now rebase

@pablogsal pablogsal merged commit 30ea09f into pablogsal:fstring-grammar-rebased-after-sprint Mar 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants