Skip to content

Commit 24e4412

Browse files
cknittclaude
andcommitted
Test primitive untagged cases before class checks
Why: A `switch` on an untagged variant ran its class checks before the `typeof` switch, so strings and numbers reached `v instanceof C` too. A class missing where the code runs, such as a user class or a newer `Intl` class, then threw for every value, even for the primitive cases. When a variant has class cases, the arms of primitive kinds (string, number, bigint, boolean, symbol) are now tested first, in an outer `switch (typeof v)` whose default holds the class checks and the remaining `object`/`function` arms. A class instance is never a primitive, so no value changes arm. Primitive and other arms no longer share a body through fall-through, since they sit in different switches. Variants with only an array case keep the old form, since `Array.isArray` cannot throw. JS modified (a class case beside primitive cases): Example: @instanceof("Nope") type nope @unboxed type t = N(nope) | S(string) | F(float) | A(array<int>) Before (js): if (v instanceof Nope) { return "N"; } if (Array.isArray(v)) { return "A"; } switch (typeof v) { case "string" : return v; case "number" : return "F"; } After (js): switch (typeof v) { case "string" : return v; case "number" : return "F"; default: if (v instanceof Nope) { return "N"; } if (Array.isArray(v)) { return "A"; } } Review: What does it cost? A second `switch (typeof v)` when there are object or function arms too, and a body shared by a primitive and an object arm is emitted twice (`awaitUser` in `UntaggedVariants.mjs`). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF Signed-off-by: Christoph Knittel <ck@cca.io>
1 parent 5f2e4de commit 24e4412

8 files changed

Lines changed: 387 additions & 219 deletions
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Test the primitive cases of an untagged variant before its class checks, so that strings and numbers skip `instanceof` and a class missing at run time no longer throws for them.

‎compiler/core/lam_compile.ml‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -853,10 +853,35 @@ let compile output_prefix =
853853
| _ -> false
854854
in
855855
let clause_is_not_typeof (tag, _) = tag_is_not_typeof tag in
856+
let tag_is_primitive = function
857+
| Variant_runtime.Payload_shape
858+
( IntType | FloatType | StringType | BigintType | BooleanType
859+
| SymbolType ) ->
860+
true
861+
| _ -> false
862+
in
863+
(* Only class checks can throw (when the class is missing), so arrays,
864+
recognized by [Array.isArray], keep the shorter form *)
865+
let has_class_case =
866+
List.exists
867+
(function
868+
| Variant_runtime.InstanceType (Class _) -> true
869+
| _ -> false)
870+
block_cases
871+
in
856872
let switch ?default ?declaration e clauses =
857873
let not_typeof_clauses, typeof_clauses =
858874
List.partition clause_is_not_typeof clauses
859875
in
876+
(* A class instance is never a primitive, so the primitive kinds can be
877+
tested first: their values then skip the instance checks, which
878+
throw when a class is missing at run time *)
879+
let primitive_clauses, typeof_clauses =
880+
if (not has_class_case) || not_typeof_clauses = [] then
881+
([], typeof_clauses)
882+
else
883+
List.partition (fun (tag, _) -> tag_is_primitive tag) typeof_clauses
884+
in
860885
let has_object_typeof =
861886
List.exists
862887
(function
@@ -903,7 +928,12 @@ let compile output_prefix =
903928
~else_:[build_if_chain rest]
904929
| _ -> (
905930
let typeof_switch () =
906-
S.string_switch ?default ?declaration (E.typeof e) typeof_clauses
931+
match (primitive_clauses, typeof_clauses) with
932+
| _ :: _, [] -> S.block (Option.value default ~default:[])
933+
| _ :: _, _ :: _ ->
934+
S.string_switch ?default (E.typeof e) typeof_clauses
935+
| [], _ ->
936+
S.string_switch ?default ?declaration (E.typeof e) typeof_clauses
907937
in
908938
let guard =
909939
match (needs_null_guard, needs_array_guard) with
@@ -918,11 +948,18 @@ let compile output_prefix =
918948
| Some guard, None -> S.if_ (E.not guard) [typeof_switch ()]
919949
| None, _ -> typeof_switch ())
920950
in
921-
build_if_chain not_typeof_clauses
951+
match primitive_clauses with
952+
| [] -> build_if_chain not_typeof_clauses
953+
| _ ->
954+
S.string_switch ?declaration (E.typeof e) primitive_clauses
955+
~default:[build_if_chain not_typeof_clauses]
922956
in
923957
let merge_cases tag1 tag2 =
924-
(* only merge typeof cases, as instanceof cases are pulled out into if-then-else *)
925-
not (tag_is_not_typeof tag1 || tag_is_not_typeof tag2)
958+
(* only merge typeof cases, as instanceof cases are pulled out into
959+
if-then-else. With instance cases, the primitive kinds are tested in
960+
a switch of their own, so they share bodies only among themselves *)
961+
(not (tag_is_not_typeof tag1 || tag_is_not_typeof tag2))
962+
&& ((not has_class_case) || tag_is_primitive tag1 = tag_is_primitive tag2)
926963
in
927964
cases
928965
|> compile_general_cases ~make_exp:E.tag_type ~eq_exp:mk_eq ~cxt ~switch

‎compiler/ml/UntaggedVariants.md‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -274,7 +274,19 @@ The payload arms compile in one of two forms.
274274
followed by `!(x instanceof C)` for every class case.
275275
- **Three or more arms**: an `if` chain of the instance checks, in the
276276
declaration order of the cases, then one `switch (typeof x)` over the other
277-
arms.
277+
arms. When a variant has class cases (not only arrays), the arms of
278+
primitive kinds (string, number, bigint, boolean, symbol) come first, in an
279+
outer `switch (typeof x)` whose default holds the rest; a class instance is
280+
never a primitive, so this changes no outcome:
281+
282+
```js
283+
switch (typeof x) {
284+
case "string": ...
285+
default:
286+
if (x instanceof Date) { ... }
287+
else switch (typeof x) { case "object": ... }
288+
}
289+
```
278290

279291
In both forms a value of an instance case never lands in the object or
280292
function case. A class check is followed by `!(x instanceof D)` for every
@@ -340,8 +352,9 @@ Recognition trusts the type. A value that belongs to no case is not rejected:
340352
A value from another JavaScript realm fails every `instanceof` check and is
341353
recognized the same way (by the semantics of `instanceof`; not tested):
342354
recognition is local to a realm. A class that does not exist where the
343-
`switch` runs makes its check throw, for every value: the instance checks
344-
precede the `typeof` checks, so primitives reach them too.
355+
`switch` runs makes its check throw for every value that reaches it: in the
356+
`switch` form, objects, functions and primitives outside every case; in the
357+
two-arm form, every value when the class arm is declared first.
345358

346359
- Implementation: `lam_compile.ml:compile_general_cases` (a complete match
347360
emits no check for its last clause).
@@ -547,6 +560,5 @@ had to answer, and the answers of rule 5:
547560
9. **Globals.** A shadowed class is read through `globalThis`. A missing
548561
class throws when the check runs (rule 10).
549562

550-
Still open: a class imported from a module cannot be named; the `typeof`
551-
checks could precede the instance checks, so that a missing class affects
552-
only objects; and the typed arrays share one type (#8955).
563+
Still open: a class imported from a module cannot be named, and the typed
564+
arrays share one type (#8955).

0 commit comments

Comments
 (0)