X Tutup
The Wayback Machine - https://web.archive.org/web/20221223174725/https://github.com/python/cpython/pull/94552
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

Jump to bottom

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
Merged

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

merged 6 commits into from Jul 5, 2022

Conversation

iritkatriel
Copy link
Member

@iritkatriel iritkatriel commented Jul 4, 2022 •
edited

@iritkatriel iritkatriel requested a review from pablogsal Jul 4, 2022
@iritkatriel iritkatriel requested a review from markshannon as a code owner 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

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

@tiran tiran left a comment

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 changes
Copy link
Member

@pablogsal pablogsal left a comment

LGTM

@iritkatriel iritkatriel added type-bug An unexpected behavior, bug, or error needs backport to 3.11 labels Jul 5, 2022
@ambv ambv merged commit 324d019 into python:main Jul 5, 2022
14 checks passed
@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.

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
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tiran tiran

@pablogsal pablogsal

@markshannon markshannon

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.

None yet
7 participants
@iritkatriel @tiran @miss-islington @ambv @bedevere-bot @pablogsal @markshannon
X Tutup