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

Jump to bottom

gh-107901: synthetic jumps which are not at end of loop no longer check the eval breaker #113721

Merged
merged 6 commits into from Jan 6, 2024
Merged

gh-107901: synthetic jumps which are not at end of loop no longer check the eval breaker #113721

merged 6 commits into from Jan 6, 2024

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jan 5, 2024 •
edited by bedevere-app bot

This PR changes some of the synthetic non-loop JUMPs to be JUMP_NO_INTERRUPT.

The has two advantages:

  1. These jumps have no line number, so we don't want to perform an eval breaker check in them.
  2. It makes the code a bit smaller because there are no caches for these jumps.

@bedevere-app bedevere-app bot mentioned this pull request Jan 5, 2024
Open
@iritkatriel iritkatriel changed the title gh-107901: synthetic jumps which are not end of loop should not check the eval breaker gh-107901: synthetic jumps which are not at end of loop no longer check the eval breaker Jan 5, 2024
@iritkatriel iritkatriel removed the request for review from brettcannon January 5, 2024 00:50
markshannon
markshannon requested changes Jan 5, 2024
View reviewed changes
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

I think we need to convert JUMP -> JUMP_NO_INTERRUPT to JUMP.
Otherwise, looks good.

Python/flowgraph.c Show resolved Hide resolved
@bedevere-app
Copy link

bedevere-app bot commented Jan 5, 2024

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

@iritkatriel
Copy link
Member Author

I have made the requested changes; please review again.

markshannon
markshannon approved these changes Jan 6, 2024
View reviewed changes
Copy link
Member

@markshannon markshannon left a comment

Choose a reason for hiding this comment

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

Looks good

@iritkatriel iritkatriel merged commit d36a365 into python:main Jan 6, 2024
33 of 34 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chris-eibl chris-eibl chris-eibl left review comments

@markshannon markshannon markshannon approved these changes

@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner

@ncoghlan ncoghlan Awaiting requested review from ncoghlan ncoghlan is a code owner

@warsaw warsaw Awaiting requested review from warsaw warsaw is a code owner

@brandtbucher brandtbucher Awaiting requested review from brandtbucher brandtbucher is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@iritkatriel @markshannon @chris-eibl