Repository navigation
Honor subtyping in generic class match arms' reified type-arg check (B-634) - #3902
Conversation
…B-634)
A generic `match` arm `let s: Opt<T>` (where `T` is the enclosing function's
type var) lowered its runtime type-test to an EXACT reified type-arg comparison.
When inference pins `T` to a supertype union of the scrutinee's actual type arg
-- which happens whenever a `default: T` argument subtypes, so `T` reifies to
the un-subsumed join `Shape | Sq` while the value is a plain `Opt<Shape>` -- the
exact `Shape | Sq == Shape` check failed and the arm silently missed, falling
through to the default. Concretely, `unwrap_or(mk<Shape>(Sq{s:5}), Sq{s:0})
.area()` returned 0 instead of 25; widening the default to `Shape` (so `T`
pins to `Shape`) already returned 25.
Fix (MIR layer): in `lower_pattern_test`, the typevar-carrying class-pattern arm
now builds its `IsType` template with `tir2_to_dispatch_guard_template` (frame
`TypeVar` -> `TypeArgRefOrWildcard`, subtype-or-wildcard) instead of
`ty_to_template` (frame `TypeVar` -> `TypeArgRef`, exact). The VM then compares
the reified frame arg with `is_subtype_of` rather than `==`, which is the
correct covariant "does this value belong to type `Opt<T>`" semantics and
matches the interface class-dispatch guard path (`emit_interface_class_guard_
branch`), which already uses the same template. This keeps `TypeArgRef`'s
exact-materialization meaning intact for other uses.
Directionality is preserved: a strictly wider runtime type-arg still fails to
match a narrower pinned `T`. Concrete parametric arms (`Foo<int>` vs
`Foo<string>`) are unaffected -- they carry no enclosing type var and take the
exact concrete path.
Tests:
- bex_vm unit tests on `guard_template_matches` locking the directional
subtype semantics (under-match fix, exact `TypeArgRef` stays exact, wider
arg does not over-match, unknown-slot wildcard, concrete discrimination).
- ns_generic_match_subtype e2e: narrow-default -> 25, widened-default -> 25,
non-Opt scrutinee falls through, concrete `Foo<int>`/`Foo<string>` still
discriminate.
- projects/compiles/generic_match_typevar_arm locks the emitted template as
`TypeArgRefOrWildcard` (`Opt<#0?>`) in the MIR/codegen snapshots.
|
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):
|
|
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 selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChanges the MIR lowering of generic ChangesGeneric Match Subtype Dispatch Guard
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
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 |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
baml_language/crates/baml_compiler2_mir/src/lower.rs (1)
15188-15221: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winGeneric class unions still bypass the template path
baml_language/crates/baml_compiler2_mir/src/lower.rs:15188-15221only handles top-levelTir2Ty::Class(..). Union-typed patterns likeOpt<T> | Other<T>still fall through to the erased runtime lowering, andtir_ty_needs_interface_shape_testonly coversInterfacemembers. If those patterns are valid here, they need the same dispatch-guard template path.🤖 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 15188 - 15221, The generic class-pattern lowering only routes top-level Tir2Ty::Class patterns with typevars through the dispatch-guard template path, so union patterns like Opt<T> | Other<T> still fall back to erased runtime lowering. Update the lowering logic in lower.rs around the class-pattern branch to detect union-typed patterns containing class members with typevars and send them through tir2_to_dispatch_guard_template and emit_is_type_template_branch as well, similar to the existing Class(..) handling and the interface guard path.
🤖 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.
Outside diff comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 15188-15221: The generic class-pattern lowering only routes
top-level Tir2Ty::Class patterns with typevars through the dispatch-guard
template path, so union patterns like Opt<T> | Other<T> still fall back to
erased runtime lowering. Update the lowering logic in lower.rs around the
class-pattern branch to detect union-typed patterns containing class members
with typevars and send them through tir2_to_dispatch_guard_template and
emit_is_type_template_branch as well, similar to the existing Class(..) handling
and the interface guard path.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 27dde455-9e06-4646-b717-3d1ea6d08df8
⛔ Files ignored due to path filters (10)
baml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/generic_match_subtype.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__01_lexer__generic_match_typevar_arm.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__02_parser__generic_match_typevar_arm.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/generic_match_typevar_arm/baml_tests__compiles__generic_match_typevar_arm__10_formatter__generic_match_typevar_arm.snapis excluded by!**/*.snap
📒 Files selected for processing (4)
baml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_tests/baml_src/ns_generic_match_subtype/generic_match_subtype.bamlbaml_language/crates/baml_tests/projects/compiles/generic_match_typevar_arm/generic_match_typevar_arm.bamlbaml_language/crates/bex_vm/src/vm.rs
Binary size checks passed✅ 7 passed
Generated by |
Bug
A generic
matcharmlet s: Opt<T>(whereTis the enclosing function's type var) lowered its runtime type-test to an exact reified type-arg comparison. When inference pinsTto a supertype union of the scrutinee's actual type arg — which happens whenever adefault: Targument subtypes, soTreifies to the un-subsumed joinShape | Sqwhile the value is a plainOpt<Shape>— the exactShape | Sq == Shapecheck fails, the arm silently misses, and control falls through to the default.Repro
bug()returned 0 (expected 25). Widening the default soTpins toShapedirectly (let d: Shape = Sq{s:0}) already returned 25. Probingreflect.type_of<T>()insideunwrap_orprints"Shape | Sq", confirming the reified frame type-arg is the un-subsumed join, notSq.Root cause
crates/baml_compiler2_mir/src/lower.rs,lower_pattern_test: the typevar-carrying class-pattern branch built theIsTypetemplate viaty_to_template, whoseTypeVararm lowers a frame type var toTyTemplate::TypeArgRef(N)(exact). The VM'sguard_template_matches(crates/bex_vm/src/vm.rs) checksTypeArgRef(n)withframe_type_args[n] == actual— structural equality. Withframe_type_args[0] = Union([Shape, Sq])and the scrutineeOpt<Shape>'sclass_type_args[0] = Shape,Union([Shape, Sq]) == Shapeis false, so the arm never matches.The sibling
TypeArgRefOrWildcard(n)arm instead doesactual.is_subtype_of(expected)— the correct covariant "does this value belong to typeOpt<T>" semantics — and this is exactly what interface class-dispatch guards already use (emit_interface_class_guard_branch, viatir2_to_dispatch_guard_template). Match arms were inconsistently on the exact path.Fix (MIR layer)
Swap the single
ty_to_templatecall at the typevar class-pattern arm fortir2_to_dispatch_guard_template, soOpt<T>lowers toClass("Opt", [TypeArgRefOrWildcard(0)]). The VM then subtype-checksShape <: (Shape | Sq)→ matches → returnss.value→area() == 25.This is preferred over broadening the VM's
TypeArgRefcomparison tois_subtype_of, because it keepsTypeArgRef's exact-materialization meaning intact and makes match/catch class-pattern arms consistent with the already-shipping interface-class-dispatch guard.Directionality / soundness
The subtype check stays directional: a strictly wider runtime type-arg still fails to match a narrower pinned
T((Shape|Sq).is_subtype_of(Sq)is false). Static typing + covariance already guarantee a well-typed scrutinee's runtime arg is<:the frameT, so this cannot over-match well-typed programs. Concrete parametric arms (Foo<int>vsFoo<string>) carry no enclosing type var and take the untouched exact concrete path.Tests
bex_vmunit tests onguard_template_matcheslocking the directional subtype semantics: under-match fix (Shapematches aShape|SqframeT), exactTypeArgRefstays exact, a wider runtime arg does not over-match a narrowerT, an unknown frame slot is a wildcard, and concreteFoo<int>vsFoo<string>still discriminate.ns_generic_match_subtypee2e (real VM): narrow-default → 25, widened-default → 25 (control), non-Optscrutinee falls through to the default, and concreteFoo<int>/Foo<string>still discriminate.projects/compiles/generic_match_typevar_armsnapshot project locks the emitted template asTypeArgRefOrWildcard(Opt<#0?>, wasOpt<#0>) in the MIR/codegen snapshots.Before / after
bug2_narrow(narrow default)025bug2_widened(widened default, control)2525Snapshots
No existing e2e/compiled snapshot shifted (verified across the whole
baml_srcbytecode snapshot: only the new namespace snapshot and the global$init_testregistrations appear). New snapshots: thegeneric_match_typevar_armcompiles project and thegeneric_match_subtypenamespace. The_root.snapchange is just the added test registrations.Local runs green:
cargo nextest run -p bex_vm -p baml_compiler2_mir(68 passed) andcargo test -p baml_tests -- --skip parser_stress(2758 passed, 0 failed).https://linear.app/boundaryml2/issue/B-634
Summary by CodeRabbit
Bug Fixes
matchpattern runtime type checks so subtype-based arms continue to match correctly when type inference widens generic arguments.Tests