Subtract a union member matched by a literal sequence-pattern element#3967
Open
markselby9 wants to merge 1 commit into
Open
Subtract a union member matched by a literal sequence-pattern element#3967markselby9 wants to merge 1 commit into
markselby9 wants to merge 1 commit into
Conversation
This comment has been minimized.
This comment has been minimized.
A `match` arm like `case ("b", _)` against a union member
`tuple[Literal["b"], int]` is irrefutable for that member, yet the member was
not subtracted from the subject in the fall-through/exhaustiveness branch — so
an otherwise-exhaustive match left a non-`Never` remainder. A full-capture
`case (k, v)` did subtract correctly.
The sequence pattern already emits an `Eq`/`Is` narrow on the element facet, but
`and_all` appends a `Placeholder` for every irrefutable element (e.g. the `_`),
and negating `And(..., Placeholder)` yields `Or(..., Placeholder)`, which
re-includes the whole type and blocks subtraction. Placeholders were only
stripped when every element was irrefutable, which a literal element defeats.
Strip the placeholders for any star-free pattern whose elements are all
irrefutable, value, or singleton patterns. This is sound because the
type-directed facet narrowing subtracts a member only when the element type
guarantees the match (exact literal/singleton) and preserves it otherwise
(e.g. `("b", _)` against `tuple[str, int]`).
Closes facebook#3883
a5ac090 to
049c8c3
Compare
|
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
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
Closes #3883.
A
matcharm whose sequence pattern has a literal element — e.g.case ("b", _)against the union membertuple[Literal["b"], int]— is irrefutable for that member, but the member was not subtracted from the subject in the fall-through / exhaustiveness branch. A full-capture pattern (case (k, v)) did subtract correctly. So this match left a non-Neverremainder when it should have been exhaustive:Root cause
The
MatchSequencearm already emits the right scope narrow —And(IsSequence, LenEq(n), <element narrows>), where a value element contributes anEq(v)narrow on the element facetIndex(i). ButNarrowOps::and_allappends anAtomicNarrowOp::Placeholderfor the subject whenever a sub-pattern contributes no narrow, which is exactly what an irrefutable element like_does. NegatingAnd(..., Placeholder)givesOr(..., Placeholder), andPlaceholdernarrows to the full type — so the union re-includes the member and nothing is subtracted. Placeholders were only stripped when every element was irrefutable, which a literal element defeats.Fix
Strip the placeholders for any star-free sequence pattern whose elements are all irrefutable, value, or singleton patterns (this is the maintainer's suggestion on the issue: treat variable bindings like wildcards, and let the literal element contribute its condition).
This is sound because the subtraction is gated by the type-directed facet narrowing (
atomic_narrow_for_facet'sNotEq/IsNotarms), which removes a union member only when the element type guarantees the match (exact literal/singleton) and preserves it otherwise:("b", _)vstuple[Literal["b"], int]→ subtracted ✓("b", _)vstuple[str, int](wider) → preserved ✓(1.0, 2.0)vstuple[float, float]→ preserved ✓ (unchanged)Star patterns are excluded (post-star elements get no facet subject), and nested/
OR/class sub-patterns fall to the existing conservative path — no behavior change there. Guarded arms (case ("b", _) if cond:) still don't subtract, because the guard re-introduces a placeholder.Test plan
pyrefly/lib/test/pattern_match.rs:test_match_sequence_literal_element_narrows_out_of_union— the issue repro (assert_neverreached)test_match_sequence_literal_element_with_capture_narrows— thecase ("b", v)capture formtest_match_sequence_literal_element_wider_type_no_strip— over-narrow guard:("b", _)vstuple[str, int]is preservedtest_match_sequence_refutable_subpattern_no_stripandtest_match_sequence_star_pattern_narrowsstill pass.cargo test -p pyrefly --lib— 5978 passed, 0 failed.python3 test.py --no-test --no-tensor-shapes --no-conformance --no-jsonschema— clean.