Skip to content

gh-92932: dis._unpack_opargs should handle EXTENDED_ARG_QUICK - #92945

Merged
corona10 merged 4 commits into
python:mainfrom
corona10:gh-92932
Jun 3, 2022
Merged

gh-92932: dis._unpack_opargs should handle EXTENDED_ARG_QUICK#92945
corona10 merged 4 commits into
python:mainfrom
corona10:gh-92932

Conversation

@corona10

@corona10corona10 commented May 19, 2022•
edited
Loading

Copy link
Copy Markdown
Member

closes: gh-92932

@corona10
corona10 requested a review from colesburyMay 19, 2022 08:46
@corona10
corona10 marked this pull request as ready for review May 19, 2022 08:46
@corona10corona10 added the needs backport to 3.11 only security fixes label May 19, 2022
@corona10
corona10force-pushed the gh-92932 branch 2 times, most recently from 3bf6f02 to 30c7a4bCompareMay 19, 2022 08:55
@Fidget-Spinner

Fidget-Spinner commented May 19, 2022

Copy link
Copy Markdown
Member

Sorry Dong-hee, I'm useless at dis. Can't help you review this here :).

@corona10

corona10 commented May 19, 2022

Copy link
Copy Markdown
MemberAuthor

Sorry Dong-hee, I'm useless at dis. Can't help you review this here :).

Okay, Then I need to request this review from Brandt.

@corona10

corona10 commented May 19, 2022•
edited
Loading

Copy link
Copy Markdown
MemberAuthor
230261,230262c230261,230262< 1118398 EXTENDED_ARG_QUICK 1< 1118400 LOAD_GLOBAL 248 (g125)---> 1118398 EXTENDED_ARG_QUICK 513> 1118400 LOAD_GLOBAL 131576 (g65789)230265,230266c230265,230266< 1118416 EXTENDED_ARG_QUICK 1< 1118418 LOAD_GLOBAL 250 (g126)---> 1118416 EXTENDED_ARG_QUICK 513> 1118418 LOAD_GLOBAL 131578 (g65790)230269,230270c230269,230270< 1118434 EXTENDED_ARG_QUICK 1< 1118436 LOAD_GLOBAL 252 (g127)---> 1118434 EXTENDED_ARG_QUICK 513> 1118436 LOAD_GLOBAL 131580 (g65791)230273,230274c230273,230274< 1118452 EXTENDED_ARG_QUICK 1< 1118454 LOAD_GLOBAL 254 (g128)---> 1118452 EXTENDED_ARG_QUICK 513> 1118454 LOAD_GLOBAL 131582 (g65792)230277,230278c230277,230278< 1118470 EXTENDED_ARG_QUICK 2< 1118472 LOAD_GLOBAL 0 (g1)---> 1118470 EXTENDED_ARG_QUICK 514> 1118472 LOAD_GLOBAL 131584 (g65793)230281,230282c230281,230282< 1118488 EXTENDED_ARG_QUICK 2< 1118490 LOAD_GLOBAL 2 (g2)---> 1118488 EXTENDED_ARG_QUICK 514> 1118490 LOAD_GLOBAL 131586 (g65794)

brandtbucher
brandtbucher requested changes May 19, 2022
View reviewed changes

@brandtbucherbrandtbucher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix!

This should also have a test in test_dis. As a simple example, the statement *_, _ = ... should always generate an instruction UNPACK_EX(256) (proving that EXTENDED_ARG works correctly).

Comment threadLib/dis.py Outdated
@bedevere-bot

bedevere-bot commented May 19, 2022

Copy link
Copy Markdown

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

@corona10
corona10force-pushed the gh-92932 branch 4 times, most recently from f392232 to bd236e0CompareMay 20, 2022 09:02
@corona10

corona10 commented May 20, 2022

Copy link
Copy Markdown
MemberAuthor

I have made the requested changes; please review again

@bedevere-bot

bedevere-bot commented May 20, 2022

Copy link
Copy Markdown

Thanks for making the requested changes!

@brandtbucher: please review the changes made to this pull request.

@corona10

corona10 commented May 23, 2022

Copy link
Copy Markdown
MemberAuthor

@brandtbucher gentle ping :)

brandtbucher
brandtbucher approved these changes Jun 2, 2022
View reviewed changes

@brandtbucherbrandtbucher left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good! I found a couple of spelling nits, but feel free to merge otherwise:

Comment threadLib/test/test_dis.py Outdated
Comment threadLib/test/test_dis.py Outdated
Comment threadLib/test/test_dis.py Outdated
@corona10
corona10 merged commit b013804 into python:mainJun 3, 2022
@miss-islington

miss-islington commented Jun 3, 2022

Copy link
Copy Markdown
Contributor

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 3, 2022
@corona10@miss-islington
pythongh-92932: dis._unpack_opargs should handle EXTENDED_ARG_QUICK (p…
1fc77e3
…ythongh-92945)
(cherry picked from commit b013804)
Co-authored-by: Dong-hee Na <donghee.na@python.org>
@corona10

corona10 commented Jun 3, 2022•
edited
Loading

Copy link
Copy Markdown
MemberAuthor

Thank you Brandt for the review!

@corona10
corona10 deleted the gh-92932 branch June 3, 2022 02:30
miss-islington added a commit that referenced this pull request Jun 3, 2022
@miss-islington@corona10
gh-92932: dis._unpack_opargs should handle EXTENDED_ARG_QUICK (gh-92945)
e798f69
(cherry picked from commit b013804)
Co-authored-by: Dong-hee Na <donghee.na@python.org>
@terryjreedyterryjreedy removed the needs backport to 3.11 only security fixes label Feb 18, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@brandtbucherbrandtbucherbrandtbucher approved these changes
@colesburycolesburyAwaiting requested review from colesbury
@Fidget-SpinnerFidget-SpinnerAwaiting requested review from Fidget-Spinner

Assignees

@brandtbucherbrandtbucher

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

dis._unpack_opargs should handle EXTENDED_ARG_QUICK

6 participants

@corona10@Fidget-Spinner@bedevere-bot@miss-islington@brandtbucher@terryjreedy