classification
Title: In parsermodule.c, replace over 2KLOC of hand-crafted validation code, with a DFA
Type: enhancement Stage: resolved
Components: Extension Modules Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: fdrake Nosy List: A. Skrobov, benjamin.peterson, berker.peksag, brett.cannon, fdrake, giampaolo.rodola, gregory.p.smith, python-dev, serhiy.storchaka, xcombelle
Priority: normal Keywords: patch

Created on 2016-03-10 09:02 by A. Skrobov, last changed 2017-01-26 07:35 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
patch A. Skrobov, 2016-03-10 09:02 review
issue26526_16704_63395.diff A. Skrobov, 2016-05-31 15:03 Fixed a stray tab and expanded a comment
Messages (11)
msg261486 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2016-03-10 09:02
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.
msg265411 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2016-05-12 15:22
Ping? This patch is two months old now.
msg266441 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2016-05-26 15:37
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.
msg266577 - (view) Author: Fred Drake (fdrake) (Python committer) Date: 2016-05-28 22:19
I've read through this, but haven't applied the patch & run tests (that's what buildbots are for).

No objections.
msg266741 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2016-05-31 07:24
Thank you Fred for your review!

I don't have commit access myself; can anybody please commit it for me?
msg266903 - (view) Author: Brett Cannon (brett.cannon) * (Python committer) Date: 2016-06-02 18:28
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).
msg266904 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-02 18:32
New changeset 4a9159ea2536 by Benjamin Peterson in branch 'default':
replace custom validation logic in the parse module with a simple DFA validator (closes #26526)
https://hg.python.org/cpython/rev/4a9159ea2536
msg272318 - (view) Author: Xavier Combelle (xcombelle) * Date: 2016-08-10 07:02
The DFA is generated by other part of existing cpython code ? If it's the case looks like you did a great job.
msg272754 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2016-08-15 12:49
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 ?
msg286261 - (view) Author: A. Skrobov (A. Skrobov) * Date: 2017-01-25 15:16
Oh btw, the comment in the beginning of Grammar/Grammar

> # Note:  Changing the grammar specified in this file will most likely
> #        require corresponding changes in the parser module
> #        (../Modules/parsermodule.c).

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?
msg286295 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2017-01-26 07:35
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 (#26526)
https://hg.python.org/cpython/rev/3275d4b584a2
History
Date User Action Args
2017-01-26 07:35:57python-devsetmessages: + msg286295
2017-01-25 15:16:32A. Skrobovsetmessages: + msg286261
2016-08-15 12:49:57A. Skrobovsetmessages: + msg272754
2016-08-10 07:02:17xcombellesetnosy: + xcombelle
messages: + msg272318
2016-08-09 19:44:29gregory.p.smithsetnosy: + gregory.p.smith
2016-06-02 18:32:59python-devsetstatus: open -> closed

nosy: + python-dev
messages: + msg266904

resolution: fixed
stage: commit review -> resolved
2016-06-02 18:28:48brett.cannonsetassignee: fdrake
messages: + msg266903
stage: patch review -> commit review
2016-05-31 15:03:10A. Skrobovsetfiles: + issue26526_16704_63395.diff
keywords: + patch
2016-05-31 07:24:11A. Skrobovsetmessages: + msg266741
2016-05-30 03:09:19giampaolo.rodolasetnosy: + giampaolo.rodola
2016-05-28 22:19:07fdrakesetmessages: + msg266577
2016-05-28 13:52:17berker.peksagsetnosy: + berker.peksag
2016-05-26 18:04:22brett.cannonsetnosy: + brett.cannon
2016-05-26 15:37:22fdrakesetmessages: + msg266441
2016-05-12 15:27:29serhiy.storchakasetstage: patch review
2016-05-12 15:22:50A. Skrobovsetmessages: + msg265411
2016-03-10 09:25:57vstinnersettitle: 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
2016-03-10 09:25:19vstinnersetnosy: + serhiy.storchaka
2016-03-10 09:23:35serhiy.storchakasetnosy: + benjamin.peterson
2016-03-10 09:02:31A. Skrobovcreate