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

bpo-33772: Fix couple of dead code paths #7418

Merged
merged 1 commit into from May 17, 2019
Merged

bpo-33772: Fix couple of dead code paths #7418

merged 1 commit into from May 17, 2019

Conversation

ghost
Copy link

@ghost ghost commented Jun 5, 2018

@ghost ghost changed the title Fix couple of dead code paths bpo-33164: Fix couple of dead code paths Jun 5, 2018
@methane methane changed the title bpo-33164: Fix couple of dead code paths bpo-33772: Fix couple of dead code paths Jun 6, 2018
Copy link
Member

@methane methane left a comment

Choose a reason for hiding this comment

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

In ast.c and compile.c, break before next case seems worth.
It notes that "this case clause doesn't fallthrough".

Others seems good to me.

@ghost
Copy link
Author

ghost commented Jun 6, 2018

In ast.c and compile.c they always return before the break

@methane
Copy link
Member

methane commented Jun 6, 2018

In ast.c and compile.c they always return before the break

I know. But the case clause is not so simple. It takes several seconds to read code for knowing it.

@ghost
Copy link
Author

ghost commented Jun 6, 2018

I hope I m not asking too much :-) but any opinion on this older PR ?

#6286

@csabella
Copy link
Contributor

Closing and re-opening to retrigger CI checks.

@csabella csabella closed this May 17, 2019
@csabella csabella reopened this May 17, 2019
@csabella csabella merged commit 27ee0f8 into python:master May 17, 2019
@csabella
Copy link
Contributor

@dcarlier-afilias, thank you for the pull request and, @methane, thank you for the code review and approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants