Repository navigation
Conversation
Changelog💅 Polish
Semantic Diff8 commits · Full page Match untagged variants with a literal switch and one typeof dispatch87b11c5 · cknitt + Claude · Oct 24, 2026, 10:17 UTC
The literals come first, compared with JS modified (primitives before classes) @unboxed
type t = Missing(missing) | Text(string) | Count(float)
let kind = value =>
switch value {
| Missing(_) => "missing"
| Text(text) => text
| Count(_) => "count"
}
-if (value instanceof UntaggedMissingClass) {
- return "missing";
-}
switch (typeof value) {
case "string" :
return value;
case "number" :
return "count";
+ default:
+ return "missing";
}JS modified (instances without an arm) @unboxed
type t = D(Date.t) | R(RegExp.t) | O({x: int}) | S(string) | I(int) | F(int => int)
let kind = value =>
switch value {
| O(_) => "O"
| S(_) => "S"
| _ => "other"
}
-if (value instanceof RegExp || value instanceof Date) {
+if (typeof value === "string") {
+ return "S";
+} else if (typeof value === "object" && !(value instanceof RegExp) && !(value instanceof Date)) {
+ return "O";
+} else {
return "other";
}
-switch (typeof value) {
- case "object" :
- return "O";
- case "string" :
- return "S";
- default:
- return "other";
-}JS modified (a subclass first) @unboxed
type upload = Blob(Blob.t) | File(File.t)
let kind = upload =>
switch upload {
| Blob(_) => "blob"
| File(_) => "file"
}
-if (upload instanceof Blob && !(upload instanceof File)) {
- return "blob";
-} else {
+if (upload instanceof File) {
return "file";
+} else {
+ return "blob";
}Review
Drop literal arms that a complete payload dispatch runs anyway7b9e47c · cknitt + Claude · Oct 24, 2026, 10:25 UTC A complete payload dispatch with one body runs it unchecked, so a literal arm with the same body needs no case of its own. JS modified (literal arm equal to the payload arm) @unboxed
type rec t = | @as(null) Null | @as(undefined) Undefined | WhyNotAnotherOne | Point({x: int, y: int})
let plus = (x, y) =>
switch y {
| Null | Undefined => x
| WhyNotAnotherOne => WhyNotAnotherOne
| Point(_) => WhyNotAnotherOne
}
-switch (y) {
- case null :
- case undefined :
- return x;
- case "WhyNotAnotherOne" :
- return "WhyNotAnotherOne";
- default:
- return "WhyNotAnotherOne";
+if (y == null) {
+ return x;
+} else {
+ return "WhyNotAnotherOne";
}Update the gentype example for the untagged dispatch2d336e3 · cknitt + Claude · Oct 24, 2026, 10:37 UTC The gentype example matches an untagged variant too; its output follows the new dispatch. Say precisely when literals use an if and what foldsd6e1a16 · cknitt + Claude · Oct 24, 2026, 16:18 UTC Rule 7 counts the literals that get a case, as the code does, and rule 11 names the values constant folding sees: literals and blocks of constants. Emit untagged checks directly instead of through Dynamic_checksdc51f4a · cknitt + Claude · Oct 24, 2026, 16:21 UTC The check AST in Ast_untagged_variants only encoded the int-tag test of tagged variants and the instance checks. Js_exp_make now builds both directly (is_int_tag from the literal list, is_instance_of), and matching_facts no longer carries has_null, has_undefined and has_other_literal, which that test derived. A class is never a switch key, so block_type_name no longer pretends to emit one. No output changes. Share which literal passes which payload check6f36150 · cknitt + Claude · Oct 24, 2026, 16:23 UTC Code generation and constant folding each had a table of the literals a payload case accepts, and they disagreed on null and an object case. Variant_runtime.literal_passes is now the one table; folding follows the generated code, where ✨ Keep the checks an empty arm or a tagged block needs8a038ee · cknitt + Claude · Oct 25, 2026, 05:59 UTC · new
Whether there are other arms comes from the dispatch entries. A literal arm joins the payload dispatch only when at most one tagged block is matched, and the last tag of a complete match is not checked, as the last arm of every other complete match. JS modified (an arm that does nothing) @unboxed type t = S(string) | O({x: int})
let f = v => {
switch v {
| S(_) => Console.log("s")
| O(_) => ()
}
Console.log("after")
}
-switch (typeof v) {
- default:
- console.log("s");
+if (typeof v === "string") {
+ console.log("s");
}
console.log("after");JS modified (a literal sharing the body of tagged blocks) type t = | @unboxed MS(string) | MB({x: int}) | MC(int) | @as(null) MNull | @as(1) MOne
let f = v =>
switch v {
| MB(_) => "other"
| MC(_) => "other"
| MOne => "one"
| MNull | MS(_) => "other"
}
+if (v === null) {
+ return "other";
+}
if (v === 1) {
return "one";
}
if (typeof v === "string") {
return "other";
}
switch (v.TAG) {
case "MB" :
- case "MC" :
+ default:
return "other";
}✨ Drop the tagged blocks of mixed variants from the dispatchcaf4899 · cknitt + Claude · Oct 25, 2026, 13:09 UTC · new Since #9040 a variant is untagged as a whole or not at all, so the payload dispatch of an untagged variant never meets a tagged block. The |
|
@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. |
|
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". |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## instanceof-typeof-annotations #9015 +/- ##
=================================================================
- Coverage 81.33% 81.32% -0.02%
=================================================================
Files 456 456
Lines 64179 64181 +2
=================================================================
- Hits 52198 52192 -6
- Misses 11981 11989 +8
🚀 New features to boost your workflow:
|
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
85c7fd6 to
77e7004
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep them coming! 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". |
77e7004 to
66880b8
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66880b8db8
ℹ️ 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".
Why:
The payload dispatch of an untagged variant ran its instance checks
before the `typeof` checks, so strings and numbers reached `instanceof`:
a class missing at run time threw for them, and a `Symbol.hasInstance`
could claim them. Literals were tested by a size heuristic that chose
between two equivalent expressions, the object case by a guard per
instance case, and a subclass by negating it in its superclass's check.
The literals come first, compared with `===` by one `switch (x)` (an `if`
per body for at most two), whose default is the payload dispatch: one
`switch (typeof x)` over the primitive kinds, whose default is an `if` chain
over the instance cases (a subclass first), the function case, the tagged
constructors and the object case. In a complete match the last arm is not
checked. A literal without an arm skips the `switch` when no payload check
would accept it, and a `null` that only the object check would accept adds
`x !== null` to it. Checks leading to the default before the last checked
arm fold into its check, so class checks run only on objects.
`Dynamic_checks.is_a_literal_case` and the per-instance guards are gone.
JS modified (primitives before classes):
Example:
@unboxed
type t = Missing(missing) | Text(string) | Count(float)
let kind = value =>
switch value {
| Missing(_) => "missing"
| Text(text) => text
| Count(_) => "count"
}
Before (js):
if (value instanceof UntaggedMissingClass) {
return "missing";
}
switch (typeof value) {
case "string" :
return value;
case "number" :
return "count";
}
After (js):
switch (typeof value) {
case "string" :
return value;
case "number" :
return "count";
default:
return "missing";
}
JS modified (instances without an arm):
Example:
@unboxed
type t = D(Date.t) | R(RegExp.t) | O({x: int}) | S(string) | I(int) | F(int => int)
let kind = value =>
switch value {
| O(_) => "O"
| S(_) => "S"
| _ => "other"
}
Before (js):
if (value instanceof RegExp || value instanceof Date) {
return "other";
}
switch (typeof value) {
case "object" :
return "O";
case "string" :
return "S";
default:
return "other";
}
After (js):
if (typeof value === "string") {
return "S";
} else if (typeof value === "object" && !(value instanceof RegExp) && !(value instanceof Date)) {
return "O";
} else {
return "other";
}
JS modified (a subclass first):
Example:
@unboxed
type upload = Blob(Blob.t) | File(File.t)
let kind = upload =>
switch upload {
| Blob(_) => "blob"
| File(_) => "file"
}
Before (js):
if (upload instanceof Blob && !(upload instanceof File)) {
return "blob";
} else {
return "file";
}
After (js):
if (upload instanceof File) {
return "file";
} else {
return "blob";
}
Review: Does `break` in an arm leave the generated `switch` instead of the
loop? No: every arm placed in a generated `switch` is compiled in a switch
context, where `break` and `continue` are labeled;
`untagged_loop_control_test.res` checks arms of every part.
Review: What changes for a value outside every case? It lands in the last
arm of the chain: with `PPromise | PFloat` the float case is now checked, so
a thenable lands in `PPromise` (rule 10).
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>
A complete payload dispatch with one body runs it unchecked, so a literal arm
with the same body needs no case of its own.
JS modified (literal arm equal to the payload arm):
Example:
@unboxed
type rec t = | @as(null) Null | @as(undefined) Undefined | WhyNotAnotherOne | Point({x: int, y: int})
let plus = (x, y) =>
switch y {
| Null | Undefined => x
| WhyNotAnotherOne => WhyNotAnotherOne
| Point(_) => WhyNotAnotherOne
}
Before (js):
switch (y) {
case null :
case undefined :
return x;
case "WhyNotAnotherOne" :
return "WhyNotAnotherOne";
default:
return "WhyNotAnotherOne";
}
After (js):
if (y == null) {
return x;
} else {
return "WhyNotAnotherOne";
}
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>
The gentype example matches an untagged variant too; its output follows the new dispatch. 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>
Rule 7 counts the literals that get a case, as the code does, and rule 11 names the values constant folding sees: literals and blocks of constants. 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>
The check AST in Ast_untagged_variants only encoded the int-tag test of tagged variants and the instance checks. Js_exp_make now builds both directly (is_int_tag from the literal list, is_instance_of), and matching_facts no longer carries has_null, has_undefined and has_other_literal, which that test derived. A class is never a switch key, so block_type_name no longer pretends to emit one. No output changes. 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>
Code generation and constant folding each had a table of the literals a payload case accepts, and they disagreed on null and an object case. Variant_runtime.literal_passes is now the one table; folding follows the generated code, where `typeof null` is "object", so null without a null case folds into the object case as it is matched at run time. 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>
Why:
The payload dispatch decided that a match had no arm besides its
primitive ones from the compiled chain of the others, which is empty when
their bodies do nothing, and then ran the last primitive arm unchecked.
A literal arm sharing the body of the payload dispatch was dropped,
although two tagged blocks are told apart by reading their tag, which a
literal such as `null` does not have.
Whether there are other arms comes from the dispatch entries. A literal arm
joins the payload dispatch only when at most one tagged block is matched, and
the last tag of a complete match is not checked, as the last arm of every
other complete match.
JS modified (an arm that does nothing):
Example:
@unboxed type t = S(string) | O({x: int})
let f = v => {
switch v {
| S(_) => Console.log("s")
| O(_) => ()
}
Console.log("after")
}
Before (js):
switch (typeof v) {
default:
console.log("s");
}
console.log("after");
After (js):
if (typeof v === "string") {
console.log("s");
}
console.log("after");
JS modified (a literal sharing the body of tagged blocks):
Example:
type t = | @unboxed MS(string) | MB({x: int}) | MC(int) | @as(null) MNull | @as(1) MOne
let f = v =>
switch v {
| MB(_) => "other"
| MC(_) => "other"
| MOne => "one"
| MNull | MS(_) => "other"
}
Before (js):
if (v === 1) {
return "one";
}
if (typeof v === "string") {
return "other";
}
switch (v.TAG) {
case "MB" :
case "MC" :
return "other";
}
After (js):
if (v === null) {
return "other";
}
if (v === 1) {
return "one";
}
if (typeof v === "string") {
return "other";
}
switch (v.TAG) {
case "MB" :
default:
return "other";
}
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>
Since #9040 a variant is untagged as a whole or not at all, so the payload dispatch of an untagged variant never meets a tagged block. The `switch` on the tag inside its `if` chain, the merging of tagged clauses, the count of tagged blocks before dropping literal arms and the tag-name parameter are gone, and rule 8 no longer lists tagged constructors. 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>
66880b8 to
caf4899
Compare
Stacked on #8950. Replaces #8967 with the dispatch structure the architecture review proposed.
An untagged variant now matches in two steps:
===, by oneswitch (x), or by anifper body for at most two literals. Its default is the payload dispatch.switch (typeof x)over the primitive kinds, or aniffor one kind. Its default is anifchain, in this order: the instance cases (a subclass before its superclasses, otherwise declaration order), the function case and the object case. Since Make @unboxed apply to a whole variant only #9040 a variant is untagged as a whole, so no tagged block appears here.In a complete match the last arm is never checked, whichever form it is in.
What this removes:
Dynamic_checks.is_a_literal_case, and with it the rest ofAst_untagged_variants.Dynamic_checksandJs_exp_make.emit_check:Js_exp_makebuilds the int-tag test of tagged variants and the instance checks directly, andmatching_factsdropshas_null,has_undefinedandhas_other_literal;null/array/instance guards in front of thetypeofswitch;!(x instanceof Sub)negations in superclass checks.Code generation and constant folding share one table of which literal passes which payload check (
Variant_runtime.literal_passes). The two copies disagreed onnulland an object case; folding now follows the generated code, wheretypeof nullis"object".What follows from the new structure:
instanceof. A class missing at run time therefore throws only for objects and functions, andSymbol.hasInstancecan't claim strings.switchwhen no payload check would accept it.nullthat only the object check would accept becomes&& x !== nullon that check. JSON decoders keep their singletypeof x === "object" && x !== null && !Array.isArray(x).typeof x === "object" && !(x instanceof Date). Class checks then run only on objects.Spec rules 7–10 in
compiler/ml/UntaggedVariants.mddescribe the new structure, and rule 11 names the values constant folding sees. Rule 10 changes one outcome: withPPromise | PFloat, the float case is now checked, so a thenable lands inPPromise.New tests:
untagged_loop_control_test.res:breakandcontinuein literal, primitive, class, object and default arms leave or continue the loop, not the generatedswitch.untagged_default_arm_test.res:Symbol.hasInstanceaccepts everything doesn't take primitives or literals (carried over from Test primitive untagged cases before class checks #8967);untagged_runtime_annotations_test.res: a class missing at run time doesn't affect primitive cases, with every arm present or with a default (partly from Test primitive untagged cases before class checks #8967). Also a default shared by primitive and object cases.ounit_lambda_constant_tests.ml:nullwithout a null case folds into the object case.🤖 Generated with Claude Code
https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t