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-40334: refactor and cleanup for the PEG generators #19775

Merged
merged 3 commits into from
Apr 29, 2020

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Apr 28, 2020

Improvements in this PR:

  • No more relying on the string names to deduce the types of local variables.
  • No more threading down the names of local variables from all functions and
    visiting: now the code has a local variable scope with a context manager.
  • Specially for @lysnikolaou: I have actually managed to do bpo-40334: Refactor peg_generator to receive a Tokens file when building c code #19745 (comment) and now we don't need all the specialized token functions. Just one for name, string and number.
  • Now we have a class to represent function calls, making the code a bit
    more explicit and carrying metadata if needed. Also helps when debugging (from experience 😉 ).
  • Fix some mypy errors (like the fact that in add_var the variable type could actually have been None as opposed to our previous declaration).

https://bugs.python.org/issue40334

@pablogsal pablogsal changed the title bpo-40334: refactor and cleanup for the PEG c_generator bpo-40334: refactor and cleanup for the PEG generators Apr 28, 2020
@lysnikolaou
Copy link
Contributor

  • Specially for @lysnikolaou: I have actually managed to do #19745 (comment) and now we don't need all the specialized token functions. Just one for name, string and number.

😃 😃

Copy link
Contributor

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really a great improvement! Thanks a a lot! Most of my comments are stylistic, so feel free to ignore them.

Tools/peg_generator/pegen/c_generator.py Outdated Show resolved Hide resolved
Tools/peg_generator/pegen/c_generator.py Outdated Show resolved Hide resolved
Tools/peg_generator/pegen/c_generator.py Outdated Show resolved Hide resolved
Tools/peg_generator/pegen/c_generator.py Outdated Show resolved Hide resolved
pablogsal and others added 2 commits April 29, 2020 10:12
Co-Authored-By: Lysandros Nikolaou <lisandrosnik@gmail.com>
Copy link
Contributor

@lysnikolaou lysnikolaou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes! 🚀

@pablogsal pablogsal merged commit 4db245e into python:master Apr 29, 2020
@pablogsal pablogsal deleted the better_generator branch April 29, 2020 09:42
@bedevere-bot

This comment has been minimized.

@bedevere-bot

This comment has been minimized.

@pablogsal
Copy link
Member Author

The buildbot failures are unrelated and they are being addressed here:

#19791

@bedevere-bot

This comment has been minimized.

@bedevere-bot

This comment has been minimized.

@gvanrossum
Copy link
Member

Thanks for this one too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants