Skip to content

bpo-32550. Remove the STORE_ANNOTATION bytecode.#5181

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

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

Conversation

@markshannon
Copy link
Copy Markdown
Member

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

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

@ilevkivskyi ilevkivskyi left a comment

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
Copy link
Copy Markdown

bedevere-bot commented Jan 14, 2018

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