Skip to content

gh-142433: Move deref to below the error when checking for laststring #142402

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
Open
AZero13 wants to merge 3 commits into
base: main
Choose a base branch
from
Open

gh-142433: Move deref to below the error when checking for laststring #142402

AZero13 wants to merge 3 commits into from
+5 −1

Conversation

@AZero13
Copy link
Contributor

@AZero13 AZero13 commented Dec 8, 2025 •
edited
Loading

Move deref of laststring to below the error checking so the deref is applied after the object in strings is replaced.

@AZero13 AZero13 requested a review from lysnikolaou as a code owner December 8, 2025 05:01
@bedevere-app
Copy link

bedevere-app bot commented Dec 8, 2025

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.

@bedevere-app
Copy link

bedevere-app bot commented Dec 8, 2025

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.

eendebakpt
eendebakpt reviewed Dec 8, 2025
View reviewed changes
@eendebakpt
Copy link
Contributor

eendebakpt commented Dec 8, 2025

@AZero13 Can you write a news entry and create a corresponding issue? See https://devguide.python.org/getting-started/pull-request-lifecycle/ for details

@bedevere-app
Copy link

bedevere-app bot commented Dec 8, 2025

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.

@AZero13 AZero13 changed the title Remove erroneous dereference of laststring gh-142433: Move deref to below the error when checking for laststring. Dec 8, 2025
@bedevere-app bedevere-app bot mentioned this pull request Dec 8, 2025
Open
@bedevere-app
Copy link

bedevere-app bot commented Dec 8, 2025

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.

@AZero13 AZero13 requested a review from eendebakpt December 8, 2025 22:35
@AZero13 AZero13 changed the title gh-142433: Move deref to below the error when checking for laststring. gh-142433: Move deref to below the error when checking for laststring Dec 8, 2025
@AZero13
pythongh-142433: Move deref to below the error when checking for last…
47557b9 
…string

Move deref of laststring to below the error checking so the deref is applied after the object in strings is replaced.
@eendebakpt
Copy link
Contributor

eendebakpt commented Dec 8, 2025

@AZero13 please avoid force pushes, it makes reviewing harder

eendebakpt
eendebakpt approved these changes Dec 8, 2025
View reviewed changes
Copy link
Contributor

@eendebakpt eendebakpt left a comment

Choose a reason for hiding this comment

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

The news entry needs a bit of work, but the change to the code looks good.

serhiy-storchaka
serhiy-storchaka approved these changes Dec 9, 2025
View reviewed changes
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

vstinner
vstinner reviewed Dec 9, 2025
View reviewed changes
vstinner
vstinner approved these changes Dec 9, 2025
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

LGTM

@vstinner vstinner added the needs backport to 3.14 bugs and security fixes label Dec 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

@vstinner vstinner vstinner approved these changes

@eendebakpt eendebakpt eendebakpt approved these changes

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

Assignees

No one assigned

Labels

awaiting merge needs backport to 3.14 bugs and security fixes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@AZero13 @eendebakpt @vstinner @serhiy-storchaka