Fix matching on lists and maps by element type#3933
Merged
Conversation
This prevents drift and allows zero-cost conversions with `RealizedTy`. TODO: `Self` is currently not properly mapped into TemplateTy
We now emit type-aware code for matching on lists and maps: - If there is only one map type/list type possible, then we match on type tag (any value must have the expected element type) - Otherwise if there are multiple then we have to emit code to check the element types
This hardens the preexisting behavior so it either works or gives a compiler error instead of unexpected runtime failures. At a later date ([B-687](https://linear.app/boundaryml2/issue/B-687/support-self-everywhere-like-rust)) we should expand support beyond the current narrow usage.
`BoundMethod` should carry its type args. Previously we extracted class generic args off the receiver, but this is insufficient as it silently dropped the method's generic args. We now produce the full type arg frame when the bound method is created, similar to `GenericFunction` (the function-as-value object) TODO: actually use the type args
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
⏭️ Performance benchmarks were skippedPerf benchmarks (CodSpeed) are opt-in on pull requests — they no longer run on every push. They always run automatically after merge to To run them on this PR, do any of the following, then push a commit (or re-run CI):
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (8)
📒 Files selected for processing (10)
🚧 Files skipped from review as they are similar to previous changes (10)
📝 WalkthroughWalkthroughThis PR separates template-only type handling from realized/runtime types, rewrites MIR/emit/VM template matching around ChangesTyTemplate/RealizedTy Runtime Type Refactor
Self-in-Body-Position Diagnostic
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
baml_language/crates/baml_compiler2_mir/src/lower.rs (1)
15553-15600: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPreserve associated-type projections in the dispatch-guard template
Lowering
AssociatedTypeProjectiontoWildcardlets patterns likeFoo<T.Assoc>orIface<Err = T.CompareError>match unrelated values. Keep the projection leaf intact here, or fail closed until projection-aware matching exists.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/baml_compiler2_mir/src/lower.rs` around lines 15553 - 15600, The dispatch-guard lowering in tir2_to_dispatch_guard_template currently drops AssociatedTypeProjection by turning it into Wildcard, which makes patterns like Foo<T.Assoc> or Iface<Err = T.CompareError> over-match unrelated values. Update the template-building path used by emit_is_type_template_branch so AssociatedTypeProjection stays preserved as a real leaf, or otherwise fail closed in this branch until projection-aware matching is implemented; keep the change localized around tir2_to_dispatch_guard_template and the contains_typevar gating in lower.rs.baml_language/crates/bex_vm/src/package_baml/resolve.rs (1)
401-420: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winRequire interface associated bindings to match both ways.
Lines 415-420 only validate bindings that already exist on the concrete side. That lets a symbolic pattern like
Pair<Iterable<Item=T>, T>matchPair<Iterable, int>onceTis bound by the second slot, even though the interface argument never provesItem = int.baml_type::Interface::to_tyexplicitly allows partial associated-type bindings, so the matcher cannot assume the two binding sets coincide.Proposed fix
TyTemplate::Interface(name, args, assoc, _) => match concrete { RuntimeTy::Interface(cname, cargs, cassoc, _) => { - // Each *concrete* binding must match a same-named pattern binding, found - // order-insensitively; extra pattern bindings don't constrain. This - // direction mirrors the compiler's selection matcher - // (`match_ty_pattern_into`'s `Interface` arm, which iterates the concrete - // bindings and requires each in the pattern) so runtime dispatch never - // selects an impl compile-time selection would reject. A positional, - // length-locked `zip` would instead diverge if the two declaration orders - // differed. (A top-level interface for-type is rejected by - // `is_valid_impl_subject`; this is only reached for a nested interface - // argument, where the binding sets coincide in well-formed code.) + // Match associated bindings by name, but require the same binding set on + // both sides. Partial interface existentials are valid runtime types. name == cname && all_match(args, cargs, bindings) - && cassoc.iter().all(|(cn, ct)| { - assoc - .iter() - .find(|(an, _)| an == cn) - .is_some_and(|(_, at)| match_template(at, ct, bindings)) - }) + && cassoc.len() == assoc.len() + && assoc.iter().all(|(an, at)| { + cassoc + .iter() + .find(|(cn, _)| cn == an) + .is_some_and(|(_, ct)| match_template(at, ct, bindings)) + }) } _ => false, },🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@baml_language/crates/bex_vm/src/package_baml/resolve.rs` around lines 401 - 420, The TyTemplate::Interface matcher only checks associated bindings from the concrete RuntimeTy::Interface side, so a pattern can match without proving the pattern’s own associated bindings are satisfied. Update match_template in the TyTemplate::Interface arm to validate associated bindings in both directions: keep the existing concrete-to-pattern check and also ensure every associated binding in the pattern exists on the concrete side and matches via match_template, using the same binding lookup logic as all_match. This should be applied alongside the existing name == cname and all_match(args, cargs, bindings) checks so interface matching is symmetric for associated types.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 1320-1369: The new tag-sufficiency gate in
switch_member_tag_sufficient still treats RuntimeTy::Enum and
RuntimeTy::EnumVariant as always safe, but type_tag_for_ty() maps them to the
shared ENUM tag so different enum values can still collide. Update
switch_member_tag_sufficient to fail closed for enum-related members, causing
the switch lowering path to fall back to the precise chain until a
per-enum/per-variant sufficiency proof exists. Use the existing
RuntimeTy::Class, RuntimeTy::List, and RuntimeTy::Map handling as the pattern
for where to branch, but make enum cases return false rather than true.
In `@baml_language/crates/bex_vm/src/package_baml/resolve.rs`:
- Around line 440-446: The symbolic function-matching path in resolve.rs is too
permissive compared with ty_equivalent because it ignores parameter names while
comparing params in the fast path. Update the matching logic in the affected arm
to also require the parameter names to match, alongside mode and type, so
symbolic signatures behave consistently with the realized path used by
ty_equivalent. Use the existing param comparison in the function
pattern-matching code as the place to align this behavior.
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 15553-15600: The dispatch-guard lowering in
tir2_to_dispatch_guard_template currently drops AssociatedTypeProjection by
turning it into Wildcard, which makes patterns like Foo<T.Assoc> or Iface<Err =
T.CompareError> over-match unrelated values. Update the template-building path
used by emit_is_type_template_branch so AssociatedTypeProjection stays preserved
as a real leaf, or otherwise fail closed in this branch until projection-aware
matching is implemented; keep the change localized around
tir2_to_dispatch_guard_template and the contains_typevar gating in lower.rs.
In `@baml_language/crates/bex_vm/src/package_baml/resolve.rs`:
- Around line 401-420: The TyTemplate::Interface matcher only checks associated
bindings from the concrete RuntimeTy::Interface side, so a pattern can match
without proving the pattern’s own associated bindings are satisfied. Update
match_template in the TyTemplate::Interface arm to validate associated bindings
in both directions: keep the existing concrete-to-pattern check and also ensure
every associated binding in the pattern exists on the concrete side and matches
via match_template, using the same binding lookup logic as all_match. This
should be applied alongside the existing name == cname and all_match(args,
cargs, bindings) checks so interface matching is symmetric for associated types.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: ccbbf8e4-e7ff-44d6-94eb-24a677fcfc40
⛔ Files ignored due to path filters (29)
baml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/csv.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/generic_match_subtype.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/generic_match_typevar.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/inferred_generic_type_args.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/iter.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/iter_impl_generics_only.core.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_alias.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_parse_stringify.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_to_from_string.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_container_types.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_self_workaround.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/patterns_new_runtime.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/toml.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiler2_mir/baml_tests__compiler2_mir__match_or_mixed_array_class_binding_uses_branch_local_rest_type.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_alias_basic/baml_tests__compiles__json_alias_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_alias_basic/baml_tests__compiles__json_alias_basic__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/llm_image_outputs/baml_tests__compiles__llm_image_outputs__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/llm_image_outputs/baml_tests__compiles__llm_image_outputs__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/patterns_new/baml_tests__compiles__patterns_new__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/self_in_body/baml_tests__diagnostic_errors__self_in_body__01_lexer__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/self_in_body/baml_tests__diagnostic_errors__self_in_body__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/self_in_body/baml_tests__diagnostic_errors__self_in_body__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/self_in_body/baml_tests__diagnostic_errors__self_in_body__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/self_in_body/baml_tests__diagnostic_errors__self_in_body__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/self_in_body/baml_tests__diagnostic_errors__self_in_body__10_formatter__main.snapis excluded by!**/*.snap
📒 Files selected for processing (38)
baml_language/crates/baml_base/src/attr.rsbaml_language/crates/baml_compiler2_emit/src/analysis.rsbaml_language/crates/baml_compiler2_emit/src/emit.rsbaml_language/crates/baml_compiler2_emit/src/lib.rsbaml_language/crates/baml_compiler2_emit/src/pull_semantics.rsbaml_language/crates/baml_compiler2_emit/src/stack_carry.rsbaml_language/crates/baml_compiler2_mir/src/ir.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/infer_context.rsbaml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_tests/baml_src/ns_generic_match_subtype/generic_match_subtype.bamlbaml_language/crates/baml_tests/baml_src/ns_generic_match_typevar/generic_match_typevar.bamlbaml_language/crates/baml_tests/baml_src/ns_match_container_types/match_container_types.bamlbaml_language/crates/baml_tests/baml_src/ns_match_self_workaround/match_self_workaround.bamlbaml_language/crates/baml_tests/baml_src/ns_toml/toml.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/self_in_body/main.bamlbaml_language/crates/baml_type/src/family.rsbaml_language/crates/baml_type/src/lib.rsbaml_language/crates/baml_type/src/realized_ty.rsbaml_language/crates/baml_type/src/template.rsbaml_language/crates/baml_type_macros/src/emit.rsbaml_language/crates/baml_type_macros/src/parse.rsbaml_language/crates/bex_engine/src/conversion.rsbaml_language/crates/bex_heap/src/gc.rsbaml_language/crates/bex_heap/src/tlab.rsbaml_language/crates/bex_vm/src/lib.rsbaml_language/crates/bex_vm/src/package_baml/mod.rsbaml_language/crates/bex_vm/src/package_baml/resolve.rsbaml_language/crates/bex_vm/src/type_context.rsbaml_language/crates/bex_vm/src/type_match.rsbaml_language/crates/bex_vm/src/vm.rsbaml_language/crates/bex_vm/tests/interface_registry.rsbaml_language/crates/bex_vm/tests/load_type.rsbaml_language/crates/bex_vm/tests/method_class_type_args.rsbaml_language/crates/bex_vm_types/src/types/class.rsbaml_language/crates/bex_vm_types/src/types/function.rs
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_type/src/lib.rs
👮 Files not reviewed due to content moderation or server errors (4)
- baml_language/crates/baml_compiler2_tir/src/infer_context.rs
- baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs
- baml_language/crates/baml_compiler2_tir/src/builder.rs
- baml_language/crates/baml_lsp2_actions/src/check.rs
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 1354-1366: The `RuntimeTy::Enum` branch in `lower.rs` is too
permissive because `flat.iter().all(...)` currently treats `None` from
`enum_type_name` as safe; update this logic to fail closed for opaque members,
matching the container branch behavior. In the
`flatten_runtime_union`/`enum_type_name` check, reject any member that does not
resolve to the same enum name as the `name` being matched so `TypeTag` safety
does not rely on shared `ENUM` tags across different enums.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 5f4c9891-b669-4e55-9894-03140f842b02
⛔ Files ignored due to path filters (8)
baml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/json_alias.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/match_enum_types.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/toml.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/__baml_std__/baml_tests__compiles____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_alias_basic/baml_tests__compiles__json_alias_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/json_alias_basic/baml_tests__compiles__json_alias_basic__06_codegen.snapis excluded by!**/*.snap
📒 Files selected for processing (10)
baml_language/crates/baml_compiler2_emit/src/emit.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsbaml_language/crates/baml_tests/baml_src/ns_match_enum_types/match_enum_types.bamlbaml_language/crates/baml_tests/baml_src/ns_toml/toml.bamlbaml_language/crates/baml_type/src/family.rsbaml_language/crates/baml_type/src/template.rsbaml_language/crates/bex_vm/src/package_baml/resolve.rsbaml_language/crates/bex_vm/src/type_match.rsbaml_language/crates/bex_vm/src/vm.rs
🚧 Files skipped from review as they are similar to previous changes (8)
- baml_language/crates/baml_compiler2_tir/src/lower_type_expr.rs
- baml_language/crates/baml_tests/baml_src/ns_toml/toml.baml
- baml_language/crates/bex_vm/src/type_match.rs
- baml_language/crates/bex_vm/src/package_baml/resolve.rs
- baml_language/crates/baml_compiler2_emit/src/emit.rs
- baml_language/crates/baml_type/src/family.rs
- baml_language/crates/bex_vm/src/vm.rs
- baml_language/crates/baml_type/src/template.rs
2kai2kai2
enabled auto-merge
July 7, 2026 22:10
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.
TyTemplateis now generated/convertible via the type family macroSelfwas already rejected in many positions (i.e. most non-interface-related positions) we now give a nice compiler error for it. At a later date, we should allowSelfin more places.Object::BoundMethodnow correctly holds type args (both from the bound receiver and the function's generics). This ensures we can actually get the type off the value.Summary by CodeRabbit
New Features
match/istype discrimination across containers (lists/maps), unions, classes, enums, and generic type arguments.Bug Fixes
Selfusage in method body type positions; it now reports clearer compile errors instead of falling back.Tests
match/isdiscrimination andSelfdiagnostics.