Repository navigation
Warn about untagged payload patterns shadowed by literal cases - #8914
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## untagged-declarations #8914 +/- ##
=========================================================
+ Coverage 81.31% 81.33% +0.01%
=========================================================
Files 456 456
Lines 64135 64273 +138
=========================================================
+ Hits 52151 52275 +124
- Misses 11984 11998 +14
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8914 |
cristianoc
left a comment
cristianoc
left a comment
There was a problem hiding this comment.
In case relevant: the original design was intentionally order independent.
Whether this changes that property I don't know.
If it does, we should document the design change.
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f39e21efcd
ℹ️ 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".
|
@codex review |
Changelog🐛 Bug fix
Semantic Diff7 commits · Full page Warn about untagged payload patterns shadowed by literal casesfdc312f · cknitt + Claude · Oct 19, 2026, 13:31 UTC A payload pattern whose values are all literal-case values can never match (warning 11/12), and a literal case covers the equal bool payload value in the exhaustiveness check. Fixes #8806. Report shadowed payload alternatives when only warning 12 is ona9780f6 · cknitt + Claude · Oct 20, 2026, 11:53 UTC The check for untagged payload patterns shadowed by literal cases ran only when warning 11 was active, so with Example (rescript): @unboxed type t = | @as("a") A | S(string)
@warning("-11")
let g = x => switch x { | S("a") => 1 | S("a" | "c") => 2 | S(_) | A => 3 }Diagnostic (text): Before: no warnings
After: Warning 12 on the "a" in S("a" | "c"); the dead S("a") case stays
silent because warning 11 is off.Describe the shadowed payload warnings in the spec552b55f · cknitt + Claude · Oct 24, 2026, 11:39 UTC Rule 7 says which payload patterns a literal case shadows, which warnings report them, and how a literal case counts in the exhaustiveness check. Compare bigint patterns and constants by their valuesbbdc52d · cknitt + Claude · Oct 25, 2026, 13:58 UTC
Bigint pattern constants and bigint constants have their literal, so the shadowed-payload warnings and constant folding compare them as Diagnostic added (shadowed bigint payload) @unboxed type t = | @as(2n) Two | BI(bigint) | S(string)
let f = (v: t) =>
switch v {
| BI(02n) => "dead"
| BI(_) => "bigint"
| _ => "other"
}
-none
+this match case is unused.
+2n is the runtime value of constructor Two, so it is always matched as Two, never as BI.Report shadowed polymorphic variant and aliased or-pattern payloads6fd2835 · cknitt + Claude · Oct 25, 2026, 15:03 UTC
A polymorphic variant without payload stands for its name, or the number its name spells, as in Diagnostic added (a shadowed polymorphic variant payload) @unboxed type t = | @as("Foo") F | P([#Foo | #Bar])
let f = x => switch x { | F => 1 | P(#Foo) => 2 | P(_) => 3 }
-none
+Warning number 11
+this match case is unused.
+"Foo" is the runtime value of constructor F, so it is always matched as F, never as P.Diagnostic added (an aliased or-pattern) @unboxed type t = | @as("a") A | S(string)
let f = x => switch x { | S(("a" | "b") as s) => s | S(_) | A => "other" }
-none
+Warning number 12
+this sub-pattern is unused.
+"a" is the runtime value of constructor A, so it is always matched as A, never as S.Cover payloads by a bool literal only when they are bools7c05b9a · cknitt + Claude · Oct 25, 2026, 16:51 UTC
Only a payload whose type expands to Diagnostic added (a @typeOf("boolean") type jsBool
@unboxed type t = | @as(true) Yes | Value(jsBool)
let f = (v: t) => switch v { | Yes => 1 }
-none
+Warning number 8
+You forgot to handle a possible case here, for example:
+| Value(_)✨ Keep outer literal cases when looking into nested untagged payloads8cb50ef · cknitt + Claude · Oct 25, 2026, 16:58 UTC · new
The literal cases that take precedence are carried down through untagged payloads, outermost first, each with the payload constructor it shadows. Diagnostic added (an or-pattern below a nested untagged payload) @unboxed type inner = | @as("x") X | N(int) | S(string)
@unboxed type outer = | @as(1) One | W(inner)
let f = v => switch v { | One => "one" | W(N(1 | 2)) => "small" | W(_) => "other" }
-none
+Warning number 12
+this sub-pattern is unused.
+1 is the runtime value of constructor One, so it is always matched as One, never as W. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 29e3ee9e98
ℹ️ 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".
29e3ee9 to
719fd62
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
719fd62 to
8ded3c5
Compare
8ded3c5 to
9d1eebb
Compare
9d1eebb to
c49c11e
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
c49c11e to
da12cf0
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
0e23a49 to
7bd5a25
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
7bd5a25 to
b2254f9
Compare
b2254f9 to
61f03be
Compare
|
@codex review |
61f03be to
c8c663e
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c8c663ecc6
ℹ️ 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".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f12e8e79f1
ℹ️ 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".
A payload pattern whose values are all literal-case values can never match (warning 11/12), and a literal case covers the equal bool payload value in the exhaustiveness check. Fixes #8806. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo Signed-off-by: Christoph Knittel <[email protected]>
The check for untagged payload patterns shadowed by literal cases ran only
when warning 11 was active, so with `-w -11+12` or `@warning("-11")` a dead
or-pattern alternative got no warning 12. It now runs when either warning is
active; the usefulness analysis is still gated on warning 11.
Example (rescript):
@unboxed type t = | @as("a") A | S(string)
@warning("-11")
let g = x => switch x { | S("a") => 1 | S("a" | "c") => 2 | S(_) | A => 3 }
Diagnostic (text):
Before: no warnings
After: Warning 12 on the "a" in S("a" | "c"); the dead S("a") case stays
silent because warning 11 is off.
Area: typechecker tests
Output: unchanged
Formats: none
PPX: unchanged
Editor: unchanged
Tests: ~tests/build_tests/super_errors/fixtures/warning_12_untagged_literal_shadows_or_pattern.res
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo
Signed-off-by: Christoph Knittel <[email protected]>
Rule 7 says which payload patterns a literal case shadows, which warnings report them, and how a literal case counts in the exhaustiveness check. Co-Authored-By: Claude Opus 5.5 <[email protected]> Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t Signed-off-by: Christoph Knittel <[email protected]>
Why:
A bigint payload pattern equal to a bigint literal case, such as
`BI(2n)` beside `@as(2n) Two`, got no warning although it can never
match, and a match on a bigint constant was never folded: both treated
bigints as never comparable, from before literals were compared by their
normalized values.
Bigint pattern constants and bigint constants have their literal, so the
shadowed-payload warnings and constant folding compare them as `===` does:
`02n` is `2n`.
Diagnostic added (shadowed bigint payload):
Example:
@unboxed type t = | @as(2n) Two | BI(bigint) | S(string)
let f = (v: t) =>
switch v {
| BI(02n) => "dead"
| BI(_) => "bigint"
| _ => "other"
}
Before (diagnostic): none
After (diagnostic):
this match case is unused.
2n is the runtime value of constructor Two, so it is always matched as Two, never as BI.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <[email protected]>
Why:
A polymorphic variant payload equal to a literal case and the
alternatives of an or-pattern named with `as` got no warning, though
they can never match.
A polymorphic variant without payload stands for its name, or the number
its name spells, as in `Lambda.const_polyvar`, and an alias is looked
through when collecting or-pattern alternatives.
Diagnostic added (a shadowed polymorphic variant payload):
Example:
@unboxed type t = | @as("Foo") F | P([#Foo | #Bar])
let f = x => switch x { | F => 1 | P(#Foo) => 2 | P(_) => 3 }
Before (diagnostic): none
After (diagnostic):
Warning number 11
this match case is unused.
"Foo" is the runtime value of constructor F, so it is always matched as F, never as P.
Diagnostic added (an aliased or-pattern):
Example:
@unboxed type t = | @as("a") A | S(string)
let f = x => switch x { | S(("a" | "b") as s) => s | S(_) | A => "other" }
Before (diagnostic): none
After (diagnostic):
Warning number 12
this sub-pattern is unused.
"a" is the runtime value of constructor A, so it is always matched as A, never as S.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <[email protected]>
Why:
The exhaustiveness check let a bool literal case cover the equal value
of any boolean-kind payload, including a `@typeOf("boolean")` type that
is not `bool`. The synthesized `true` pattern did not fit that type, so
a match missing the payload was taken as complete.
Only a payload whose type expands to `bool` gets the synthesized pattern.
Diagnostic added (a @typeof("boolean") payload):
Example:
@typeof("boolean") type jsBool
@unboxed type t = | @as(true) Yes | Value(jsBool)
let f = (v: t) => switch v { | Yes => 1 }
Before (diagnostic): none
After (diagnostic):
Warning number 8
You forgot to handle a possible case here, for example:
| Value(_)
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <[email protected]>
Why:
Below a second untagged constructor, an or-pattern was checked against
the inner variant's literal cases only, although the outer variant's
literal cases are tested first on the same value.
The literal cases that take precedence are carried down through untagged
payloads, outermost first, each with the payload constructor it shadows.
Diagnostic added (an or-pattern below a nested untagged payload):
Example:
@unboxed type inner = | @as("x") X | N(int) | S(string)
@unboxed type outer = | @as(1) One | W(inner)
let f = v => switch v { | One => "one" | W(N(1 | 2)) => "small" | W(_) => "other" }
Before (diagnostic): none
After (diagnostic):
Warning number 12
this sub-pattern is unused.
1 is the runtime value of constructor One, so it is always matched as One, never as W.
Co-Authored-By: Claude Opus 5.5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <[email protected]>
f12e8e7 to
8cb50ef
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Delightful! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
Fixes #8806.
Stacked on #9017. Rebased from master onto the untagged variant stack: the literal comparison is now
Variant_runtime.same_literal_value(#8965), and a constructor's literal comes resolved from the layout (#9017). The last commit describes the warnings in rule 7 ofcompiler/ml/UntaggedVariants.md.An untagged variant tests its literal cases first, so in
@as("a") A | S(string)the value"a"always matchesA. The pattern checker treatedAandS("a")as unrelated.Dead payload patterns. A payload pattern whose values are all the runtime values of literal cases is now reported, whatever the arm order: warning 11 for a whole case, warning 12 for an or-pattern alternative. The message names the literal case:
Values are compared as at runtime, reusing
Variant_runtime.same_literal_value: a constructor without@asis its name,1equals1.0, ints are 32-bit, and nested untagged constants count. Bigints and the predefinedunit/list/optionconstructors are left out, so there are no false positives there.Bool exhaustiveness. For the exhaustiveness check only, a bool literal case also covers the equal bool payload, so
Yes | Ans(false)is exhaustive for@as(true) Yes | Ans(bool): no warning 8, and no unreachableMatch_failurethrow. Other payload types are infinite, so the overlap is not added for them.Warnings 11 and 12 now carry an optional explanation, following
Partial_match.Tests. New
super_errorsfixtures cover each case (string, constructor name, int vs float, nested, tuple, multiple reasons, or-pattern, bool).unboxed_variant_fold_test.resgets an exhaustive bool match, which only compiles under-warn-error Awithout the old warning 8. Its existinganswerNamekeeps its deliberately dead arm under@warning("-11").Not addressed: the switch compiler still emits dead branches for such arms (e.g. a second
x === "a"test).🤖 Generated with Claude Code
https://claude.ai/code/session_01Kr4cjoUi2f5VpMvE6wT9bo