Skip to content

gh-133555: Allow to regenerate the parser with Python < 3.14#133557

Merged
AA-Turner merged 5 commits into
python:mainfrom
alexprengere:fix_pegen_tstrings_tokens
May 8, 2025
Merged

gh-133555: Allow to regenerate the parser with Python < 3.14#133557
AA-Turner merged 5 commits into
python:mainfrom
alexprengere:fix_pegen_tstrings_tokens

Conversation

@alexprengere

@alexprengere alexprengere commented May 7, 2025 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Contributor

@bedevere-app bedevere-app Bot mentioned this pull request May 7, 2025
Closed
lysnikolaou
lysnikolaou approved these changes May 7, 2025
View reviewed changes

@lysnikolaou lysnikolaou left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch! LGTM! Thanks @alexprengere!

@AA-Turner

AA-Turner commented May 7, 2025

Copy link
Copy Markdown
Member

Should we add a test for this?

@lysnikolaou

lysnikolaou commented May 7, 2025

Copy link
Copy Markdown
Member

Hmm, not sure what the test would look like. If it's verifying the list of token, we would have to update every time a new token is added, which is as easy to miss as just changing the ifs like in this PR.

And I don't think we have the ability to run scripts with different python versions to verify they work, right?

@encukou

encukou commented May 7, 2025

Copy link
Copy Markdown
Member

Maybe add a reminder comment to e.g. Grammar/Tokens?

@AA-Turner

AA-Turner commented May 7, 2025

Copy link
Copy Markdown
Member

And I don't think we have the ability to run scripts with different python versions to verify they work, right?

We could add something to the Check if generated files are up to date CI check perhaps? Though just having a comment may be simpler.

A

AA-Turner
AA-Turner reviewed May 7, 2025
View reviewed changes
Comment thread Tools/peg_generator/pegen/parser_generator.py Outdated
@python-cla-bot

python-cla-bot Bot commented May 7, 2025 •
edited
Loading

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.

CLA signed

@alexprengere

alexprengere commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

After committing @AA-Turner suggestion directly from the GitHub UI, the CLA bot now require signing for the noreply mail used. Should I edit the commit manually to update the mail address, or is there a another way to do this?

@AA-Turner

AA-Turner commented May 7, 2025

Copy link
Copy Markdown
Member

If you click the 'CLA not signed' button, you should be able to sign it. If you have problems, can you paste a screenshot of what you see?

A

@alexprengere

alexprengere commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

All good now, thanks! I was not sure if we were supposed to sign those noreply mail addresses as well (my primary gmail addresses was already signed).

@lysnikolaou

lysnikolaou commented May 7, 2025

Copy link
Copy Markdown
Member

@alexprengere Could you add a comment to Grammar/Tokens that the peg generator (ideally mentioning specific files and lines) needs to be udpated as well when adding new tokens?

@alexprengere

alexprengere commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

Done.
Is there a fundamental reason why python -m pegen python does not take Grammar/Tokens as input, the same way that python -m pegen c does? This would also solve the issue of updating parser_generator.py when new tokens are added.

AA-Turner
AA-Turner approved these changes May 7, 2025
View reviewed changes
Comment thread Grammar/Tokens Outdated
Comment thread Grammar/Tokens
@alexprengere alexprengere force-pushed the fix_pegen_tstrings_tokens branch from 3609894 to e1ebec4 Compare May 7, 2025 17:17
alexprengere and others added 5 commits May 7, 2025 22:54
@alexprengere
pythongh-133555: allow to regenerate the parser with Python < 3.14
f56a805
@alexprengere @AA-Turner
Update Tools/peg_generator/pegen/parser_generator.py
bc0ffe8 
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@alexprengere
Add comment regarding updating the peg generator when adding tokens
5043a30
@alexprengere @AA-Turner
Update Grammar/Tokens
e336531 
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@alexprengere @AA-Turner
Update Grammar/Tokens
68066fd 
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@alexprengere alexprengere force-pushed the fix_pegen_tstrings_tokens branch from e1ebec4 to 68066fd Compare May 7, 2025 20:54
@AA-Turner AA-Turner added the needs backport to 3.14 bugs and security fixes label May 8, 2025
@AA-Turner AA-Turner merged commit b48599b into python:main May 8, 2025
@miss-islington-app

miss-islington-app Bot commented May 8, 2025

Copy link
Copy Markdown

Thanks @alexprengere for the PR, and @AA-Turner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.14.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 8, 2025
@alexprengere @AA-Turner @miss-islington
pythongh-133555: Allow regenerating the parser with Python < 3.14 (py…
c51f96f 
…thonGH-133557)

(cherry picked from commit b48599b)

Co-authored-by: Alex Prengère <2138730+alexprengere@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented May 8, 2025

Copy link
Copy Markdown

GH-133630 is a backport of this pull request to the 3.14 branch.

@bedevere-app bedevere-app Bot removed the needs backport to 3.14 bugs and security fixes label May 8, 2025
AA-Turner added a commit that referenced this pull request May 8, 2025
@miss-islington @alexprengere @AA-Turner
[3.14] gh-133555: Allow regenerating the parser with Python < 3.14 (G…
aadfc2c 
…H-133557) (#133630)

gh-133555: Allow regenerating the parser with Python < 3.14 (GH-133557)
(cherry picked from commit b48599b)

Co-authored-by: Alex Prengère <2138730+alexprengere@users.noreply.github.com>
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Pranjal095 pushed a commit to Pranjal095/cpython that referenced this pull request Jul 12, 2025
@alexprengere @AA-Turner @Pranjal095
pythongh-133555: Allow regenerating the parser with Python < 3.14 (py…
fe5ed86 
…thon#133557)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@alexprengere @AA-Turner @taegyunkim
pythongh-133555: Allow regenerating the parser with Python < 3.14 (py…
31279a4 
…thon#133557)

Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AA-Turner AA-Turner AA-Turner approved these changes
@lysnikolaou lysnikolaou lysnikolaou approved these changes
@pablogsal pablogsal Awaiting requested review from pablogsal pablogsal is a code owner

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@alexprengere @AA-Turner @lysnikolaou @encukou