Narrow tuple out of union for irrefutable sequence patterns#3428
Open
lordhaa123 wants to merge 1 commit into
Open
Narrow tuple out of union for irrefutable sequence patterns#3428lordhaa123 wants to merge 1 commit into
lordhaa123 wants to merge 1 commit into
Conversation
The `match`/`case` matcher was failing to narrow sequence types out of a union when the sequence pattern had only irrefutable sub-patterns. For `match value:` with subject `float | tuple[float, float]` and case `(_, _)`, the fall-through cases still saw `tuple[float, float]`, so a final `assert_never(unreachable)` would wrongly fire. The sequence-pattern handler inserts a scope-level `And(IsSequence, LenEq(N))` narrow on the subject, then iterates sub-patterns calling `NarrowOps::and_all`. For wildcard sub-patterns each call appends a spurious `AtomicNarrowOp::Placeholder`, leaving `And(IsSequence, LenEq, Placeholder, Placeholder)`. Negating yields `Or(IsNotSequence, LenNotEq, Placeholder, Placeholder)` — and `Placeholder` is a no-op narrow, so the Or's join with `unions()` re-introduces the matched tuple type and defeats negative narrowing. This mirrors the same fix already in place for `Pattern::MatchClass`: when every sub-pattern is irrefutable (wildcards, bare names, `*rest`), the structural narrow alone fully describes what the pattern proves, so the Placeholders are spurious and should be stripped. For refutable sub-patterns (e.g. `case (1.0, 2.0)`) we leave them — the case can fail even when the structural narrow holds, so we must not remove length-2 sequences from later cases. Fixes facebook#3147. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Contributor
|
@stroxler has imported this pull request. If you are a Meta employee, you can view this in D105722881. |
|
According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅ |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
match/casedoes not narrowtupleout of a union #3147 — sequence patterns likecase (_, _):failed to narrow the matched tuple type out of a union in later cases, causing spuriousbad-argument-typeerrors onassert_neverexhaustiveness checks.MatchClassfix: when every sub-pattern is irrefutable (wildcards, bare names,*rest), strip the spuriousPlaceholdernarrow ops thatand_allappends, so negation of the scope-levelAnd(IsSequence, LenEq/LenGte)narrow actually eliminates the matched sequence type from subsequent cases. Refutable sub-patterns (e.g.case (1.0, 2.0)) are left alone — the case can still fail when the structural narrow holds.The commit message has the full root-cause analysis.
Test plan
test_match_sequence_pattern_narrows_tuple_out_of_union— verbatim issue repro, asserts zero errors.test_match_sequence_star_pattern_narrows— confirms[*_]narrows correctly.test_match_sequence_refutable_subpattern_no_strip— guards against over-stripping:case (1.0, 2.0)followed by_must leavefloat | tuple[float, float]in the fall-through.cargo test --package pyrefly --lib pattern_match— 64 passed.cargo test --package pyrefly --lib test::narrow— 192 passed.cargo test --package pyrefly --lib flow_branching— 151 passed.python3 test.py --no-test --no-conformance --no-jsonschema— clean.🤖 Generated with Claude Code