Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Jul 5, 2022

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jul 4, 2022

@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

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.

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

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

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
… 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

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
…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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants