Repository navigation
Keep cases handled by a default arm out of the arms a switch names - #9010
Merged
Merged
Conversation
Why:
A `switch` on an untagged variant with a `_` arm let the cases it does
not name reach the arms it names: a `Date` matched the object arm, since
only arrays were guarded, and an `@as("lit")` literal matched the string
arm, since the literal test was skipped when the switch names no
literal (#8995).
The `switch (typeof x)` is preceded by a guard sending every instance case
without an arm to the default, as it already did for arrays. The literal test
is skipped only when no literal of the variant would pass the check of a
payload arm. Rules 7 and 9 of the spec state both.
JS modified (an instance handled by the default):
Example:
@unboxed type t = D(Date.t) | R(RegExp.t) | O({x: int}) | S(string) | I(int) | F(int => int)
let f = v => switch v { | O(_) => "O" | S(_) => "S" | _ => "other" }
Before (js):
switch (typeof v) {
case "object" :
return "O";
After (js):
if (v instanceof RegExp || v instanceof Date) {
return "other";
}
switch (typeof v) {
case "object" :
return "O";
JS modified (a literal handled by the default):
Example:
@unboxed type t = | @as("lit") Lit | S(string) | I(int) | B(bool) | O({x: int}) | F(int => int)
let f = v => switch v { | S(_) => "S" | O(_) => "O" | _ => "other" }
Before (js):
switch (typeof v) {
case "string" :
return "S";
After (js):
if (v === "lit") {
return "other";
}
switch (typeof v) {
case "string" :
return "S";
Fixes: #8995
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <ck@cca.io>
Changelog
Semantic Diff1 commit · Full page Keep cases handled by a default arm out of the arms a switch namesb35f485 · cknitt + Claude · Oct 24, 2026, 06:17 UTC
The JS modified (an instance handled by the default) @unboxed type t = D(Date.t) | R(RegExp.t) | O({x: int}) | S(string) | I(int) | F(int => int)
let f = v => switch v { | O(_) => "O" | S(_) => "S" | _ => "other" }
+if (v instanceof RegExp || v instanceof Date) {
+ return "other";
+}
switch (typeof v) {
case "object" :
return "O";JS modified (a literal handled by the default) @unboxed type t = | @as("lit") Lit | S(string) | I(int) | B(bool) | O({x: int}) | F(int => int)
let f = v => switch v { | S(_) => "S" | O(_) => "O" | _ => "other" }
+if (v === "lit") {
+ return "other";
+}
switch (typeof v) {
case "string" :
return "S"; |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #9010 +/- ##
=======================================
Coverage 81.29% 81.29%
=======================================
Files 456 456
Lines 63836 63852 +16
=======================================
+ Hits 51893 51908 +15
- Misses 11943 11944 +1
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
cknitt
marked this pull request as ready for review
October 24, 2026 06:41
Member
Author
|
@codex review |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-9010 |
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. |
|
Codex Review: Didn't find any major issues. What shall we delve into next? 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". |
cristianoc
approved these changes
Oct 24, 2026
cknitt
added a commit
that referenced
this pull request
Oct 24, 2026
Why:
Two forms still ran class checks on primitives: the two-arm `if`, with
the class arm declared first, and the literal test, in its form of
negated payload checks. And the guard that sends instance cases without
an arm to the default (#9010) knew only object instances, while a
declared class can have callable instances, and its class checks ran
before the primitive arms.
Of a class arm and an arm of another kind, the two-arm form checks the
other one. A variant with class cases tests its literals by comparison. The
guard for instance cases without an arm also protects a function arm from
class instances, and when the guard has class checks, the primitive arms are
tested first. The spec's rule 10 example of an unchecked branch changes
accordingly: with `PPromise(promise<int>) | PFloat(float)`, the float case
is checked, so a thenable that is not a `Promise` lands in `PPromise`.
JS modified (an instance handled by the default):
Example:
@unboxed type t = D(Date.t) | R(RegExp.t) | O({x: int}) | S(string) | I(int) | F(int => int)
let f = v => switch v { | O(_) => "O" | S(_) => "S" | _ => "other" }
Before (js):
if (v instanceof RegExp || v instanceof Date) {
return "other";
}
switch (typeof v) {
case "object" :
return "O";
case "string" :
return "S";
After (js):
switch (typeof v) {
case "string" :
return "S";
default:
if (v instanceof RegExp || v instanceof Date) {
return "other";
}
switch (typeof v) {
case "object" :
return "O";
JS modified (two arms):
Example:
@unboxed type t = W(weird) | S(string)
Before (js): if (v instanceof Weird) { return "W"; } else { return "S"; }
After (js): if (typeof v === "string") { return "S"; } else { return "W"; }
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <ck@cca.io>
cknitt
added a commit
that referenced
this pull request
Oct 24, 2026
Why:
Two forms still ran class checks on primitives: the two-arm `if`, with
the class arm declared first, and the literal test, in its form of
negated payload checks. And the guard that sends instance cases without
an arm to the default (#9010) knew only object instances, while a
declared class can have callable instances, and its class checks ran
before the primitive arms.
Of a class arm and an arm of another kind, the two-arm form checks the
other one. A variant with class cases tests its literals by comparison. The
guard for instance cases without an arm also protects a function arm from
class instances, and when the guard has class checks, the primitive arms are
tested first. The spec's rule 10 example of an unchecked branch changes
accordingly: with `PPromise(promise<int>) | PFloat(float)`, the float case
is checked, so a thenable that is not a `Promise` lands in `PPromise`.
JS modified (an instance handled by the default):
Example:
@unboxed type t = D(Date.t) | R(RegExp.t) | O({x: int}) | S(string) | I(int) | F(int => int)
let f = v => switch v { | O(_) => "O" | S(_) => "S" | _ => "other" }
Before (js):
if (v instanceof RegExp || v instanceof Date) {
return "other";
}
switch (typeof v) {
case "object" :
return "O";
case "string" :
return "S";
After (js):
switch (typeof v) {
case "string" :
return "S";
default:
if (v instanceof RegExp || v instanceof Date) {
return "other";
}
switch (typeof v) {
case "object" :
return "O";
JS modified (two arms):
Example:
@unboxed type t = W(weird) | S(string)
Before (js): if (v instanceof Weird) { return "W"; } else { return "S"; }
After (js): if (typeof v === "string") { return "S"; } else { return "W"; }
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t
Signed-off-by: Christoph Knittel <ck@cca.io>
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.
Fixes #8995.
A
switchon an untagged variant with a_arm let the cases it does not name reach the arms it names. It shows when the default survives into the untagged dispatch, which takes a variant with several cases:switcharmsD(Date.t) | R(RegExp.t) | O({x: int}) | S(string) | I(int) | F(int => int)O,S,_D(Date.make())O_@as("lit") Lit | S(string) | I(int) | B(bool) | O({x: int}) | F(int => int)S,O,_LitS_Two fixes in
lam_compile.ml:switch (typeof x)form is preceded by a guard sending every instance case without an arm to the default, generalizing the existing guard for arrays:if (v instanceof RegExp || v instanceof Date) { return "other"; }.compile_switchskipped the literal test whenever the switch names no literal case and literals and payloads share the default. It now does so only when no literal would pass the check of a payload arm of its kind, so"lit"is tested first.UntaggedVariants.md: rules 7 and 9 state both.New test:
tests/tests/src/untagged_default_arm_test.res. No other generated output changes.This fix is also part of #8967 in the
@instanceOfstack, adapted there to declared classes. That PR will be rebased onto this one.🤖 Generated with Claude Code
https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t