Skip to content

bpo-36292: Mark unreachable code as such#12333

Merged
vstinner merged 2 commits into
python:masterfrom
stratakis:longobject_master
Mar 18, 2019
Merged

bpo-36292: Mark unreachable code as such#12333
vstinner merged 2 commits into
python:masterfrom
stratakis:longobject_master

Conversation

@stratakis
Copy link
Copy Markdown
Contributor

@stratakis stratakis commented Mar 14, 2019 •
edited by bedevere-bot
Loading

vstinner
vstinner requested changes Mar 14, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are modiying deadcode, it doesn't deserve a NEWS entry :-)

Comment thread Objects/longobject.c Outdated
Copy link
Copy Markdown
Member

@vstinner vstinner Mar 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is deadcode: long_bitwise() is a private function only called with '&', '^' or '|'. So I would prefer to use "Py_UNREACHABLE();" here.

Comment thread Objects/longobject.c Outdated
Copy link
Copy Markdown
Member

@vstinner vstinner Mar 14, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto: use "Py_UNREACHABLE();" instead. No need to both with references. It's deadcode.

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Mar 14, 2019

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.

@stratakis stratakis changed the title bpo-36292: Properly decref operands on failed bitwise operations bpo-36292: Mark unreachable code as such Mar 14, 2019
@terryjreedy
Copy link
Copy Markdown
Member

terryjreedy commented Mar 15, 2019

The news entry was deleted and Py_UNREACHABLE() used instead. I don't know if our convention is to add the somewhat redundant /* Unreachable code */. Either way, this seems ready to merge.

vstinner
vstinner requested changes Mar 18, 2019
View reviewed changes
Comment thread Objects/longobject.c Outdated
default:
PyErr_BadArgument();
return NULL;
/* Unreachable code */
Copy link
Copy Markdown
Member

@vstinner vstinner Mar 18, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I concur with @terryjreedy: the comment (here and above) is redudant, please remove it.

@vstinner vstinner merged commit a10d426 into python:master Mar 18, 2019
@stratakis stratakis deleted the longobject_master branch January 3, 2020 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@stratakis @bedevere-bot @terryjreedy @vstinner @serhiy-storchaka @the-knights-who-say-ni