Repository navigation
In parsermodule.c, replace over 2KLOC of hand-crafted validation code, with a DFA #70713
Description
| BPO | 26526 |
|---|---|
| Nosy | @freddrake, @brettcannon, @gpshead, @giampaolo, @benjaminp, @berkerpeksag, @serhiy-storchaka, @tyomitch |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/freddrake'
closed_at = <Date 2016-06-02.18:32:59.721>
created_at = <Date 2016-03-10.09:02:31.787>
labels = ['extension-modules', 'type-feature']
title = 'In parsermodule.c, replace over 2KLOC of hand-crafted validation code, with a DFA'
updated_at = <Date 2017-01-26.07:35:57.343>
user = 'https://github.com/tyomitch'bugs.python.org fields:
activity = <Date 2017-01-26.07:35:57.343>
actor = 'python-dev'
assignee = 'fdrake'
closed = True
closed_date = <Date 2016-06-02.18:32:59.721>
closer = 'python-dev'
components = ['Extension Modules']
creation = <Date 2016-03-10.09:02:31.787>
creator = 'A. Skrobov'
dependencies = []
files = ['42110', '43069']
hgrepos = []
issue_num = 26526
keywords = ['patch']
message_count = 11.0
messages = ['261486', '265411', '266441', '266577', '266741', '266903', '266904', '272318', '272754', '286261', '286295']
nosy_count = 10.0
nosy_names = ['fdrake', 'brett.cannon', 'gregory.p.smith', 'giampaolo.rodola', 'benjamin.peterson', 'python-dev', 'berker.peksag', 'serhiy.storchaka', 'xcombelle', 'A. Skrobov']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue26526'
versions = ['Python 3.6']
Activity
tyomitch commentedon Mar 10, 2016
Updating Modules/parsermodule.c for every change in grammar and/or parser is a maintenance burden, listed as such at https://docs.python.org/devguide/grammar.html
The attached patch lets the validation code use the auto-generated DFA structures, thus ensuring it stays up to date with the grammar. It also trims the code by over 2KLOC.
[-]In parsemodule.c, replace over 2KLOC of hand-crafted validation code, with a DFA[/-][+]In parsermodule.c, replace over 2KLOC of hand-crafted validation code, with a DFA[/+]tyomitch commentedon May 12, 2016
Ping? This patch is two months old now.
freddrake commentedon May 26, 2016
I see your message to python-dev, and apologize for taking so long to get to this.
I do intend to read through your changes, and hope to be able to make time while I'm at PyCon this coming week.
freddrake commentedon May 28, 2016
I've read through this, but haven't applied the patch & run tests (that's what buildbots are for).
No objections.
tyomitch commentedon May 31, 2016
Thank you Fred for your review!
I don't have commit access myself; can anybody please commit it for me?
brettcannon commentedon Jun 2, 2016
Are you up for trying to commit this while at the sprints, Fred? If not then assign it back to me and I can eventually commit it (busy w/ a ton of stuff so I can't promise when I will get to it).
python-dev commentedon Jun 2, 2016
New changeset 4a9159ea2536 by Benjamin Peterson in branch 'default':
replace custom validation logic in the parse module with a simple DFA validator (closes bpo-26526)
https://hg.python.org/cpython/rev/4a9159ea2536
xcombelle commentedon Aug 10, 2016
The DFA is generated by other part of existing cpython code ? If it's the case looks like you did a great job.
tyomitch commentedon Aug 15, 2016
Thanks Xavier! Yes, this is the same DFA that's used by the main Python parser. For some reason, parsermodule didn't previously reuse it, but instead did its own thing.
Any volunteers to review the other patch for Python parser, at http://bugs.python.org/issue26415 ?
tyomitch commentedon Jan 25, 2017
Oh btw, the comment in the beginning of Grammar/Grammar
is no longer true: after this patch went in, changing the grammar no longer requires translating the changes into Modules/parsermodule.c
Can somebody please remove the obsolete comment from there?
python-dev commentedon Jan 26, 2017
New changeset 3275d4b584a2 by Benjamin Peterson in branch '3.6':
remove comment about updating the parser module; we do not need to do that anymore (bpo-26526)
https://hg.python.org/cpython/rev/3275d4b584a2