[TimonGaertner]

Summary

When a branch does not terminate syntactically but ends in a statement whose type may be Never (a NoReturn call, or the fall-through of a type-exhaustive match), the merge of that branch already defers the "may be uninitialized" decision to solve time via a termination key. However, merge_flow always reset the merged flow's termination key to None, so this information was lost as soon as the construct was nested inside another branch. The enclosing merge then saw a live branch that does not define the variable and reported a false unbound-name:

def f(flag: bool, x: A) -> int:
    if flag:
        y = 1
    else:
        match x:
            case A():
                raise ValueError("x")
    return y  # ERROR `y` may be uninitialized [unbound-name]

The same happened for an if/else where both arms call a NoReturn function, nested inside another branch.

This PR makes merge_flow carry a termination key when every live branch has one. With a single live branch the key is reused as-is; with several, a Binding::Phi under a new Key::FlowTermination joins them, using each key as its own termination key so the result is Never exactly when all branches are. Loops are excluded, since the pre-loop path always stays live. Merges where all branches terminate syntactically are unchanged.

Fixes #5069

• I am a human, and I fully understand the code I am submitting.

Test Plan

Added tests in flow_branching.rs and pattern_match.rs covering:

• nested type-exhaustive match inside an if branch (no error)
• nested match inside a match arm, name bound in sibling arm (no error, the issue's original repro)
• nested if/else where both arms call NoReturn (no error)
• nested if/else where one arm is not NoReturn (still an error)
• nested non-exhaustive match (still an error)
• NoReturn inside a for loop body (still an error)

The three "no error" tests fail without the fix. Ran python3 test.py.

Note: the related symptom from the issue (no unreachable warning after an exhaustive match) is not addressed here. The reachability gate in bindings.rs deliberately skips exhaustive paths so that the assert_never idiom is not flagged, so that would be a separate design decision.

[meta-cla]

Hi @TimonGaertner!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

[meta-codesync]

This pull request has been imported. If you are a Meta employee, you can view this in D126795894. (Because this pull request was imported automatically, there will not be any future comments.)

[rchen152]

Thanks! I think this is likely good to merge, pending a mypy primer run to evaluate the impact on real-world code, but the merge conflicts prevent our auto-import hooks from working. Could you rebase?

[TimonGaertner]

@rchen152 Thanks! Rebased onto current main, and it's mergeable again. The only conflict was in pattern_match.rs next to the new sequence-pattern tests, and I kept both. The fix itself is unchanged.

[rchen152]

Review automatically exported from Phabricator review in Meta.

[rchen152]

Thanks again for the fix!

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

============================================================
SUMMARY
============================================================
Total: +6 new errors, -18 fixed errors
By preset: +0/-16 (default), +1/-16 (strict)

Projects with changes (11):
  mongo-python-driver: +0 -2
  egglog-python: +0 -1
  pytest: +0 -2
  jax: +1 -0
  manticore: +0 -1
  pandera: +0 -1
  stone: +1 -1
  bokeh: +0 -1
  hydpy: +0 -1
  pytensor: +0 -3
  core: +4 -5
============================================================

FULL DIFF DETAILS
------------------------------------------------------------

mongo-python-driver (https://github.com/mongodb/mongo-python-driver)
- ERROR pymongo/asynchronous/_kms_connect.py:87:19-39: Expected a callable, got `None` [not-callable]
- ERROR pymongo/synchronous/_kms_connect.py:87:19-39: Expected a callable, got `None` [not-callable]

egglog-python (https://github.com/egraphs-good/egglog-python)
- ERROR python/egglog/egraph.py:2817:24-35: Argument `BaseExpr | Fact` is not assignable to parameter `expr` with type `BaseExpr` in function `expr_action` [bad-argument-type]

pytest (https://github.com/pytest-dev/pytest)
- ERROR src/_pytest/doctest.py:585:33-39: `module` may be uninitialized [unbound-name]
- ERROR src/_pytest/unittest.py:296:57-64: `excinfo` may be uninitialized [unbound-name]

jax (https://github.com/google/jax)
+ ERROR jax/_src/pjit.py:1957:3-36: Unused `# pyrefly: ignore` comment for code(s): unbound-name [unused-ignore]

manticore (https://github.com/trailofbits/manticore)
- ERROR manticore/core/smtlib/visitors.py:644:16-21: The type of the returned value is partially unknown; it contains an implicit `Any` [partially-unknown-type]

pandera (https://github.com/pandera-dev/pandera)
- ERROR tests/pyspark/test_pyspark_container.py:657:32-39: `read_df` may be uninitialized [unbound-name]

stone (https://github.com/dropbox/stone)
- ERROR stone/compiler.py:125:28-48: Implicit conversion of `Unknown | None` to `bool` is not allowed [implicit-bool]
+ ERROR stone/compiler.py:125:28-48: Implicit conversion of `None` to `bool` is not allowed [implicit-bool]

bokeh (https://github.com/bokeh/bokeh)
- ERROR src/bokeh/command/bootstrap.py:117:8-11: `ret` may be uninitialized [unbound-name]

hydpy (https://github.com/hydpy-dev/hydpy)
- ERROR hydpy/core/filetools.py:609:16-26: Returned type `str | None` is not assignable to declared return type `str` [bad-return]

pytensor (https://github.com/pymc-devs/pytensor)
- ERROR pytensor/compile/executor.py:712:12-19: `outputs` may be uninitialized [unbound-name]
- ERROR pytensor/scan/op.py:1794:49-53: `t_fn` may be uninitialized [unbound-name]
- ERROR pytensor/scan/op.py:1796:44-51: `n_steps` may be uninitialized [unbound-name]

core (https://github.com/home-assistant/core)
- ERROR homeassistant/components/notion/coordinator.py:162:29-36: `bridges` may be uninitialized [unbound-name]
- ERROR homeassistant/components/notion/coordinator.py:163:31-40: `listeners` may be uninitialized [unbound-name]
- ERROR homeassistant/components/notion/coordinator.py:164:29-36: `sensors` may be uninitialized [unbound-name]
- ERROR homeassistant/components/notion/coordinator.py:165:38-54: `user_preferences` may be uninitialized [unbound-name]
+ ERROR homeassistant/components/notion/coordinator.py:162:29-45: The type of this argument is unknown because it comes from an error [unknown-from-error]
+ ERROR homeassistant/components/notion/coordinator.py:163:31-49: The type of this argument is unknown because it comes from an error [unknown-from-error]
+ ERROR homeassistant/components/notion/coordinator.py:164:29-45: The type of this argument is unknown because it comes from an error [unknown-from-error]
+ ERROR homeassistant/components/notion/coordinator.py:165:38-63: The type of this argument is unknown because it comes from an error [unknown-from-error]
- ERROR homeassistant/util/loop.py:105:19-36: `integration_frame` may be uninitialized [unbound-name]

[meta-codesync]

This pull request has been merged in 0ad7d1d.