Skip to content

bpo-32550. Remove the STORE_ANNOTATION bytecode.#5181

Merged
rhettinger merged 2 commits into
python:masterfrom
markshannon:remove-add-annotation-bytecode
Jan 30, 2018
Merged

bpo-32550. Remove the STORE_ANNOTATION bytecode.#5181
rhettinger merged 2 commits into
python:masterfrom
markshannon:remove-add-annotation-bytecode

Conversation

@markshannon

@markshannon markshannon commented Jan 14, 2018 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Member

The STORE_ANNOTATION bytecode can be implemented using STORE_SUBSCR.
This PR does just that.
Removes over 50 lines from the interpreter, adds 4 lines to the compiler.

https://bugs.python.org/issue32550

@markshannon
markshannon force-pushed the remove-add-annotation-bytecode branch from ac59ba2 to 662cc07 Compare January 14, 2018 12:39
ilevkivskyi
ilevkivskyi previously requested changes Jan 14, 2018
View reviewed changes

@ilevkivskyi ilevkivskyi 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.

I am -1 on this. As Serhiy mentioned on b.p.o. this will make annotations slower and doesn't cover corner cases.

I am however surprised that this is not caught by the tests.

@bedevere-bot

bedevere-bot commented Jan 14, 2018

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@rhettinger
rhettinger dismissed ilevkivskyi’s stale review January 29, 2018 06:00

Guido has opined that these little difference don't matter: https://bugs.python.org/issue32550#msg310354

@rhettinger rhettinger self-assigned this Jan 29, 2018
@markshannon
markshannon force-pushed the remove-add-annotation-bytecode branch from 7d5e204 to 5cd13af Compare January 29, 2018 23:37
@rhettinger
rhettinger merged commit 332cd5e into python:master Jan 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@1st1 1st1 1st1 approved these changes
@rhettinger rhettinger rhettinger approved these changes
@ilevkivskyi ilevkivskyi ilevkivskyi left review comments

Assignees

@rhettinger rhettinger

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@markshannon @bedevere-bot @1st1 @rhettinger @ilevkivskyi @the-knights-who-say-ni