Skip to content

Decide the untagged dispatch's literal cases by routing them - #9047

Merged
cknitt merged 2 commits into
dispatch-plan-foldfrom
dispatch-plan-outputs
Oct 26, 2026
Merged

cknitt merged 2 commits into
dispatch-plan-foldfrom
dispatch-plan-outputs

Conversation

@cknitt

@cknitt cknitt commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

Stacked on #9046.

With the dispatch as a plan, two decisions can use the facts they were missing.

Literal cases by routing. A literal without an arm used to get a case whenever the variant had a class case, even where no class check runs on it. Now it skips the literal switch exactly when the payload dispatch, evaluated on it with select, takes it to the default without a class check. A class check is avoided because it throws when the class does not exist at run time. This replaces has_class_case and reaches_a_payload_arm, and Variant_runtime.literal_passes is gone.

Simplification. A test whose branches run the same body, and a case that runs what its default runs, are left out. Arms of different kinds with one body thus become one unchecked branch. The literal arm left to "a complete payload dispatch with one body" now requires the payload plan to really be one unchecked body. Before, that held only because matching usually shares such bodies: the property test found plans where a literal reached an instanceof.

Generated code changes:

  • if (x === "a") return 3; is no longer emitted ahead of a switch (typeof x) that already sends "a" to return 3.
  • if (typeof x === "string") { log; return } log; becomes log;.
  • In UntaggedVariants.res, preserveDiscriminatorGroups merges three equal loop bodies.
  • In a complete dispatch on primitive kinds, the cases falling through to the last arm, which is the default, are left out: Num(_) => 1 | Text(_) => 2 | Flag(_) => 2 becomes if (typeof x === "number") return 1; else return 2;, and a literal with that body loses its case.

New tests:

  • untagged_variants_heuristic.res adds dateOrObject, logPointOrName, numberOrText and letterOrNumber.
  • The property test also checks that no literal or primitive value reaches a class check.

Spec rule 7 now states the routing rule and the simplification.

Deliberately not included: dropping the case of a literal that has its own arm when the payload dispatch would run the same body anyway; it would save little and add a rule.

🤖 Generated with Claude Code

https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t

@cknitt
cknitt added this pull request to stack #9048 October 25, 2026 20:38
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Changelog

💅 Polish

  • Leave out checks of an untagged variant switch whose outcome runs the same code, and test a literal without an arm with === only when the payload dispatch would run a class check on it.

Semantic Diff

2 commits · Full page

Decide the untagged dispatch's literal cases by routing them

446dce1 · cknitt + Claude · Oct 25, 2026, 20:37 UTC

Why

A literal without an arm got a case whenever the variant had a class case, even where no class check runs on it, and arms of different kinds with one body kept the checks telling them apart. The literal arm left to a complete payload dispatch with one body assumed that dispatch runs no check, which held only because matching shares such bodies.

A literal without an arm skips the literal switch when the payload dispatch, evaluated on it with select, takes it to the default without a class check. The plan is simplified: a test whose branches run the same, and a case that runs what its default runs, are left out, and a literal with the body of a payload dispatch that runs one body unchecked gets no case. The property test also checks that no literal or primitive reaches a class check. Variant_runtime.literal_passes is gone.

Generated code changed (a literal without an arm beside a class case):

@unboxed type t = | @as("a") A | S(string) | N(float) | D(Date.t) | O({x: int})
let f = (x: t) => switch x { | D(_) => 1 | O(_) => 2 | _ => 3 }

Before (JS):

if (x === "a") {
  return 3;
}
switch (typeof x) {
  case "string" :
  case "number" :
    return 3;
  default:
    ...
}

After (JS):

switch (typeof x) {
  case "string" :
  case "number" :
    return 3;
  default:
    ...
}

Generated code changed (arms of different kinds with one body):

@unboxed type t = | @as(null) NoPoint | Point({x: int}) | Name(string)
switch x {
| NoPoint => Console.log("null")
| Point(_) => Console.log("payload")
| Name(_) => Console.log("payload")
}

Before (JS):

if (x === null) {
  console.log("null");
  return;
}
if (typeof x === "string") {
  console.log("payload");
  return;
}
console.log("payload");

After (JS):

if (x === null) {
  console.log("null");
  return;
}
console.log("payload");
✨ Leave the cases falling through to the default of a complete typeof switch

7085291 · cknitt + Claude · Oct 26, 2026, 06:11 UTC · new

Why

In a complete dispatch on primitive kinds only, the last arm becomes the default, but the cases of other kinds sharing its body stayed in the switch, falling through into default:. That also kept the dispatch from being one unchecked body, so a literal with that body kept its case.

The cases falling through to the last arm are left out with it.

Generated code changed (kinds sharing the last arm's body):

@unboxed type t = Num(int) | Text(string) | Flag(bool)
let f = (x: t) => switch x { | Num(_) => 1 | Text(_) => 2 | Flag(_) => 2 }

Before (JS):

switch (typeof x) {
  case "number" :
    return 1;
  case "string" :
  default:
    return 2;
}

After (JS):

if (typeof x === "number") {
  return 1;
} else {
  return 2;
}

@codecov

codecov Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 74.54545% with 28 lines in your changes missing coverage. Please review.
⚠️ Please upload report for BASE (dispatch-plan-fold@3cf4066). Learn more about missing BASE report.

Files with missing lines Patch % Lines
compiler/ml/untagged_dispatch.ml 75.24% 25 Missing ⚠️
tests/ounit_tests/ounit_untagged_dispatch_tests.ml 66.66% 3 Missing ⚠️
Additional details and impacted files
@@                  Coverage Diff                  @@
##             dispatch-plan-fold    #9047   +/-   ##
=====================================================
  Coverage                      ?   81.37%           
=====================================================
  Files                         ?      458           
  Lines                         ?    64499           
  Branches                      ?        0           
=====================================================
  Hits                          ?    52485           
  Misses                        ?    12014           
  Partials                      ?        0           
Files with missing lines Coverage Δ
compiler/ml/variant_runtime.ml 93.10% <ø> (ø)
tests/ounit_tests/ounit_untagged_dispatch_tests.ml 94.40% <66.66%> (ø)
compiler/ml/untagged_dispatch.ml 86.81% <75.24%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@pkg-pr-new

pkg-pr-new Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

rescript

npm i https://pkg.pr.new/rescript@9047

@rescript/belt

npm i https://pkg.pr.new/@rescript/belt@9047

@rescript/darwin-arm64

npm i https://pkg.pr.new/@rescript/darwin-arm64@9047

@rescript/linux-arm64

npm i https://pkg.pr.new/@rescript/linux-arm64@9047

@rescript/linux-x64

npm i https://pkg.pr.new/@rescript/linux-x64@9047

@rescript/runtime

npm i https://pkg.pr.new/@rescript/runtime@9047

@rescript/win32-x64

npm i https://pkg.pr.new/@rescript/win32-x64@9047

commit: 7085291

@cknitt
cknitt marked this pull request as ready for review October 25, 2026 21:27
@cknitt

cknitt commented Oct 25, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-26T06:35:16.848047Z 7085291 Manual request
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@cknitt

cknitt commented Oct 26, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Hooray!

Reviewed commit: 83f154356c

ℹ️ 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".

@cknitt
cknitt requested a review from cristianoc October 26, 2026 06:02
cknitt and others added 2 commits October 26, 2026 06:09
Why:
    A literal without an arm got a case whenever the variant had a class
    case, even where no class check runs on it, and arms of different kinds
    with one body kept the checks telling them apart. The literal arm left
    to a complete payload dispatch with one body assumed that dispatch runs
    no check, which held only because matching shares such bodies.

A literal without an arm skips the literal switch when the payload dispatch,
evaluated on it with `select`, takes it to the default without a class check.
The plan is simplified: a test whose branches run the same, and a case that
runs what its default runs, are left out, and a literal with the body of a
payload dispatch that runs one body unchecked gets no case. The property test
also checks that no literal or primitive reaches a class check.
`Variant_runtime.literal_passes` is gone.

Generated code changed (a literal without an arm beside a class case):
Example:
    @unboxed type t = | @as("a") A | S(string) | N(float) | D(Date.t) | O({x: int})
    let f = (x: t) => switch x { | D(_) => 1 | O(_) => 2 | _ => 3 }
Before (JS):
    if (x === "a") {
      return 3;
    }
    switch (typeof x) {
      case "string" :
      case "number" :
        return 3;
      default:
        ...
    }
After (JS):
    switch (typeof x) {
      case "string" :
      case "number" :
        return 3;
      default:
        ...
    }

Generated code changed (arms of different kinds with one body):
Example:
    @unboxed type t = | @as(null) NoPoint | Point({x: int}) | Name(string)
    switch x {
    | NoPoint => Console.log("null")
    | Point(_) => Console.log("payload")
    | Name(_) => Console.log("payload")
    }
Before (JS):
    if (x === null) {
      console.log("null");
      return;
    }
    if (typeof x === "string") {
      console.log("payload");
      return;
    }
    console.log("payload");
After (JS):
    if (x === null) {
      console.log("null");
      return;
    }
    console.log("payload");

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>
…witch

Why:
    In a complete dispatch on primitive kinds only, the last arm becomes the
    default, but the cases of other kinds sharing its body stayed in the
    switch, falling through into `default:`. That also kept the dispatch
    from being one unchecked body, so a literal with that body kept its case.

The cases falling through to the last arm are left out with it.

Generated code changed (kinds sharing the last arm's body):
Example:
    @unboxed type t = Num(int) | Text(string) | Flag(bool)
    let f = (x: t) => switch x { | Num(_) => 1 | Text(_) => 2 | Flag(_) => 2 }
Before (JS):
    switch (typeof x) {
      case "number" :
        return 1;
      case "string" :
      default:
        return 2;
    }
After (JS):
    if (typeof x === "number") {
      return 1;
    } else {
      return 2;
    }

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
cknitt force-pushed the dispatch-plan-outputs branch from 83f1543 to 7085291 Compare October 26, 2026 06:16
@cknitt

cknitt commented Oct 26, 2026

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 708529147d

ℹ️ 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".

@cknitt
cknitt merged commit 27b5460 into master Oct 26, 2026
27 of 42 checks passed
@cknitt
cknitt deleted the dispatch-plan-outputs branch October 26, 2026 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants