Skip to content

Changes how unreachable statements are detected, refs #11437 #11443

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

Closed
wants to merge 8 commits into from
Closed

Changes how unreachable statements are detected, refs #11437 #11443

wants to merge 8 commits into from

Conversation

sobolevn
Copy link
Member

@sobolevn sobolevn commented Nov 3, 2021

Closes #11437

I suspect that some mypy / mypy_primer tests can fail after this change.
But, it looks logical and consistent to me.

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

The overall idea seems good, but I think that there may be various tweaks that could make this even better.


def second() -> NoReturn:
raise Exception
raise Exception
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if this should be treated as unreachable code as well. Maybe the empty statement check for the following statements should be stricter, so that only pass, ... and maybe reveal_type(...) would be considered as empty, and maybe asserts.

Copy link
Member Author

@sobolevn sobolevn Nov 3, 2021

Choose a reason for hiding this comment

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

I think that the initial motivation was to allow some "debug" code to reduce the number of false positives. For example, I can see some cases where people would use code like:

  • assert False, 'unreachable'
  • raise RuntimeError('should not be reached')
  • sys.exit(1) # should not be reached

@sobolevn
Copy link
Member Author

sobolevn commented Nov 4, 2021

Thanks a lot, @JukkaL! I've addressed all your comments, except this one

Now:

  • reveal_type and reveal_locals are always considered "reachable"
  • Module level reachability has better locations

@KotlinIsland
Copy link
Contributor

@sobolevn looks like #10916 is very similar/duplicate of #11437, do you think this pr will resolve it as well?

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(🐞) 'warn-unreachable' doesn't reactivate after it is suppressed with a <nothing> statement
3 participants