Add a command-line flag and an environment variable to enable pegen

[lysnikolaou]

This is certainly a WIP and in no way complete. I'm just opening a PR for feedback, for things I might have missed etc.

Closes #4 and closes #5.

[lysnikolaou]

For example, a question that arises is what exactly the difference between _PyConfig_InitCompatConfig, PyConfig_InitPythonConfig and PyConfig_InitIsolatedConfig is.

[pablogsal]

For example, a question that arises is what exactly the difference between _PyConfig_InitCompatConfig, PyConfig_InitPythonConfig and PyConfig_InitIsolatedConfig is.

The first one is private and I think is used because a subset of the config is needed to initialize some parts first but I can ask Victor Stinner for it, for the other two you can read https://docs.python.org/3/c-api/init_config.html#init-python-config and https://docs.python.org/3/c-api/init_config.html#init-isolated-conf

[lysnikolaou]

So, at this time invoking python with the -p flag indeed enables pegen. But test_embed still fails and I can't really find out why. My instinct is that we need to add the new PyConfig field to some other function in initconfig.c as well. Any other ideas on what's missing?

[lysnikolaou]

I dropped Victor an email and he told me what the problem was. Turns out everything is documented here for anyone who might be interested: https://pythondev.readthedocs.io/pyconfig.html.

[gvanrossum]

I'd also like to see -p in the help output; otherwise this seems to be functional.

I suppose the environment variable (#5) and the explicit APIs (#6) will be separate PRs?

[lysnikolaou]

I suppose the environment variable (#5) and the explicit APIs (#6) will be separate PRs?

Since adding the environment variable is such a small change, I did the work on this PR.

[lysnikolaou]

In PyRun_InteractiveOneObjectEx there is the "problem" that the filename string is not present. Would we prefer to add another argument filename_str to that function or create a new pegen C API function called PyPegen_ASTFromFilePointer or something similar?

[pablogsal]

We should run the test suite with the new parser or at least some selected files like test_ast, test_grammar, ... to know that it works.

[pablogsal]

We should run the test suite with the new parser or at least some selected files like test_ast, test_grammar, ... to know that it works.

We can do this maybe by exporting the environment variable in the test.

[gvanrossum]

I just thought of something. I realize this may invalidate most of the work here, but I'd like to point it out anyway (I apologize and should have thought of this before):

When we switch to making the new parser the default, would we just invert the meaning of the -p flag and the PYTHONPEG env var? That seems weird, because it would mean there'd be no safe way for early adopters to force the PEG parser on (or off). It would be nicer if we could instead have the flag syntax be -p old and -p new, where we'd just switch the default. For the env var maybe we could use PYTHONPARSER=old and PYTHONPARSER=new. Other flag values would be errors; other env var values would be ignored.

Also, do we want compile() to use the new parser (if the flag is set)? I think so. Note that this will also affect ast.parse(), since it just calls compile(). That seems fine, but it means there would be no explicit way to call the old parser.

[pablogsal]

When we switch to making the new parser the default, would we just invert the meaning of the -p flag and the PYTHONPEG env var? That seems weird, because it would mean there'd be no safe way for early adopters to force the PEG parser on (or off). It would be nicer if we could instead have the flag syntax be -p old and -p new, where we'd just switch the default. For the env var maybe we could use PYTHONPARSER=old and PYTHONPARSER=new. Other flag values would be errors; other env var values would be ignored.

I think that makes sense. There are similar environment variables already like PYTHONMALLOC=debug or PYTHONMALLOC=malloc. Another possibility is just to design the flag so it only allows to call the old parser and just start with the new by default. This will simplify things a lot and there isn't much time until the first beta, so it should not make much difference since many people won't have time to use it.

[gvanrossum]

Another possibility is just to design the flag so it only allows to call the old parser and just start with the new by default.

Hm, maybe we've been acting with too much caution. We'd have to change this in the PEP. @lysnikolaou what do you think?

[lysnikolaou]

Another possibility is just to design the flag so it only allows to call the old parser and just start with the new by default.

Hm, maybe we've been acting with too much caution. We'd have to change this in the PEP. @lysnikolaou what do you think?

This is definitely true. There's only around 26 days between alpha6 and beta1. Many people won't really know about the change, let alone use and test it in these first 26 days.

Then again, having something -p new and PYTHONPARSER=new doesn't seem like that much extra work and it gives us a bit more flexibility, if you're reluctant about having pegen be the default.

I'm certainly leaning on making pegen the default parser. I don't think it makes much of a difference. Also, it would certainly give me much more confidence, because it would force that the whole CPython test suite passes with pegen enabled (which it still doesn't) really early on, which I think is a good thing.

[gvanrossum]

Let's implement the new/old option. That also gives us a negotiating position -- if people think it's too soon for Python 3.9, we can propose to have it available but off by default in 3.9, and reconsider in 3.10.

Also it seems that if the flag was on by default the test suite doesn't pass yet? We would need to fix that before turning it on by default (and the fix oughtn't be "run the test suite with the old parser" :-). That would be a separate PR I presume.

[lysnikolaou]

Also it seems that if the flag was on by default the test suite doesn't pass yet? We would need to fix that before turning it on by default (and the fix oughtn't be "run the test suite with the old parser" :-).

It's going to be multiple PRs most probably. Starting with #32.

[lysnikolaou]

In PyRun_InteractiveOneObjectEx there is the "problem" that the filename string is not present. Would we prefer to add another argument filename_str to that function or create a new pegen C API function called PyPegen_ASTFromFilePointer or something similar?

This is still an open question.

[gvanrossum]

Two more.

[gvanrossum]

I just discovered that interactive mode doesn't use the new parser yet even if you specify -pnew.

(Also, I noticed a few more PEP 7 violations -- basically you cannot have } else.)

[lysnikolaou]

I just discovered that interactive mode doesn't use the new parser yet even if you specify -pnew.

This is related to my comment above.

[gvanrossum]

This is related to my comment above.

How? Sorry for being dense.

[lysnikolaou]

This is related to my comment above.

How? Sorry for being dense.

In order for interactive mode to use pegen, we need to have the same if check in PyRun_InteractiveOneObjectEx. To do so, we first need to solve the problem, that we don't the filename string in that function. There is only a FILE * and a PyObject *.

We can either implement a new C API function that accepts a FILE * or we can just call PyUnicode_AsUTF8 with the filename object. What do you think is best?

[gvanrossum]

We can either implement a new C API function that accepts a FILE * or we can just call PyUnicode_AsUTF8 with the filename object. What do you think is best?

We need to make a function taking a FILE *, because in this case the file is already open. Typically it's the UNIX process stdin, which may not have a filename (e.g. a pipe).

[gvanrossum]

Okay, let's land this first and then deal with interactive mode.