Skip to content

fix(compiler2): widen empty-collection branches in control-flow joins (B-236)#3781

Open
codeshaunted wants to merge 2 commits into
canaryfrom
avery/b-236-join-fix
Open

fix(compiler2): widen empty-collection branches in control-flow joins (B-236)#3781
codeshaunted wants to merge 2 commits into
canaryfrom
avery/b-236-join-fix

Conversation

@codeshaunted

@codeshaunted codeshaunted commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #3779. That PR fixed several adjacent soundness holes found while investigating B-236, but the actual reported repro still crashed — this PR fixes the real root cause.

The bug

function f(xs: string[], m: int) -> string {
  let top = if (m > 0) { xs.slice(0, m) } else { [] };
  top.join(" ")   // VM internal error: expected map, got array
}

The if/else joined string[] (then) with the empty-array never[] (else) to the union string[] | never[]. A list-union lowers to a non-List runtime type, so method dispatch resolves array methods (.join) to the map implementation — and the VM aborts when handed an array.

The fix

join_types now recurses into matching container constructors instead of building a union:

  • List + List → List<join(elem)>
  • Map + Map → Map<join(key), join(value)>

An empty [] / {} has never element/key/value types, which join away, so if c { xs } else { [] } widens to string[] (the issue's own suggested fix). Two list branches of differing element types likewise widen to (a | b)[] rather than a[] | b[].

Notes

  • The std library itself hit this pattern (OrchestrationStep[] | never[]OrchestrationStep[]) — a latent bug this also fixes. TIR + corpus-bytecode snapshots updated; both diffs are exactly T[] | never[]T[].
  • Regression test added (test-first): the repro now returns "a b".
  • Full workspace suite green (5891), baml_src corpus green, fmt + clippy clean.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed type handling for conditional expressions that combine lists with empty else branches, preventing runtime errors when using containers in control flow operations.

… (B-236)

The actual B-236 repro: `let top = if c { xs } else { [] }; top.join(" ")` joined
`string[]` with the empty-array `never[]` to the union `string[] | never[]`.
Method dispatch on a list-union resolves array methods (`.join`) to the map
implementation, so the VM aborts at runtime with `expected map, got array`.

`join_types` now recurses into matching container constructors — `List + List ->
List<join(elem)>`, `Map + Map -> Map<join(k), join(v)>` — so two list (or two map)
branches widen element-wise to a single container instead of a union. An empty
`[]` / `{}` has `never` element/key/value types, which join away, so
`if c { xs } else { [] }` yields `string[]`.

The std library itself hit this (`OrchestrationStep[] | never[]` -> `OrchestrationStep[]`);
TIR and corpus-bytecode snapshots updated accordingly. Adds the runtime repro as a
regression test.
@linear

linear Bot commented Jun 16, 2026

Copy link
Copy Markdown

B-236

@vercel

vercel Bot commented Jun 16, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
beps Ready Ready Preview, Comment Jun 17, 2026 12:16am
promptfiddle Ready Ready Preview, Comment Jun 17, 2026 12:16am
promptfiddle2 Ready Ready Preview, Comment Jun 17, 2026 12:16am

Request Review

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: d8e44f76-3221-4c8c-8cee-733cf6eee6e5

📥 Commits

Reviewing files that changed from the base of the PR and between 8921e4c and fac79f4.

⛔ Files ignored due to path filters (2)
  • baml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.snap is excluded by !**/*.snap
  • baml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_tir.snap is excluded by !**/*.snap
📒 Files selected for processing (2)
  • baml_language/crates/baml_compiler2_tir/src/builder.rs
  • baml_language/crates/baml_tests/tests/type_error_repro.rs

📝 Walkthrough

Walkthrough

join_types in the TIR builder gains an early match block that detects same-kind container pairs (List/EvolvingList or Map/EvolvingMap) and returns the container with recursively joined inner types instead of producing a union. A regression test for B-236 is added to verify the fix.

Changes

Container type join fix (B-236)

Layer / File(s) Summary
join_types container special-case and regression test
baml_language/crates/baml_compiler2_tir/src/builder.rs, baml_language/crates/baml_tests/tests/type_error_repro.rs
Adds an early match in join_types that handles matching List/EvolvingList and Map/EvolvingMap pairs by recursively joining their element/key/value types, preventing degradation to Ty::Union. The regression test if_else_concrete_list_with_empty_else_branch_runs exercises an if/else that combines string[] with [] and asserts .join(" ") returns "a b".

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

A rabbit hopped left, a rabbit hopped right,
Two lists in a branch—which type is right?
No union of emptiness shall spoil the stew,
We join them together, both through and through!
🐇 string[] | [] → just string[], hooray!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(compiler2): widen empty-collection branches in control-flow joins (B-236)' accurately describes the main change: fixing a runtime crash by widening empty collection branches in control-flow type joins, with specific reference to the issue number.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch avery/b-236-join-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

github-actions Bot commented Jun 16, 2026

Copy link
Copy Markdown

⏭️ Performance benchmarks were skipped

Perf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to canary/main.

To run them on this PR, do any of the following, then push a commit (or re-run CI):

  • Add RUN_CODSPEED=1 to the PR description, or
  • Include run-perf or /perf in the PR title or any commit message.

@github-actions

Copy link
Copy Markdown

No description provided.

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.

1 participant