Skip to content

Refactor error handling code in Parser/pegen/pegen.c#20440

Merged
miss-islington merged 3 commits into
python:masterfrom
lysnikolaou:refactor-pegen
May 27, 2020
Merged

Refactor error handling code in Parser/pegen/pegen.c#20440
miss-islington merged 3 commits into
python:masterfrom
lysnikolaou:refactor-pegen

Conversation

@lysnikolaou

@lysnikolaou lysnikolaou commented May 26, 2020 •
edited by miss-islington
Loading

Copy link
Copy Markdown
Member

Set p->error_indicator in various places, where it's needed, but it's
not done.

Automerge-Triggered-By: @gvanrossum

@lysnikolaou
Refactor error handling code in Parser/pegen/pegen.c
78ddc5c 
Set p->error_indicator in various places, where it's needed, but it's
not done. Also refactor `_PyPegen_expect_soft_keyword` to avoid a
double call of `PyBytes_AsString` and a double check for the `NAME` type.
@lysnikolaou lysnikolaou requested a review from pablogsal as a code owner May 26, 2020 23:57
@lysnikolaou lysnikolaou requested a review from gvanrossum May 26, 2020 23:57
gvanrossum
gvanrossum reviewed May 27, 2020
View reviewed changes
Comment thread Parser/pegen/pegen.c Outdated
gvanrossum
gvanrossum reviewed May 27, 2020
View reviewed changes
Comment thread Parser/pegen/pegen.c
@lysnikolaou lysnikolaou requested a review from gvanrossum May 27, 2020 15:37
@gvanrossum

gvanrossum commented May 27, 2020

Copy link
Copy Markdown
Member

This is going to be tricky to backport to 3.9 unless we backport soft keywords too (which I am fine with TBH). Thoughts?

@miss-islington

miss-islington commented May 27, 2020

Copy link
Copy Markdown
Contributor

@lysnikolaou: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 526e23f into python:master May 27, 2020
@lysnikolaou lysnikolaou deleted the refactor-pegen branch May 27, 2020 16:14
@lysnikolaou

lysnikolaou commented May 27, 2020 •
edited
Loading

Copy link
Copy Markdown
Member Author

This is going to be tricky to backport to 3.9 unless we backport soft keywords too (which I am fine with TBH). Thoughts?

Certainly fine by me as well. Shouldn't we ask Łukasz first though, since soft keyword support is really on the edge of being a new feature for the parser?

@gvanrossum

gvanrossum commented May 27, 2020

Copy link
Copy Markdown
Member

It's a new feature for pegen, not for CPython. So I think it's fine as long as we don't start using soft keywords. (Adding aprint statement was fun but broke too many tests to be a clean easter egg.)

Note that there are two PRs you need to backport -- my original PR, and Pablo's PR to fix lookaheads.

@lysnikolaou

lysnikolaou commented May 27, 2020

Copy link
Copy Markdown
Member Author

Ok, I'm on it.

lysnikolaou added a commit to lysnikolaou/cpython that referenced this pull request May 27, 2020
@lysnikolaou
[3.9] Backport pythonGH-20440: Set p->error_indicator in more places
b6c477b
pablogsal pushed a commit that referenced this pull request May 27, 2020
@lysnikolaou
[3.9] Backport GH-20440: Set p->error_indicator in more places (GH-20457
c011d1b 
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

@pablogsal pablogsal Awaiting requested review from pablogsal

Assignees

No one assigned

Labels

skip issue skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@lysnikolaou @gvanrossum @miss-islington @the-knights-who-say-ni @bedevere-bot