Replace the ASDL parser carried with CPython

[elibendersky]

BPO	19655
Nosy	@brettcannon, @rhettinger, @ncoghlan, @scoder, @larryhastings, @benjaminp, @skrah, @ericsnowcurrently, @serhiy-storchaka
Files	new-asdl-parser.issue19655.1.patch new-asdl-parser.issue19655.2.patch

^{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 = None
closed_at = <Date 2014-05-10.00:59:18.348>
created_at = <Date 2013-11-19.14:01:36.034>
labels = ['type-feature', 'build']
title = 'Replace the ASDL parser carried with CPython'
updated_at = <Date 2014-05-10.18:29:05.965>
user = 'https://bugs.python.org/elibendersky'

bugs.python.org fields:

activity = <Date 2014-05-10.18:29:05.965>
actor = 'scoder'
assignee = 'eli.bendersky'
closed = True
closed_date = <Date 2014-05-10.00:59:18.348>
closer = 'eli.bendersky'
components = ['Build']
creation = <Date 2013-11-19.14:01:36.034>
creator = 'eli.bendersky'
dependencies = []
files = ['34679', '34742']
hgrepos = []
issue_num = 19655
keywords = ['patch']
message_count = 34.0
messages = ['203376', '203381', '203382', '203384', '203385', '203429', '204069', '204225', '204240', '204266', '204267', '204279', '204281', '204282', '204368', '204377', '215210', '215217', '215219', '215234', '215447', '215518', '215530', '215574', '215613', '215615', '215616', '215635', '218207', '218210', '218213', '218215', '218216', '218232']
nosy_count = 12.0
nosy_names = ['brett.cannon', 'rhettinger', 'ncoghlan', 'scoder', 'larry', 'techtonik', 'benjamin.peterson', 'eli.bendersky', 'skrah', 'python-dev', 'eric.snow', 'serhiy.storchaka']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue19655'
versions = ['Python 3.5']

[elibendersky]

It was mentioned in one of the recent python-dev threads that making the Python code-base simpler to encourage involvement of contributors is a goal, so I figured this may be relevant.

I've recently written a new parser for the ASDL specification language from scratch and think it may be beneficial to replace the existing parser we use:

• The existing parser uses an external parser-generator (Spark) that we carry around in the Parser/ directory; the new parser is a very simple stand-alone recursive descent, which makes it easier to maintain and doesn't require a familiarity with Spark.
• The new code is significantly smaller. ~400 LOC for the whole stand-alone parser (asdl.py) as opposed to >1200 LOC for the existing parser+Spark.
• The existing asdl.py is old code and was only superficially ported to Python 3.x - this shows, and isn't a good example of using modern Python techniques. My asdl.py uses Python 3.4 with modern idioms like dict comprehensions, generators and enums.

For a start, it may be easier to review the parser separately and not as a patch file. I split it to a stand-alone project here: https://github.com/eliben/asdl_parser

The asdl.py there is a drop-in replacement for Parser/asdl.py; asdl_c.py is for Parser/asdl_c.py - with tiny modifications to interface the new parser (mainly getting rid of some Spark-induced quirks). The AST .c and .h files produced are identical. The repo also has some tests for the parser, which we may find useful in adding to the test suite or the Parser directory.

[elibendersky]

FWIW, asdl_c.py could use some "modernization", but I'll defer this to a later cleanup in order to do things gradually.

The same can be said for the Makefile rules - they can be simpler and more efficient (no need to invoke asdl_c / parse the ASDL twice, for example).

Incidentally, where would be a good place to put the ASDL tests? Can/should we reach into the Parser/ directory when running the Python regression tests (will this even work in an installed Python)? Or should I just plop asdl_test.py alongside asdl.py and mention that it should be run when asdl.py changes?

[larryhastings]

A week before beta? How confident are you in this new parser?

[elibendersky]

Larry, ease your worries: note that I only tagged this on version 3.5!

That said, this parser runs during the build and produces a .h file and .c file - these partake in the build; I verified that the generated code is *identical* to before, so there's not much to worry about. Still, I don't see a reason to land this before the beta branches. I'll do it onto the default branch after this weekend (assuming the reviews come through).

[BreamoreBoy]

Are we at beta for 3.5 already? :)

[benjaminp]

We could take the opportunity to ast scripts to a Tools/ subdir. Then you could use whatever it is test_tools.py uses.

[techtonik]

+1 for initiative, points that are nice to be addressed are below.

1. "Python 3.4 with modern idioms" - too Python-specific code raises the barrier. I'd prefer simplicity and portability over modernism. Like how hard is it to port the parser into JS with PythonJS, for example?

2. ASDL specification is mostly offline. One PDF survived, but IR browser and source for did not, which is a pity, because visual tools are one of the most attractive. In any case, it may worth to contact university - they might have backups and resurrect browser in Python (GCI, GSoC).

3. File organization. This is bad:
   Grammar/Grammar
   Parser/
   Python/
   This is good:
   Core/README.md
   Core/Grammar
   Core/Parser/
   Core/Processor/ (builds AST)
   Core/Interpreter/
   Core/Tests/
   I wonder what is PyPy layout? It may worth to steal it for consistency.

4. Specific problem with ASDL parsing - currently, by ASDL syntax all expr are allowed on the left side of assign node. This is not true for real app. It makes sense to clarify in README.md these borders (who checks what) and modify ASDL to reflect the restriction.

[brettcannon]

All code going into Python should be idiomatic unless it's meant to be released externally and there are backwards-compatibility concerns. It's Eli's call as to whether he wants to maintain a PyPI project for this after integration.

Points 2-4 are off-topic for this issue.

[elibendersky]

Does anyone have comments on the code or can I prepare a patch for default?

Would it make sense to wait with this until the 3.4 branch is created or can I just commit to default? Note that this change is not a new feature and is essentially a no-op as far as the resulting CPython executable - it's just tweaking the build process to generate the same data in a different way.

[brettcannon]

It's Larry's call in the end but I personally don't care either way, especially since this isn't user-facing code.

[larryhastings]

Are the generated files *byte for byte* the same as produced by the existing parser generation process?

[elibendersky]

Larry Hastings added the comment:

Are the generated files *byte for byte* the same as produced by the
existing parser generation process?

Correct. The generator runs during the build (in the Makefile), but only if
the files were out-of-date. It takes Python.asdl and produces Python-ast.h
and Python-ast.c; the latter two are compiled into the CPython executable.
The .h/.c files produced by my alternative generator are exactly identical
to the ones in there now.

I don't feel strongly about this, but I may need a refresher in the release
process rules. From today and until Feb 23rd, 2014 - are we not supposed to
be adding new features/PEPs or are we also not supposed to do any code
refactorings and/or any changes at all unless those are directly related to
fixing a specific bug/issue?