Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-40939: Rename PyPegen* functions to PyParser* #21016

Merged
merged 5 commits into from Jun 21, 2020

Conversation

lysnikolaou
Copy link
Contributor

@lysnikolaou lysnikolaou commented Jun 20, 2020

Renaame PyPegen* functions to PyParser*, so that we can remove the
old set of PyParser* functions that were using the old parser, but keep
everything backwards-compatible.

https://bugs.python.org/issue40939

Renaame PyPegen* functions to PyParser*, so that we can remove the
old set of PyParser* functions that were using the old parser.
@pablogsal
Copy link
Member

pablogsal commented Jun 20, 2020

Seems that there some errors in Windows in Azure

@lysnikolaou lysnikolaou requested a review from a team as a code owner Jun 20, 2020
@pablogsal
Copy link
Member

pablogsal commented Jun 21, 2020

The windows build still fails, not sure where the errors are coming from :(

@lysnikolaou
Copy link
Contributor Author

lysnikolaou commented Jun 21, 2020

Yeah, me neither. I'm gonna continue investigating, but if someone from the @python/windows-team could enlighten us, it'd be helpful.

@lysnikolaou lysnikolaou reopened this Jun 21, 2020
@lysnikolaou
Copy link
Contributor Author

lysnikolaou commented Jun 21, 2020

I guess the failure is unrelated to the changes of this PR. It probably has something to do with the Azure environment, since the exact same build target succeeds in the Windows Github Action, but fails with multiple compile errors on Azure.

@pablogsal
Copy link
Member

pablogsal commented Jun 21, 2020

I guess the failure is unrelated to the changes of this PR. It probably has something to do with the Azure environment, since the exact same build target succeeds in the Windows Github Action, but fails with multiple compile errors on Azure.

But then why other PRs do not have the same problem?

@lysnikolaou
Copy link
Contributor Author

lysnikolaou commented Jun 21, 2020

But then why other PRs do not have the same problem?

Unfortunately, I don't know. Will investigate more tomorrow.

@nnemkin
Copy link
Contributor

nnemkin commented Jun 21, 2020

The reason for the breakage is that you're indirectly including Python-ast.h into Python.h (via pegen_interface.h).
Python-ast.h defines a lot of unprefixed names (historical mistake), like Yield or Set. It should never ever be included into Python.h (or any other header for that matter). Consider using struct _mod instead of mod_ty in header files.

@pablogsal
Copy link
Member

pablogsal commented Jun 21, 2020

Consider using struct _mod instead of mod_ty in header files.

Or split the header file into a private version and a public one, which I think is cleaner

@lysnikolaou
Copy link
Contributor Author

lysnikolaou commented Jun 21, 2020

It should never ever be included into Python.h (or any other header for that matter). Consider using struct _mod instead of mod_ty in header files.

And I was wondering why in pythonrun.h struct _mod * is used instead of mod_ty.

Include/pegen_interface.h Outdated Show resolved Hide resolved
@pablogsal pablogsal merged commit 564cd18 into python:master Jun 21, 2020
4 checks passed
fasih pushed a commit to fasih/cpython that referenced this pull request Jun 29, 2020
Rename PyPegen* functions to PyParser*, so that we can remove the
old set of PyParser* functions that were using the old parser.
arun-mani-j pushed a commit to arun-mani-j/cpython that referenced this pull request Jul 21, 2020
Rename PyPegen* functions to PyParser*, so that we can remove the
old set of PyParser* functions that were using the old parser.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants