Skip to content

Build the untagged dispatch as a plan before emitting it - #9045

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

cknitt merged 2 commits into
masterfrom
dispatch-plan

Conversation

@cknitt

@cknitt cknitt commented Oct 25, 2026 •

Copy link
Copy Markdown
Member

The dispatch of a switch on an untagged variant was decided in two places while printing JavaScript: the literal side in compile_switch, and the payload side in compile_untagged_cases, each with only part of the facts. Constant folding in Lambda.switch had a third copy of the rules.

This PR builds the dispatch as data first. The new module compiler/ml/untagged_dispatch.ml turns the variant's facts and the match's arms into a tree of tests (x === lit, typeof x, instanceof, x !== null, &&, ||, !) and statements (if, switch (x), switch (typeof x)). It makes exactly today's decisions, step by step. Lam_compile.compile_untagged_plan prints the tree, and compile_untagged_cases and the literal closure in compile_switch are gone.

The output is byte-identical across the tests, the stdlib and gentype, with one deliberate exception. A body of an if is no longer compiled in a switch context, so a loop left from such an arm loses a needless label (loop_0: … break loop_0; becomes break;). Bodies are compiled in the order they are printed, so loop labels in arms are numbered top to bottom; in the arms of the literal ifs that reverses master's numbering, which no output in the tests shows.

New tests:

  • ounit_untagged_dispatch_tests.ml pins plans as data, one per dispatch rule.
  • untagged_loop_control_test.res (scanIf) covers the break from an arm of an if.

The next PRs in the stack fold constants by evaluating the plan (#9046), and decide the literal cases and same-body arms with it (#9047).

🤖 Generated with Claude Code

https://claude.ai/code/session_01CMkrYaktctdKc4M41WF36t

Why:
    The dispatch of a switch on an untagged variant was decided in two
    places, the literal side in `compile_switch` and the payload side in
    `compile_untagged_cases`, each with part of the facts, while printing
    JavaScript.

`Untagged_dispatch.plan` builds the dispatch as a tree of tests and
statements from the variant's facts and the match's arms, making today's
decisions in one place; `Lam_compile.compile_untagged_plan` prints it. ounit
tests pin plans as data. The output is unchanged except that a body of an
`if` is no longer compiled in a `switch` context.

Generated code changed (a loop left from an arm of an if):
Example:
    @unboxed type t = TextItem(string) | CountItem(float)
    for i in 0 to Array.length(values) - 1 {
      switch values->Array.getUnsafe(i) {
      | TextItem(text) => seen->Array.push(text)
      | CountItem(_) => break
      }
    }
Before (JS):
    loop_0: for (let i = 0, i_finish = values.length; i < i_finish; ++i) {
      ...
      } else {
        break loop_0;
      }
    }
After (JS):
    for (let i = 0, i_finish = values.length; i < i_finish; ++i) {
      ...
      } else {
        break;
      }
    }

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>
@github-actions

github-actions Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Changelog

🏠 Internal

  • Build the dispatch of a switch on an untagged variant as a plan before printing it.

Semantic Diff

2 commits · Full page

Build the untagged dispatch as a plan before emitting it

fbfe930 · cknitt + Claude · Oct 25, 2026, 20:21 UTC

Why

The dispatch of a switch on an untagged variant was decided in two places, the literal side in compile_switch and the payload side in compile_untagged_cases, each with part of the facts, while printing JavaScript.

Untagged_dispatch.plan builds the dispatch as a tree of tests and statements from the variant's facts and the match's arms, making today's decisions in one place; Lam_compile.compile_untagged_plan prints it. ounit tests pin plans as data. The output is unchanged except that a body of an if is no longer compiled in a switch context.

Generated code changed (a loop left from an arm of an if):

@unboxed type t = TextItem(string) | CountItem(float)
for i in 0 to Array.length(values) - 1 {
  switch values->Array.getUnsafe(i) {
  | TextItem(text) => seen->Array.push(text)
  | CountItem(_) => break
  }
}

Before (JS):

loop_0: for (let i = 0, i_finish = values.length; i < i_finish; ++i) {
  ...
  } else {
    break loop_0;
  }
}

After (JS):

for (let i = 0, i_finish = values.length; i < i_finish; ++i) {
  ...
  } else {
    break;
  }
}
✨ Compile the bodies of an if in the order they are printed

d764e3d · cknitt + Claude · Oct 26, 2026, 06:09 UTC · new

Why

The if chain of the payload dispatch compiled its last arm first, so loop labels in its arms were numbered bottom to top. A dispatch on one typeof kind fell back to a switch for an empty case, with bodies compiled outside a switch context.

An if compiles its then branch before its else branch, so labels are numbered top to bottom, as master numbered them in the class, array and object chain; arms of the literal ifs are now numbered in that order too. A dispatch on one typeof kind is always an if.

Generated code changed (loop labels in literal if arms):

a loop needing a label in each of two literal arms printed as `if`s

Before (JS): the label of the second arm is loop_0, of the first loop_1 After (JS): the label of the first arm is loop_0, of the second loop_1

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

codecov Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.80969% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.36%. Comparing base (1659aee) to head (d764e3d).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
compiler/ml/untagged_dispatch.ml 91.72% 11 Missing ⚠️
compiler/core/lam_compile.ml 96.59% 3 Missing ⚠️
tests/ounit_tests/ounit_untagged_dispatch_tests.ml 98.52% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #9045      +/-   ##
==========================================
+ Coverage   81.33%   81.36%   +0.02%     
==========================================
  Files         456      458       +2     
  Lines       64273    64366      +93     
==========================================
+ Hits        52275    52369      +94     
+ Misses      11998    11997       -1     
Files with missing lines Coverage Δ
tests/ounit_tests/ounit_tests_main.ml 100.00% <ø> (ø)
tests/ounit_tests/ounit_untagged_dispatch_tests.ml 98.52% <98.52%> (ø)
compiler/core/lam_compile.ml 91.63% <96.59%> (+0.45%) ⬆️
compiler/ml/untagged_dispatch.ml 91.72% <91.72%> (ø)
🚀 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@9045

@rescript/belt

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

@rescript/darwin-arm64

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

@rescript/linux-arm64

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

@rescript/linux-x64

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

@rescript/runtime

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

@rescript/win32-x64

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

commit: d764e3d

@github-actions

Copy link
Copy Markdown

@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:34:22.236225Z d764e3d 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.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 👍

Reviewed commit: fbfe93006f

ℹ️ 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
Why:
    The `if` chain of the payload dispatch compiled its last arm first, so
    loop labels in its arms were numbered bottom to top. A dispatch on one
    `typeof` kind fell back to a `switch` for an empty case, with bodies
    compiled outside a `switch` context.

An `if` compiles its `then` branch before its `else` branch, so labels are
numbered top to bottom, as master numbered them in the class, array and
object chain; arms of the literal `if`s are now numbered in that order too.
A dispatch on one `typeof` kind is always an `if`.

Generated code changed (loop labels in literal `if` arms):
Example:
    a loop needing a label in each of two literal arms printed as `if`s
Before (JS): the label of the second arm is `loop_0`, of the first `loop_1`
After (JS): the label of the first arm is `loop_0`, of the second `loop_1`

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 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. Can't wait for the next one!

Reviewed commit: d764e3d835

ℹ️ 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 91c9360 into master Oct 26, 2026
26 checks passed
@cknitt
cknitt deleted the dispatch-plan 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