Fix type checker literal union upcasting issues#3299
Merged
Conversation
- Extend infer_arithmetic's base_ty to handle Ty::Union by folding over
members with a promote helper, fixing InvalidBinaryOp for union-of-literal
arithmetic (e.g. `1 + if(true){2} else {3}`)
- Make widen_fresh() recursive into Union, List, Map, Optional so container
literals at unannotated let-bindings widen correctly (e.g. `[1,2,3]` infers
as int[] instead of (1|2|3)[])
- Fix propagate_copies in MIR cleanup to guard against inlining phi-like
temps that have multiple definitions
- Add subtype unit tests for union-of-literals scenarios
- Un-ignore 6 tests across soundness, if_else, and match_basics
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
📝 WalkthroughWalkthroughAdded local-definition counting to MIR cleanup to restrict copy/constant propagation; extended TIR arithmetic inference to handle unions and promote numeric literals; normalized/deduplicated unions during widening; added literal-union tests and BAML examples; updated many test snapshots and LSP2 action expectations. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
Binary size checks passed✅ 7 passed
Generated by |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ec5f0226-428c-41a1-a440-eab77f29ec4b
⛔ Files ignored due to path filters (29)
baml_language/crates/baml_tests/snapshots/catch_throw/baml_tests__catch_throw__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_throw/baml_tests__catch_throw__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/format_checks/baml_tests__format_checks__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_call/baml_tests__function_call__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_call/baml_tests__function_call__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/headers_edge_cases/baml_tests__headers_edge_cases__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/headers_edge_cases/baml_tests__headers_edge_cases__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__01_lexer__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_arithmetic/baml_tests__literal_union_arithmetic__10_formatter__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__01_lexer__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/literal_union_widening/baml_tests__literal_union_widening__10_formatter__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/parser_statements/baml_tests__parser_statements__05_diagnostics.snapis excluded by!**/*.snap
📒 Files selected for processing (9)
baml_language/crates/baml_compiler2_mir/src/cleanup.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/normalize.rsbaml_language/crates/baml_compiler2_tir/src/ty.rsbaml_language/crates/baml_tests/projects/literal_union_arithmetic/main.bamlbaml_language/crates/baml_tests/projects/literal_union_widening/main.bamlbaml_language/crates/baml_tests/tests/if_else.rsbaml_language/crates/baml_tests/tests/match_basics.rsbaml_language/crates/baml_tests/tests/soundness.rs
…r defs Document why widen_fresh drops _attr on Union (always default in TIR layer) and why count_local_defs omits DispatchFuture/Await (lowering never reuses locals across Assign and async terminators). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
baml_language/crates/baml_compiler2_mir/src/cleanup.rs (1)
304-311: 🛠️ Refactor suggestion | 🟠 MajorMake the sync-vs-async def invariant executable.
The new
defs[dest] == 1gate is only sound becausecount_local_defs()intentionally ignoresAwait/DispatchFuturewrites. Right now that contract lives only in comments, so a future lowering tweak can silently re-open the same phi-like miscompile on mixed sync/async branches. Please add a debug assertion/verifier check for “no local is written by bothAssignand async terminators”, or count those terminators here.Also applies to: 506-510, 523-526
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 1bc131f2-19c9-43d5-b41b-cb1f8177b4ed
📒 Files selected for processing (2)
baml_language/crates/baml_compiler2_mir/src/cleanup.rsbaml_language/crates/baml_compiler2_tir/src/ty.rs
Literal unions like `1 | 2 | 3` are now widened to their primitive type (e.g. `int`), so error messages and diagnostics reflect the widened types. Spurious operator errors on literal unions (Add, Mul) are now resolved. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…thread TyAttr - cleanup.rs: Use Place::base_local() instead of hand-rolled projection walk. Count DispatchFuture/Await destinations as definition sites. - ty.rs: Thread the union's TyAttr through dedup_and_collapse instead of dropping it and rebuilding with TyAttr::default(). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 55433123-bfcc-4d78-b97f-8702f48c44d0
📒 Files selected for processing (2)
baml_language/crates/baml_compiler2_mir/src/cleanup.rsbaml_language/crates/baml_compiler2_tir/src/ty.rs
hellovai
deleted the
hellovai/fix-type-checker-literal-union-upcasting-issues
branch
codeshaunted
pushed a commit
that referenced
this pull request
Mar 30, 2026
[Artifacts](https://cloud.codelayer.cloud/tasks/019d3357-d681-74d6-8d5a-02e6fe7c4038/artifacts) | [Task](https://cloud.codelayer.cloud/deep/tasks/019d3357-d681-74d6-8d5a-02e6fe7c4038) ## What problems was I solving The BAML type checker had two categories of failures around literal union types: 1. **Arithmetic on unions of literals failed**: Expressions like `1 + if (true) { 2 } else { 3 }` produced `InvalidBinaryOp` errors because `infer_arithmetic`'s `base_ty` closure only handled `Ty::Primitive` and `Ty::Literal` — `Ty::Union` fell through to `None`. This blocked six tests that were marked `#[ignore]`. 2. **Container inference didn't widen fresh literals**: `[1, 2, 3]` at an unannotated `let`-binding inferred as `(1 | 2 | 3)[]` instead of `int[]`, because `widen_fresh()` only matched top-level `Ty::Literal` and didn't recurse into compound types (`Union`, `List`, `Map`, `Optional`). After this PR, union-of-literal arithmetic works correctly, container literals widen as expected, and all six previously-ignored tests pass. ## What user-facing changes did I ship - [cleanup.rs](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_compiler2_mir/src/cleanup.rs) — MIR constant propagation guard for multi-definition locals (phi-like temps) - [builder.rs](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_compiler2_tir/src/builder.rs) — Extended `infer_arithmetic` to handle `Ty::Union` with recursive `base_ty` + `promote` helper - [ty.rs](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_compiler2_tir/src/ty.rs) — Recursive `widen_fresh()` for compound types + `dedup_and_collapse` helper - [normalize.rs](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_compiler2_tir/src/normalize.rs) — New subtype unit tests for union-of-literals scenarios - [literal_union_arithmetic/main.baml](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_tests/projects/literal_union_arithmetic/main.baml) — New test project for union-of-literal arithmetic - [literal_union_widening/main.baml](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_tests/projects/literal_union_widening/main.baml) — New test project for recursive widening - [if_else.rs](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_tests/tests/if_else.rs) — Un-ignored 3 arithmetic tests - [match_basics.rs](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_tests/tests/match_basics.rs) — Un-ignored 2 arithmetic tests - [soundness.rs](https://github.com/BoundaryML/baml/pull/3299/files#diff-baml_language/crates/baml_tests/tests/soundness.rs) — Un-ignored 1 arithmetic test ## How I implemented it ### TIR: Fix arithmetic on union types (`builder.rs`) Refactored `infer_arithmetic`'s `base_ty` from a closure to a local `fn` and added a `Ty::Union` arm that recursively extracts the base primitive from each union member. Added a `promote` helper that handles same-type identity and `Int`/`Float` promotion. This means `1 + if (true) { 2 } else { 3 }` now correctly resolves both operands to `Int` and produces an `Int` result. ### TIR: Recursive `widen_fresh()` (`ty.rs`) Extended `widen_fresh()` to recurse into `Ty::Union`, `Ty::List`, `Ty::Map`, and `Ty::Optional`, following the recursive transformation pattern from `substitute_ty` in `generics.rs`. Added a `dedup_and_collapse` helper that flattens nested unions, deduplicates members, and collapses singletons. This means `[1, 2, 3]` at a `let`-binding now widens each `Fresh` literal inside the union, deduplicates three `Primitive(Int)` values into one, and produces `int[]`. ### MIR: Guard constant propagation for phi-like temps (`cleanup.rs`) Added `count_local_defs` to count assignment sites per local. The `propagate_copies` pass now skips constant propagation for locals defined in multiple branches (e.g., a temp assigned in both arms of an if-else). This prevents the MIR optimizer from incorrectly collapsing phi-like temporaries — a bug that surfaced when the arithmetic fixes produced correct TIR that the optimizer then mishandled. ### Tests: New projects + un-ignored tests - Created `literal_union_arithmetic/main.baml` with 5 functions exercising union arithmetic (if-else add, match add, both-sides union, subtract, loop match accumulator) - Created `literal_union_widening/main.baml` with 4 functions exercising recursive widening (array of ints, map of ints, nested array, annotated literal union preservation) - Un-ignored all 6 previously-failing tests across `soundness.rs`, `if_else.rs`, and `match_basics.rs` - Added 3 subtype unit tests in `normalize.rs` for `Union([Lit(1), Lit(2)]) <: Int`, `List(Union(...)) <: List(Int)`, and mixed int/float literal union subtyping ## Deviations from the plan ### Implemented as planned - Phase 0: Both test projects created verbatim as specified - Phase 1: `infer_arithmetic` refactored with `base_ty` fn + `promote` helper + `Ty::Union` arm; all 6 tests un-ignored - Phase 2: `widen_fresh()` made recursive with `Union`/`List`/`Map`/`Optional` arms + `dedup_and_collapse` helper - Phase 3: All 3 subtype unit tests added in `normalize.rs` ### Deviations/surprises - `promote` takes `b` by reference (`&PrimitiveType`) instead of by value — minor ergonomic adjustment, semantically identical - `normalize.rs` tests use `HashMap::new()` instead of `FxHashMap::default()` — matches the existing test module imports ### Additions not in plan - **`cleanup.rs` — `count_local_defs` + phi-like temp guard in `propagate_copies`**: The MIR constant propagation pass was incorrectly collapsing phi-like temporaries (locals assigned in multiple if-else branches). This bug was latent but surfaced when the arithmetic fixes produced correct TIR that the optimizer then mishandled. The fix counts definition sites per local and skips propagation for multi-definition locals. - **Snapshot updates for existing test projects** (`catch_throw`, `format_checks`, `function_call`, `headers_edge_cases`, `parser_statements`): Downstream effects of both the `widen_fresh` recursion (literal types in TIR now widen correctly) and the `propagate_copies` fix (MIR local numbering shifts). ### Items planned but not implemented - None — all phases fully implemented ## How to verify it ### Setup ```bash git fetch scripts/create_worktree.sh hellovai/fix-type-checker-literal-union-upcasting-issues cd ~/wt/baml/fix-type-checker-literal-union-upcasting-issues/baml_language ``` ### Manual Testing - [ ] Review `crates/baml_tests/projects/literal_union_arithmetic/main.baml` — confirm the 5 arithmetic test cases cover the intended scenarios - [ ] Review `crates/baml_tests/projects/literal_union_widening/main.baml` — confirm the 4 widening test cases cover arrays, maps, nested arrays, and annotated preservation - [ ] Review the `cleanup.rs` changes — confirm the `count_local_defs` logic correctly counts assignments and call destinations ### Automated Tests ```bash cargo test -p baml_tests cargo test -p baml_compiler2_tir ``` ## Description for the changelog Fix type checker to support arithmetic on unions of literal types and recursive widening of fresh literals in containers. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **New Features** * Arithmetic now handles union-typed operands and promotes numeric combinations; widening of fresh literal types now propagates through unions and container types. * **Bug Fixes** * Copy/constant propagation tightened to avoid unsafe inlining into locals with multiple definitions. * **Tests** * Added literal-union subtyping/widening tests. * Re-enabled and updated bytecode snapshots for control-flow + union-arithmetic cases. * Updated editor/LSP test expectations to relax prior diagnostics. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Artifacts | Task
What problems was I solving
The BAML type checker had two categories of failures around literal union types:
Arithmetic on unions of literals failed: Expressions like
1 + if (true) { 2 } else { 3 }producedInvalidBinaryOperrors becauseinfer_arithmetic'sbase_tyclosure only handledTy::PrimitiveandTy::Literal—Ty::Unionfell through toNone. This blocked six tests that were marked#[ignore].Container inference didn't widen fresh literals:
[1, 2, 3]at an unannotatedlet-binding inferred as(1 | 2 | 3)[]instead ofint[], becausewiden_fresh()only matched top-levelTy::Literaland didn't recurse into compound types (Union,List,Map,Optional).After this PR, union-of-literal arithmetic works correctly, container literals widen as expected, and all six previously-ignored tests pass.
What user-facing changes did I ship
infer_arithmeticto handleTy::Unionwith recursivebase_ty+promotehelperwiden_fresh()for compound types +dedup_and_collapsehelperHow I implemented it
TIR: Fix arithmetic on union types (
builder.rs)Refactored
infer_arithmetic'sbase_tyfrom a closure to a localfnand added aTy::Unionarm that recursively extracts the base primitive from each union member. Added apromotehelper that handles same-type identity andInt/Floatpromotion. This means1 + if (true) { 2 } else { 3 }now correctly resolves both operands toIntand produces anIntresult.TIR: Recursive
widen_fresh()(ty.rs)Extended
widen_fresh()to recurse intoTy::Union,Ty::List,Ty::Map, andTy::Optional, following the recursive transformation pattern fromsubstitute_tyingenerics.rs. Added adedup_and_collapsehelper that flattens nested unions, deduplicates members, and collapses singletons. This means[1, 2, 3]at alet-binding now widens eachFreshliteral inside the union, deduplicates threePrimitive(Int)values into one, and producesint[].MIR: Guard constant propagation for phi-like temps (
cleanup.rs)Added
count_local_defsto count assignment sites per local. Thepropagate_copiespass now skips constant propagation for locals defined in multiple branches (e.g., a temp assigned in both arms of an if-else). This prevents the MIR optimizer from incorrectly collapsing phi-like temporaries — a bug that surfaced when the arithmetic fixes produced correct TIR that the optimizer then mishandled.Tests: New projects + un-ignored tests
literal_union_arithmetic/main.bamlwith 5 functions exercising union arithmetic (if-else add, match add, both-sides union, subtract, loop match accumulator)literal_union_widening/main.bamlwith 4 functions exercising recursive widening (array of ints, map of ints, nested array, annotated literal union preservation)soundness.rs,if_else.rs, andmatch_basics.rsnormalize.rsforUnion([Lit(1), Lit(2)]) <: Int,List(Union(...)) <: List(Int), and mixed int/float literal union subtypingDeviations from the plan
Implemented as planned
infer_arithmeticrefactored withbase_tyfn +promotehelper +Ty::Unionarm; all 6 tests un-ignoredwiden_fresh()made recursive withUnion/List/Map/Optionalarms +dedup_and_collapsehelpernormalize.rsDeviations/surprises
promotetakesbby reference (&PrimitiveType) instead of by value — minor ergonomic adjustment, semantically identicalnormalize.rstests useHashMap::new()instead ofFxHashMap::default()— matches the existing test module importsAdditions not in plan
cleanup.rs—count_local_defs+ phi-like temp guard inpropagate_copies: The MIR constant propagation pass was incorrectly collapsing phi-like temporaries (locals assigned in multiple if-else branches). This bug was latent but surfaced when the arithmetic fixes produced correct TIR that the optimizer then mishandled. The fix counts definition sites per local and skips propagation for multi-definition locals.catch_throw,format_checks,function_call,headers_edge_cases,parser_statements): Downstream effects of both thewiden_freshrecursion (literal types in TIR now widen correctly) and thepropagate_copiesfix (MIR local numbering shifts).Items planned but not implemented
How to verify it
Setup
Manual Testing
crates/baml_tests/projects/literal_union_arithmetic/main.baml— confirm the 5 arithmetic test cases cover the intended scenarioscrates/baml_tests/projects/literal_union_widening/main.baml— confirm the 4 widening test cases cover arrays, maps, nested arrays, and annotated preservationcleanup.rschanges — confirm thecount_local_defslogic correctly counts assignments and call destinationsAutomated Tests
Description for the changelog
Fix type checker to support arithmetic on unions of literal types and recursive widening of fresh literals in containers.
Summary by CodeRabbit
New Features
Bug Fixes
Tests