Skip to content

gh-94485: Set line number of module's RESUME instruction to 0, as specified by PEP 626#94552

Merged
ambv merged 6 commits intopython:mainfrom
iritkatriel:gh-94485
Jul 5, 2022
Merged

gh-94485: Set line number of module's RESUME instruction to 0, as specified by PEP 626#94552
ambv merged 6 commits intopython:mainfrom
iritkatriel:gh-94485

Conversation

@iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jul 4, 2022 •
edited
Loading

@iritkatriel iritkatriel requested a review from pablogsal July 4, 2022 23:30
@iritkatriel iritkatriel requested a review from markshannon as a code owner July 4, 2022 23:30
@tiran
Copy link
Member

tiran commented Jul 5, 2022

I think this needs a bytecode magic bump. The necessity to regen test_frozenmain.h indicate that the byte code has changed in an incompatible way. Better bump the magic now than in a later beta or rc.

@tiran
Copy link
Member

tiran commented Jul 5, 2022

Could you please run make patchcheck, too? One Azure test is failing. Commit c0453a4 added a stray white space:

diff --git a/Lib/test/test_compile.py b/Lib/test/test_compile.py
index b778eff12f5..ab1685d3e5b 100644
--- a/Lib/test/test_compile.py
+++ b/Lib/test/test_compile.py
@@ -1032,8 +1032,8 @@ def test_uses_slice_instructions(self):
         def check_op_count(func, op, expected):
             actual = 0
             for instr in dis.Bytecode(func):
-                 if instr.opname == op:
-                     actual += 1
+                if instr.opname == op:
+                    actual += 1
             self.assertEqual(actual, expected)
 
         def load():
[heimes@seneca cpytho

tiran
tiran approved these changes Jul 5, 2022
View reviewed changes
Copy link
Member

@tiran tiran left a comment

Choose a reason for hiding this comment

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

The PR looks technically correct and the change makes sense to me. I'm not an expert for Python bytecode, though. Feel free to either merge the PR now or wait for approval by an expert.

pablogsal
pablogsal approved these changes Jul 5, 2022
View reviewed changes
Copy link
Member

@pablogsal pablogsal left a comment

Choose a reason for hiding this comment

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

LGTM

@iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error needs backport to 3.11 only security fixes labels Jul 5, 2022
@ambv ambv merged commit 324d019 into python:main Jul 5, 2022
@miss-islington
Copy link
Contributor

miss-islington commented Jul 5, 2022

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

@miss-islington
Copy link
Contributor

miss-islington commented Jul 5, 2022

Sorry, @iritkatriel and @ambv, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 324d01944d16868b07df9e8eef6987766a31a36d 3.11

@ambv
Copy link
Contributor

ambv commented Jul 5, 2022 

Now backporting '324d01944d16868b07df9e8eef6987766a31a36d' into '3.11'
Error cherry-pick 324d01944d16868b07df9e8eef6987766a31a36d.
Auto-merging Python/compile.c
CONFLICT (content): Merge conflict in Python/compile.c
Auto-merging Programs/test_frozenmain.h
CONFLICT (content): Merge conflict in Programs/test_frozenmain.h
Auto-merging Lib/test/test_dis.py
CONFLICT (content): Merge conflict in Lib/test/test_dis.py
Auto-merging Lib/test/test_compile.py
CONFLICT (content): Merge conflict in Lib/test/test_compile.py
Auto-merging Lib/importlib/_bootstrap_external.py
CONFLICT (content): Merge conflict in Lib/importlib/_bootstrap_external.py
error: could not apply 324d01944d... gh-94485: Set line number of module's RESUME instruction to 0, as specified by PEP 626 (GH-94552)

Fun backport, looking 😎

ambv pushed a commit to ambv/cpython that referenced this pull request Jul 5, 2022
@iritkatriel @ambv
[3.11] pythongh-94485: Set line number of module's RESUME instruction…
2d83c5f 
… to 0, as specified by PEP 626 (pythonGH-94552)

Co-authored-by: Mark Shannon <mark@hotpy.org>
(cherry picked from commit 324d019)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
@bedevere-bot
Copy link

bedevere-bot commented Jul 5, 2022

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

@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 5, 2022
ambv added a commit that referenced this pull request Jul 5, 2022
@ambv
[3.11] gh-94485: Set line number of module's RESUME instruction to 0 …
1bfe83a 
…as specified by PEP 626 (GH-94552) (GH-94562)

Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com>
Co-authored-by: Mark Shannon <mark@hotpy.org>

(cherry picked from commit 324d019)
@SnoopJ SnoopJ mentioned this pull request Dec 10, 2022
Closed
@SonOfLilit SonOfLilit mentioned this pull request Jul 5, 2023
Closed
@iritkatriel iritkatriel deleted the gh-94485 branch July 25, 2023 18:07
@wimglenn wimglenn mentioned this pull request Dec 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiran tiran tiran approved these changes

@pablogsal pablogsal pablogsal approved these changes

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

Assignees

@ambv ambv

Labels

type-bug An unexpected behavior, bug, or error

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@iritkatriel @tiran @miss-islington @ambv @bedevere-bot @pablogsal @markshannon