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
Appearance settings

bpo-45056: Remove trailing unused constants from co_consts#28109

Merged
ambv merged 6 commits into
python:mainpython/cpython:mainfrom
methane:trim-unused-constsmethane/cpython:trim-unused-constsCopy head branch name to clipboard
Sep 2, 2021
Merged

bpo-45056: Remove trailing unused constants from co_consts#28109
ambv merged 6 commits into
python:mainpython/cpython:mainfrom
methane:trim-unused-constsmethane/cpython:trim-unused-constsCopy head branch name to clipboard

Conversation

@methane

@methane methane commented Sep 1, 2021 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

@methane methane requested a review from markshannon as a code owner September 1, 2021 08:16
@methane methane added the needs backport to 3.10 only security fixes label Sep 1, 2021
@pablogsal

pablogsal commented Sep 1, 2021

Copy link
Copy Markdown
Member

You need to regenerate importlib no?

@methane

methane commented Sep 1, 2021

Copy link
Copy Markdown
Member Author

You need to regenerate importlib no?

Yes. But it will cause conflict soon. So I want to delay make regen after code review.

gvanrossum
gvanrossum reviewed Sep 1, 2021
View reviewed changes
Comment thread Python/compile.c
@@ -7868,6 +7871,9 @@ assemble(struct compiler *c, int addNone)
if (duplicate_exits_without_lineno(c)) {
return NULL;

@gvanrossum gvanrossum Sep 1, 2021

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.

Now, I am curious why this isn’t “go to error”.

@gvanrossum

gvanrossum commented Sep 1, 2021

Copy link
Copy Markdown
Member

What if unused constants appear in the middle? Can that happen?

@methane

methane commented Sep 2, 2021

Copy link
Copy Markdown
Member Author

What if unused constants appear in the middle? Can that happen?

Yes, there are some cases. This pull request doesn't remove them because I am focusing the regressin at Python 3.10.
I uploaded the dump of unused constants of _pyio.py file at the b.p.o. Please see the diff of them.
Python 3.10 appends unused None for many functions.

@ambv ambv merged commit 55c4a92 into python:main Sep 2, 2021
@miss-islington

miss-islington commented Sep 2, 2021

Copy link
Copy Markdown
Contributor

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

@miss-islington

miss-islington commented Sep 2, 2021

Copy link
Copy Markdown
Contributor

Sorry, @methane and @ambv, I could not cleanly backport this to 3.10 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 55c4a92fc1abfe388335071f1d64b3addfa5793f 3.10

@ambv

ambv commented Sep 2, 2021

Copy link
Copy Markdown
Contributor

@methane, I'll deal with the backport.

ambv pushed a commit to ambv/cpython that referenced this pull request Sep 2, 2021
@methane @ambv
[3.10] bpo-45056: Remove trailing unused constants from co_consts (py…
4db6727 
…thonGH-28109).

(cherry picked from commit 55c4a92)

Co-authored-by: Inada Naoki <songofacandy@gmail.com>
@ambv ambv mentioned this pull request Sep 2, 2021
Merged
@bedevere-bot bedevere-bot removed the needs backport to 3.10 only security fixes label Sep 2, 2021
@bedevere-bot

bedevere-bot commented Sep 2, 2021

Copy link
Copy Markdown

GH-28125 is a backport of this pull request to the 3.10 branch.

ambv added a commit that referenced this pull request Sep 8, 2021
@ambv @methane
[3.10] bpo-45056: Remove trailing unused constants from co_consts (GH…
d41abe8 
…-28109) (GH-28125)

(cherry picked from commit 55c4a92)

Co-authored-by: Inada Naoki <songofacandy@gmail.com>
@gvanrossum gvanrossum mentioned this pull request Sep 14, 2021
Merged
@methane methane deleted the trim-unused-consts branch April 28, 2025 03:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@gvanrossum gvanrossum gvanrossum approved these changes

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

Assignees

@ambv ambv

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@methane @pablogsal @gvanrossum @miss-islington @ambv @bedevere-bot @the-knights-who-say-ni
Morty Proxy This is a proxified and sanitized view of the page, visit original site.