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's make regen-pegen-metaparser fails with 3.12+ #125529

Open
encukou opened this issue Oct 15, 2024 · 2 comments
Open

3.10's make regen-pegen-metaparser fails with 3.12+ #125529

encukou opened this issue Oct 15, 2024 · 2 comments
Labels
3.10 only security fixes topic-parser

Comments

@encukou
Copy link
Member

encukou commented Oct 15, 2024

On the 3.10 branch:

$ PYTHON_FOR_REGEN=/usr/bin/python3.11 make regen-pegen-metaparser 
PYTHONPATH=./Tools/peg_generator /usr/bin/python3.11 -m pegen -q python \
./Tools/peg_generator/pegen/metagrammar.gram \
-o ./Tools/peg_generator/pegen/grammar_parser.py.new
/usr/bin/python3.11 ./Tools/scripts/update_file.py ./Tools/peg_generator/pegen/grammar_parser.py \
./Tools/peg_generator/pegen/grammar_parser.py.new

$ PYTHON_FOR_REGEN=/usr/bin/python3.12 make regen-pegen-metaparser 
PYTHONPATH=./Tools/peg_generator /usr/bin/python3.12 -m pegen -q python \
./Tools/peg_generator/pegen/metagrammar.gram \
-o ./Tools/peg_generator/pegen/grammar_parser.py.new
  File "./Tools/peg_generator/pegen/metagrammar.gram", line 87
    | NAME '[' type=NAME '*' ']' '=' ~ item {NamedItem(name.string, item, f"{type.string}*")}
                                                                          ^
SyntaxError: pegen parse failure
For full traceback, use -v
make: *** [Makefile:854: regen-pegen-metaparser] Error 1

This now makes PR CI fail, see e.g. https://github.com/python/cpython/actions/runs/11274132145/job/31352641619?pr=125255

Linked PRs

@lysnikolaou
Copy link
Contributor

lysnikolaou commented Oct 15, 2024

That's because of PEP 701 and the fact that f-strings are no longer STRING tokens (grammar actions need to be valid Python tokens and the accepted tokens need to be listed in the actions mini-grammar).

This patch solves the issue, though I don't know how we would fix this now that 3.10 is security-only:

diff --git a/Tools/peg_generator/pegen/grammar_parser.py b/Tools/peg_generator/pegen/grammar_parser.py
index 892df5cf8cc..de735649d46 100644
--- a/Tools/peg_generator/pegen/grammar_parser.py
+++ b/Tools/peg_generator/pegen/grammar_parser.py
@@ -420,7 +420,7 @@ def named_item(self) -> Optional[NamedItem]:
+++ b/Tools/peg_generator/pegen/grammar_parser.py
@@ -420,7 +420,7 @@ def named_item(self) -> Optional[NamedItem]:
             and
             (item := self.item())
         ):
-            return NamedItem ( name . string , item , f"{type.string}*" )
+            return NamedItem ( name . string , item , type . string + "*" )
         self.reset(mark)
         if cut: return None
         cut = False
diff --git a/Tools/peg_generator/pegen/metagrammar.gram b/Tools/peg_generator/pegen/metagrammar.gram
index bb4355fd189..77695d5e0a9 100644
--- a/Tools/peg_generator/pegen/metagrammar.gram
+++ b/Tools/peg_generator/pegen/metagrammar.gram
@@ -84,7 +84,7 @@ items[NamedItemList]:
     | named_item { [named_item] }
 
 named_item[NamedItem]:
-    | NAME '[' type=NAME '*' ']' '=' ~ item {NamedItem(name.string, item, f"{type.string}*")}
+    | NAME '[' type=NAME '*' ']' '=' ~ item {NamedItem(name.string, item, type.string+"*")}
     | NAME '[' type=NAME ']' '=' ~ item {NamedItem(name.string, item, type.string)}
     | NAME '=' ~ item {NamedItem(name.string, item)}
     | item {NamedItem(None, item)}

@encukou
Copy link
Member Author

encukou commented Oct 16, 2024

Thank you!

Ultimately that's the release manager's call, but I think this is the right solution. I'll prepare a PR and write some reasoning there.

encukou added a commit to encukou/cpython that referenced this issue 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.

Co-Authored-By: Petr Viktorin <[email protected]>
pablogsal pushed a commit that referenced this issue Oct 22, 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.

Co-authored-by: Lysandros Nikolaou <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.10 only security fixes topic-parser
Projects
None yet
Development

No branches or pull requests

2 participants