Narrow polymorphic variant row at ...rest catch-all#8382
Conversation
In a switch over a polymorphic variant, `| ...rest =>` (optionally
`...rest as r`) now binds `rest` to the scrutinee's row at that
structural position minus the tags matched by earlier unguarded arms,
rather than inheriting the full row. Works for closed and open rows,
for top-level scrutinees and nested positions (`Error(...rest)`,
`{field: ...rest}`, tuples with permissive siblings).
Implementation is type-checker-only, no parser changes. A pre-pass in
type_cases walks each arm's pattern, attaches a
res.polyVariantNarrowMatchedTags attribute to each Ppat_type+spread
node carrying the tags matched at the same structural path in earlier
arms. build_ppat_or_for_variant_spread rewrites the node to Ppat_var;
the Ppat_var handler reads the attribute and binds the variable via
the new Btype.narrow_row_by_tags helper, while pat_type stays at the
scrutinee's type so arm-wise unification and exhaustiveness checking
continue to see a full catch-all.
Signed-off-by: Jaap Frolich <jaap@tella.com>
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb1ed3ac02
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| | Ppat_variant (tag, Some payload) -> | ||
| (path, tag) :: extract_matched_at_path (Step_variant tag :: path) payload |
There was a problem hiding this comment.
Require permissive payload before subtracting a variant tag
Do not record the parent tag as definitely matched when a polymorphic-variant arm has a selective payload pattern. With | #A(#B) => ... | ...rest => ..., the first arm does not handle all #A values, but this code always adds (path, "A"), so the later ...rest can be narrowed as if #A were impossible. That can mis-type rest (e.g. rejecting valid #A(#C) flow or allowing code that assumes #A cannot appear). The parent tag should only be subtracted when the payload pattern is permissive.
Useful? React with 👍 / 👎.
Summary
In a switch over a polymorphic variant,
| ...rest =>(optionally...rest as r) now bindsrestto the residual row — the scrutinee's row at that structural position minus the tags matched by earlier unguarded arms — instead of inheriting the full row.Before
After
Scope
Works for:
[#A | #B | #C]) and open ([> #A | #B], inferred rows,row_fixed) scrutinee rows....restand nested positions: inside constructors (Error(...rest)), record fields ({inner: ...rest}), tuple elements with permissive siblings ((_, ...rest)), and variants-within-variants (#Outer(...rest))....rest as r— the alias also binds to the narrowed type.| #A | #B =>subtracts both tags....restarm itself — it doesn't contribute to the matched set so later arms see the same matched-so-far.Design
Type-checker-only. No parser changes, no new AST constructors, no new keywords.
Pre-pass in
type_cases(annotate_rest_nodes_with_matched_tags) walks each arm'sParsetree.patterntracking a structural path (constructor / tuple index / record field / variant payload) and, for everyPpat_type + res.patFromVariantSpreadnode, attaches a newres.polyVariantNarrowMatchedTagsattribute whose payload lists the tags matched by earlier unguarded arms at the same path.extract_matched_at_pathcollects what each arm unconditionally handles:Ppat_variant (tag, _)→ one match at the current path.Ppat_or→ union of branches (both branches together cover their tags).Ppat_tuple/Ppat_record→ descend into a sub-position only if every other sub-position is permissive (Ppat_any/Ppat_var), avoiding unsound subtraction from conditional patterns like(#A, #B).build_ppat_or_for_variant_spreadgets a new first branch: whenexpected_tyexpands toTvariant row, rewritePpat_type + spread attrtoPpat_var + matched_tags attrand return the narrowed type so the outerPpat_alias (…, r)handler propagates the narrowed type ontor.Btype.narrow_row_by_tags(new helper) produces the residual: matched tags becomeRabsent;row_fixedandrow_closedare inherited;row_nameis dropped;row_moreis a fresh variable so the residual is isolated from the source.Ppat_varhandler consumes the matched-tags attribute and binds the variable (enter_variable) to the narrowed row.pat_typestays atexpected_ty(the full scrutinee row) so per-arm unification intype_casesand parmatch's exhaustiveness checks still see a full catch-all.Soundness
(#A, _)subtracts#Aat position 0 (sibling is_);(#A, #B)subtracts nothing (neither position's match is unconditional). This preserves the invariant that a tag marked absent in the residual is truly unreachable at that path regardless of the sibling positions' runtime values.#P when g =>arm can fall through whengis false, so#Pstays reachable in later arms.Tests
Thirteen integration tests in
tests/tests/src/poly_variant_narrow_test.res:...rest as rbinds both names to the narrowed typeError(...rest)[> ...]annotationFirst(...)vsSecond(...))...restarm itself#Outer(...rest))'a. ([> …] as 'a) => …— exercisesrow_fixed)Plus two
super_errorsfixtures:polyvariant_narrow_wrong_type.res— passing the residual to a function that expects a narrower row produces a clear "doesn't include constructor" error.polyvariant_narrow_unreachable.res— pinning the Warning 11 behavior when every declared tag is already matched.Files changed
compiler/ml/typecore.mlbuild_ppat_or_for_variant_spread, narrow-awarePpat_varhandlercompiler/ml/variant_type_spread.mlmk_poly_variant_narrow_matched_tags_attr/get_poly_variant_narrow_matched_tagscompiler/ml/btype.ml+.mlinarrow_row_by_tagstests/tests/src/poly_variant_narrow_test.res(+ generated.mjs)tests/build_tests/super_errors/fixtures/polyvariant_narrow_*.res(+.expected)CHANGELOG.md:rocket: New FeatureTest plan
make test— full suite greennode scripts/test.js -mocha— all 13 new integration tests passnode ./tests/build_tests/super_errors/input.js— new snapshots match, no unrelated snapshot churnmake checkformat— OCaml/ReScript/JS/Rust formatting clean