GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-130080: implement PEP 765 by iritkatriel · Pull Request #130087 · python/cpython

Conversation

@iritkatriel

@iritkatriel iritkatriel commented Feb 13, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Implement PEP 765 #130080

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@iritkatriel
iritkatriel requested a review from ncoghlan February 13, 2025 16:56
@iritkatriel
iritkatriel requested review from Eclips4, isidentical and markshannon as code owners February 13, 2025 16:56
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 13, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Feb 13, 2025
Implement PEP 765 #130080
Closed
@iritkatriel
iritkatriel requested review from AA-Turner and willingc as code owners February 13, 2025 19:08
@iritkatriel
iritkatriel marked this pull request as draft February 14, 2025 00:26
@bedevere-app bedevere-app Bot removed the awaiting core review label Feb 14, 2025
iritkatriel added 6 commits February 17, 2025 18:48
@iritkatriel
pythongh-130080: implement PEP 765
714c3f9
@iritkatriel
add docs
830fcfd
@iritkatriel
doctest doesn't like it
3a8d59e
@iritkatriel
formatting
51cdf5a
@iritkatriel
always run ast optimizer
480ac8e
@iritkatriel
fix test_unparse
17a554f
@iritkatriel
iritkatriel force-pushed the pep-765 branch from 7f3284e to 17a554f Compare February 18, 2025 10:23
@iritkatriel
add tests for ast.parse()
d344fba
@iritkatriel
iritkatriel marked this pull request as ready for review February 18, 2025 10:32
@iritkatriel
iritkatriel requested a review from ericsnowcurrently as a code owner February 18, 2025 10:32
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 18, 2025
iritkatriel and others added 2 commits February 18, 2025 10:32
@iritkatriel
Merge branch 'main' into pep-765
90f6514
@iritkatriel
update test___all__
0994bf9
markshannon
markshannon requested changes Mar 3, 2025
View reviewed changes

@markshannon markshannon left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good, but I'm not sure I entirely trust the overwrite/save/overwrite logic.

Saving, overwriting and then overwriting again seems an error prone way to maintain a stack of the current context. ControlFlowInFinallyState can fit in a single byte and the depth of try-finally is limited to 20 (CO_MAXBLOCKS) so a stack will only take 20 bytes or so.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ast_opt.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Python/ast_opt.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting core review label Mar 3, 2025
@bedevere-app

bedevere-app Bot commented Mar 3, 2025

Copy link
Copy Markdown

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added the awaiting changes label Mar 3, 2025
iritkatriel added 2 commits March 4, 2025 16:52
@iritkatriel
rename State->Context. no return value
df8f210
@iritkatriel
use stack for context
ed2f4e7
@iritkatriel

iritkatriel commented Mar 5, 2025

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
tomasr8
tomasr8 reviewed Mar 15, 2025
View reviewed changes

@tomasr8 tomasr8 left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I noticed I had some pending comments which I forgot to send.. 😄

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/whatsnew/3.14.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.14.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel and others added 4 commits March 15, 2025 18:27
@iritkatriel
fix crash
1f5323c
@iritkatriel @tomasr8
Apply suggestions from code review
7a9a3e1 
Co-authored-by: Tomas R. <[email protected]>
@iritkatriel
typo
6a37d81
@iritkatriel
fix windows issue
0b257c3
@iritkatriel

iritkatriel commented Mar 15, 2025

Copy link
Copy Markdown
Member Author

I have made the requested changes; please review again.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting change review and removed awaiting changes labels Mar 15, 2025
@bedevere-app

bedevere-app Bot commented Mar 15, 2025

Copy link
Copy Markdown

Thanks for making the requested changes!

@markshannon: please review the changes made to this pull request.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app
bedevere-app Bot requested a review from markshannon March 15, 2025 20:45
@iritkatriel
Merge branch 'main' into pep-765
3425378
markshannon
markshannon approved these changes Mar 17, 2025
View reviewed changes

@markshannon markshannon left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

There are two unused fields in _PyASTOptimizeState that need removing, otherwise looks good.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Python/ast_opt.c Outdated
int ff_features;
int syntax_check_only;

int recursion_depth; /* current recursion depth */

@markshannon markshannon Mar 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

This doesn't appear to be used anywhere (same for recursion_limit)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@iritkatriel iritkatriel Mar 17, 2025

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks. I think they were removed in the const folding PRs after I created this one.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting change review labels Mar 17, 2025
@iritkatriel
removed unused
a4b65f7
picnixz
picnixz reviewed Mar 17, 2025
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Just some docs comments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/reference/compound_stmts.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/reference/compound_stmts.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/reference/compound_stmts.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread Doc/whatsnew/3.14.rst
* :ref:`PEP 741: Python Configuration C API <whatsnew314-pep741>`
* :ref:`PEP 761: Discontinuation of PGP signatures <whatsnew314-pep761>`
* :ref:`A new type of interpreter <whatsnew314-tail-call>`
* :ref:`PEP 765: Disallow return/break/continue that exit a finally block <whatsnew314-pep765>`

@picnixz picnixz Mar 17, 2025

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

For the reader, I think it may be better to have the PEP bullet points grouped.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Doc/whatsnew/3.14.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
iritkatriel and others added 2 commits March 17, 2025 20:12
@iritkatriel @picnixz
Apply suggestions from code review
a00f297 
Co-authored-by: Bénédikt Tran <[email protected]>
@iritkatriel @picnixz
Update Doc/whatsnew/3.14.rst
459ccf0 
Co-authored-by: Bénédikt Tran <[email protected]>
@iritkatriel
iritkatriel enabled auto-merge (squash) March 17, 2025 20:30
@iritkatriel
iritkatriel merged commit ffc2f1d into python:main Mar 17, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Mar 17, 2025
@AA-Turner AA-Turner mentioned this pull request Mar 18, 2025
PEP 765: Mark as Final python/peps#4305
Merged
6 tasks done
colesbury pushed a commit to colesbury/cpython that referenced this pull request Mar 20, 2025
@iritkatriel @colesbury
pythongh-130080: implement PEP 765 (python#130087)
eb47811
@iritkatriel iritkatriel mentioned this pull request Mar 22, 2025
gh-130080: do not fold match case constants in unoptimized AST #131577
Merged
@brianschubert brianschubert mentioned this pull request Aug 1, 2025
compile fails on 3.14 on a valid expression when -OO is set #137308
Closed
@sobolevn sobolevn mentioned this pull request Aug 2, 2025
gh-137308: Replace a single docstring with pass #137318
Merged
sobolevn added a commit that referenced this pull request Aug 2, 2025
@sobolevn
gh-137308: Replace a single docstring with pass in -OO mode (#137318
b74f3be 
)

This is required so we would never have empty node bodies.
Refs #130087
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Aug 2, 2025
@sobolevn @miss-islington
pythongh-137308: Replace a single docstring with pass in -OO mode (…
b150daa 
…pythonGH-137318)

This is required so we would never have empty node bodies.
Refs pythonGH-130087
(cherry picked from commit b74f3be)

Co-authored-by: sobolevn <[email protected]>
@miss-islington miss-islington mentioned this pull request Aug 2, 2025
[3.14] gh-137308: Replace a single docstring with pass in -OO mode (GH-137318) #137322
Merged
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
@sobolevn @Agent-Hellboy
pythongh-137308: Replace a single docstring with pass in -OO mode (…
c4415d4 
…python#137318)

This is required so we would never have empty node bodies.
Refs python#130087
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomasr8 tomasr8 tomasr8 left review comments
@picnixz picnixz picnixz left review comments
@markshannon markshannon markshannon approved these changes
@ncoghlan ncoghlan Awaiting requested review from ncoghlan
@isidentical isidentical Awaiting requested review from isidentical
@Eclips4 Eclips4 Awaiting requested review from Eclips4 Eclips4 is a code owner
@willingc willingc Awaiting requested review from willingc willingc is a code owner
@AA-Turner AA-Turner Awaiting requested review from AA-Turner AA-Turner is a code owner
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
+1 more reviewer
@WolframAlph WolframAlph WolframAlph left review comments
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@iritkatriel @tomasr8 @markshannon @picnixz @WolframAlph
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.