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

[3.10] gh-125529: Avoid f-strings in the metagrammar #125582

Merged
merged 1 commit into from
Oct 22, 2024

Conversation

encukou
Copy link
Member

@encukou encukou commented Oct 16, 2024

Grammar actions need to be valid Python tokens and the accepted tokens need to be listed in the actions mini-grammar).

In Python 3.12+ (PEP 701), f-strings are no longer STRING tokens, so pegen fails to regenerate the metaparser on this Python version, as in:

PYTHON_FOR_REGEN=python3.12 make regen-pegen-metaparser

Use + and plain strings rather than f-strings.

Note that 3.9 and 3.11+ don't use f-strings in the metagrammar


This affects our PR CI fail in the Check if generated files are up to date step, like here: https://github.com/python/cpython/actions/runs/11274132145/job/31352641619?pr=125255

I see several ways we could solve this:

  • Remove this step from our CI
  • Change our CI to require Python 3.11
  • Merge this patch

I believe this patch is best:

  • The failing check is relevant: There are a few parties that build 3.10 using a current Python -- and I expect it to become more common as 3.12+ becomes the “system Python” even on conservative platforms.
  • Low chance of impactful breakage: OTOH, not that many people run pegen, and ones that do are likely equipped handle any failures.

But of course, it's entirely the RM's call. @pablogsal?

Grammar actions need to be valid Python tokens and the accepted tokens need to be
listed in the actions mini-grammar).

In Python 3.12+ (PEP 701), f-strings are no longer STRING tokens, so pegen fails
to regenerate the metaparser on this Python version, as in:

   PYTHON_FOR_REGEN=python3.12 make regen-pegen-metaparser

Use `+` and plain strings rather than f-strings.

Co-Authored-By: Petr Viktorin <[email protected]>
Copy link
Member

@pablogsal pablogsal 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 a good think for now. I will work separately on a patch to allow fixing this by joining the tokens

@pablogsal
Copy link
Member

Opened #125588

@pablogsal pablogsal closed this Oct 16, 2024
@pablogsal pablogsal reopened this Oct 16, 2024
@encukou
Copy link
Member Author

encukou commented Oct 16, 2024

Thanks! But, this change is still needed to regen the 3.10 metagrammar with existing releases of 3.12.

@pablogsal
Copy link
Member

Thanks! But, this change is still needed to regen the 3.10 metagrammar with existing releases of 3.12.

I know, but I want to fix the peg generator

@pablogsal pablogsal merged commit 6a2f12a into python:3.10 Oct 22, 2024
21 checks passed
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.

3 participants