Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

gh-130775: Allow negative locations inast#130795

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
sobolevn merged 5 commits intopython:mainfromsobolevn:issue-130775
Apr 7, 2025

Conversation

@sobolevn
Copy link
Member

@sobolevnsobolevn commentedMar 3, 2025•
edited by bedevere-appbot
Loading

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

If we cannot bypass the ast.c layer and directly end up in the assemble.c, then this should be the correct approach (I didn't know where we validate the locations).

Comment on lines +211 to +214
{'lineno':-2,'col_offset':0},
{'lineno':0,'col_offset':-2},
{'lineno':0,'col_offset':-2,'end_col_offset':-2},
{'lineno':-2,'end_lineno':-2,'col_offset':0},
Copy link
Member

Choose a reason for hiding this comment

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

  • Off topic: are we already testing that passing a non-int is fine?
  • Maybe test with values that are excessively large (either in + or -) just for overflow checks.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

I will open a new issue for that, if we don't. Thanks for the idea.

Co-authored-by: Victor Stinner <vstinner@python.org>
vstinner
vstinner previously approved these changesMar 3, 2025
Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM. I just suggest a shorter test name :-)

@vstinnervstinner dismissed theirstale reviewMarch 7, 2025 16:08

I remove my LGTM vote, I didn't notice that -1 value is still allowed. I'm not sure about rejecting values <=-2.

@sobolevn
Copy link
MemberAuthor

sobolevn commentedMar 13, 2025•
edited
Loading

Ok, I followed the idea in#130795 (comment) by@JelleZijlstra and now all negative locations are just allowed. This way we won't have any compat issues and can backport this fix easily.

Thanks a lot, everyone!

Copy link
Member

@vstinnervstinner left a comment

Choose a reason for hiding this comment

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

LGTM.

@sobolevn
Copy link
MemberAuthor

Looks like#130627 changed a lot of related things. Digging into it.

@sobolevnsobolevn merged commitbc5233b intopython:mainApr 7, 2025
42 checks passed
@sobolevnsobolevn added needs backport to 3.12only security fixes needs backport to 3.13bugs and security fixes labelsApr 7, 2025
@miss-islington-app
Copy link

Thanks@sobolevn for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks@sobolevn for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Sorry,@sobolevn, I could not cleanly backport this to3.12 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker bc5233b6a5cdd8f77a4737ce317f94110869c082 3.12

@miss-islington-app
Copy link

Sorry,@sobolevn, I could not cleanly backport this to3.13 due to a conflict.
Please backport usingcherry_picker on command line.

cherry_picker bc5233b6a5cdd8f77a4737ce317f94110869c082 3.13

@sobolevn
Copy link
MemberAuthor

Thanks everyone! 🎉

@sobolevn
Copy link
MemberAuthor

I will work on manual backports today.

@sobolevnsobolevn changed the titlegh-130775: Validate negative locations inastgh-130775: Allow negative locations inastApr 7, 2025
sobolevn added a commit to sobolevn/cpython that referenced this pull requestApr 7, 2025
…30795)(cherry picked from commitbc5233b)Co-authored-by: sobolevn <mail@sobolevn.me>Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-app
Copy link

GH-132243 is a backport of this pull request to the3.13 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.13bugs and security fixes labelApr 7, 2025
Yhg1s pushed a commit that referenced this pull requestApr 8, 2025
)(cherry picked from commitbc5233b)Co-authored-by: Victor Stinner <vstinner@python.org>
sobolevn added a commit to sobolevn/cpython that referenced this pull requestApr 8, 2025
…30795)(cherry picked from commitbc5233b)Co-authored-by: sobolevn <mail@sobolevn.me>Co-authored-by: Victor Stinner <vstinner@python.org>
@bedevere-app
Copy link

GH-132260 is a backport of this pull request to the3.12 branch.

@bedevere-appbedevere-appbot removed the needs backport to 3.12only security fixes labelApr 8, 2025
Yhg1s pushed a commit that referenced this pull requestApr 8, 2025
)(cherry picked from commitbc5233b)Co-authored-by: sobolevn <mail@sobolevn.me>Co-authored-by: Victor Stinner <vstinner@python.org>
seehwan pushed a commit to seehwan/cpython that referenced this pull requestApr 16, 2025
Co-authored-by: Victor Stinner <vstinner@python.org>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@cfbolzcfbolzcfbolz left review comments

@picnixzpicnixzpicnixz left review comments

@vstinnervstinnervstinner approved these changes

@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

@Eclips4Eclips4Awaiting requested review from Eclips4Eclips4 is a code owner

@isidenticalisidenticalAwaiting requested review from isidentical

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

@iritkatrieliritkatrielAwaiting requested review from iritkatrieliritkatriel is a code owner

Assignees

@sobolevnsobolevn

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@sobolevn@cfbolz@vstinner@JelleZijlstra@iritkatriel@picnixz
[8]ページ先頭
©2009-2025 Movatter.jp