Flag unreachable return and yield statements

[terror]

Resolves #1812

This diff adds detection for return and yield statements that can never be reached because they follow a statement that always terminates flow, such as return, raise, break, or continue. We now have a new unreachable-statement error kind.

[meta-codesync]

@yangdanny97 has imported this pull request. If you are a Meta employee, you can view this in D88952397.

[ndmitchell]

Review automatically exported from Phabricator review in Meta.

[yangdanny97]

I found 2 cases where this would give false positives:

error swallowing context managers

https://pyrefly.org/sandbox/?project=N4IgZglgNgpgziAXKOBDAdgEwEYHsAeAdAA4CeS4ATrgLYAEAxrugC4z4tQTZ0Q3G5KLOnACuxYpXhwAOujmYYYXnAD6ALxjUAFPkS9WASkRy6ZugHcILABYjxk6doBaW3ABEIANwhwIzAFFKakpjU3MIgEY6AHo6fHCIsykWUUp0OgAxVCg4GES6FLSMgBVKURgQABoQMikwKFJCFlooCgBiOgAFUnrGkQwcAkZmSABzNNQWf3RCOU6AZRgYOhsWFmI4RBiYuqVGwkExmJh0GMxcBjgYpnRxyenmOLBBOlQvVGhUbFgRu4gJpQpjM6LhiI90HA5ugyLZmABaLxaPzMOgAXjoMhAAGZCJEAExYuQgAC%2BNVQDGmSMy0EqiBAaCweCIZFJQA

Since the body of the context manager can throw, the return statement inside may not be execute so the bottom is still reachable

sys.version_info checks

https://pyrefly.org/sandbox/?project=N4IgZglgNgpgziAXKOBDAdgEwEYHsAeAdAA4CeSIEAtsbgE4AuABHKXADrqeYxhMPwGACgCUiTk0lMIfVnEIA3GHTgRc6APoR0YXEwB8AXiZCAzABomARitiJUh3RgMArnXRMAKnRcx7kp1d3JgAxVCg4GBBzEDInMChSQgZcKigKAGImAAVSeMSWDBwCJgBjdUgAczdUBjV0Qk4sgGUYGCYACwYGYjhEAHp%2BuN5EwnpK-ph0fsxcUrh%2B8p0Iarpa%2Bv6mXTomVAVUaFRsWDKKlZq69SZcYkv0eU4yBg71AFolFXqmY3YQU0IrAAmX6cEAAXxiqFKdSUIWgUUQIDQWDwRDI4KAA

Even though we can statically determine which path the code takes by evaluating sys.version_info, libraries still need to support multiple versions.

I'm not sure if it would over-complicate this PR to support those cases. Will think about this some more.

[yangdanny97]

For context managers, we can't determine whether it's error-swallowing or not at the bindings phase, so the current analysis assumes it is non-swallowing.

This is a tricky tradeoff, since it might produce better default behavior in other cases, but for this specific analysis we really want a "are we 100% sure this statement is unreachable", and the way we treat context managers makes has_terminated not have that property.

Perhaps we can just add a different flag called is_unreachable that does encode the property that we want, where we treat context managers and statically evaluated conditions differently than we currently do.

I'll play around with that idea next week, no action needed from you for now, if it's easy enough to do I'll patch it onto this PR before merging.

[meta-codesync]

@yangdanny97 merged this pull request in 0ba978b.