Skip to content

gh-94438: Add additional cases to mark_stacks with tests #111237

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.

Sign up for GitHub

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
Merged
merged 8 commits into from
Oct 24, 2023
Merged

gh-94438: Add additional cases to mark_stacks with tests #111237

merged 8 commits into from
Oct 24, 2023

Conversation

savannahostrowski
Copy link
Member

@savannahostrowski savannahostrowski commented Oct 23, 2023 •
edited by bedevere-app bot
Loading

After reading through #94438, it appeared that there were still outstanding issues with mark_stacks as there was no case that checking for POP_JUMP_IF_NONE and POP_JUMP_IF_NOT_NONE, which could break pdb's jump in 3.11 or higher. This adds these case to the switch statement plus a couple of tests.

@ghost
Copy link

ghost commented Oct 23, 2023 •
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot mentioned this pull request Oct 23, 2023
Closed
3 tasks
@bedevere-app
Copy link

bedevere-app bot commented Oct 23, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

@savannahostrowski
Copy link
Member Author

cc: @brandtbucher

@brandtbucher brandtbucher self-assigned this Oct 23, 2023
@brandtbucher brandtbucher self-requested a review October 23, 2023 22:00
@brandtbucher brandtbucher added type-bug An unexpected behavior, bug, or error interpreter-core (Objects, Python, Grammar, and Parser dirs) needs backport to 3.11 only security fixes needs backport to 3.12 only security fixes labels Oct 23, 2023
@brandtbucher
Copy link
Member

brandtbucher commented Oct 23, 2023 •
edited
Loading

(Note to self that the 3.11 backport will probably need to be manual.)

@bedevere-app
Copy link

bedevere-app bot commented Oct 23, 2023

Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply the skip news label instead.

brandtbucher
brandtbucher requested changes Oct 23, 2023
View reviewed changes
Copy link
Member

@brandtbucher brandtbucher left a comment

Choose a reason for hiding this comment

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

Thanks a ton for this! This is some really tricky code.

Just a couple of comments on the tests:

@bedevere-app
Copy link

bedevere-app bot commented Oct 23, 2023

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

@brandtbucher
Copy link
Member

Sorry, one last thing... can you add yourself to Misc/ACKS? :)

@brandtbucher brandtbucher merged commit 6640f1d into python:main Oct 24, 2023
@miss-islington-app
Copy link

Thanks @savannahostrowski for the PR, and @brandtbucher for merging it 🌮🎉.. I'm working now to backport this PR to: 3.11, 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry, @savannahostrowski and @brandtbucher, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 6640f1d8d2462ca0877e1d2789e1721767e9caf2 3.11

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 24, 2023
@miss-islington
pythonGH-94438: Restore ability to jump over None tests (pythonGH-111237
2c77dfb 
)

(cherry picked from commit 6640f1d)

Co-authored-by: Savannah Ostrowski <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Oct 24, 2023

GH-111243 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Oct 24, 2023
@willingc
Copy link
Contributor

Congrats @savannahostrowski for the successful contribution. 🎉

@brandtbucher 😄

@savannahostrowski savannahostrowski mentioned this pull request Oct 25, 2023
Merged
@bedevere-app
Copy link

bedevere-app bot commented Oct 25, 2023

GH-111338 is a backport of this pull request to the 3.11 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.11 only security fixes label Oct 25, 2023
@gaogaotiantian gaogaotiantian mentioned this pull request Oct 26, 2023
Merged
aisk pushed a commit to aisk/cpython that referenced this pull request Feb 11, 2024
@aisk
pythonGH-94438: Restore ability to jump over None tests (pythonGH-111237
d9715b6 
)
Glyphack pushed a commit to Glyphack/cpython that referenced this pull request Sep 2, 2024
@Glyphack
pythonGH-94438: Restore ability to jump over None tests (pythonGH-111237
a721258 
)
@savannahostrowski savannahostrowski deleted the jump_if_none branch September 27, 2024 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@brandtbucher brandtbucher brandtbucher approved these changes

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

Assignees

@brandtbucher brandtbucher

Labels
interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@savannahostrowski @brandtbucher @willingc