fix(tir): report type errors inside ${...} interpolation (B-836)#4096
Conversation
The untagged-backtick arm typed the elaborated `string.from` concat
tree, then discarded every resulting diagnostic except UnresolvedName.
That filter predates the `string.from` desugar: the old `.to_string()`
wrapping could genuinely fail on synthetic spans, but the current
wrapper is total, so the filter only swallowed the user's own errors
(member access, operators, calls) inside `${...}`. `baml check` passed,
and the `Ty::Error` recovery type flowed into MIR, where runtime
lowering ICEs (previously: a degraded type reached the VM as a raw
"type error: expected map, got array").
Keep every elaborated-tree diagnostic; they anchor on the original
`${...}` spans because the segment ExprIds are shared with the tree.
Delete the dead retain_user_name_diagnostics helper.
Dropping the filter exposed one real cascade: `string.from<T>` reported
"cannot infer type parameter" (E0002) when its argument had already
failed to type. Extend check_call_inner's cascade suppression per
parameter: skip CannotInferTypeParameter when the variable occurs in a
param whose argument's recorded type carries an error-recovery sentinel
(Error or Unknown); an argument only ever holds one through its own
already-diagnosed failure. Consolidate the hand-rolled contains_*
walkers in generics.rs onto a shared contains_ty_where and express
contains_typevar, contains_typevar_where, and the new
contains_error_recovery through it.
|
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)
📝 WalkthroughWalkthroughThis change centralizes recursive type inspection, suppresses cascading generic inference diagnostics caused by prior errors, and preserves type errors from backtick interpolation expressions. Regression tests cover generic inference and interpolation segments. ChangesDiagnostic reporting
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.
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_tir/src/generics.rs`:
- Around line 170-230: Add a #[cfg(test)] module in generics.rs with focused
unit tests for contains_ty_where and contains_error_recovery, covering direct
matches, nested types, and non-matching types across the relevant Ty branches,
including function, collection, union, and generic argument structures. Use
existing Ty construction utilities and assert both positive and negative results
without changing the helper implementations.
🪄 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: 0b2357e4-d212-40ec-b7b0-1448ec5c4b05
⛔ Files ignored due to path filters (12)
baml_language/crates/baml_tests/snapshots/diagnostic_errors/backtick_interp_type_errors/baml_tests__diagnostic_errors__backtick_interp_type_errors__01_lexer__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/backtick_interp_type_errors/baml_tests__diagnostic_errors__backtick_interp_type_errors__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/backtick_interp_type_errors/baml_tests__diagnostic_errors__backtick_interp_type_errors__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/backtick_interp_type_errors/baml_tests__diagnostic_errors__backtick_interp_type_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/backtick_interp_type_errors/baml_tests__diagnostic_errors__backtick_interp_type_errors__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/backtick_interp_type_errors/baml_tests__diagnostic_errors__backtick_interp_type_errors__10_formatter__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_error_arg_no_cascade/baml_tests__diagnostic_errors__generic_error_arg_no_cascade__01_lexer__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_error_arg_no_cascade/baml_tests__diagnostic_errors__generic_error_arg_no_cascade__02_parser__main.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_error_arg_no_cascade/baml_tests__diagnostic_errors__generic_error_arg_no_cascade__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_error_arg_no_cascade/baml_tests__diagnostic_errors__generic_error_arg_no_cascade__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_error_arg_no_cascade/baml_tests__diagnostic_errors__generic_error_arg_no_cascade__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/generic_error_arg_no_cascade/baml_tests__diagnostic_errors__generic_error_arg_no_cascade__10_formatter__main.snapis excluded by!**/*.snap
📒 Files selected for processing (5)
baml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/generics.rsbaml_language/crates/baml_compiler2_tir/src/infer_context.rsbaml_language/crates/baml_tests/projects/diagnostic_errors/backtick_interp_type_errors/main.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/generic_error_arg_no_cascade/main.baml
💤 Files with no reviewable changes (1)
- baml_language/crates/baml_compiler2_tir/src/infer_context.rs
Binary size checks passed✅ 7 passed
Generated by |
Fixes B-836: type errors inside backtick
${...}interpolation were silently swallowed.baml checkexited 0 on code like`result: ${[1,2,3].len()}`, thenbaml runcrashed (on current canary: the runtime-lowering ICE guard inruntime_ty.rs; on older builds: a rawVM internal error: type error: expected map, got array).Root cause
The sub-expressions were never "skipped" by the checker. The untagged backtick desugars into a
string.from(...)concat tree that shares the original${expr}nodes, and inferring that tree emits the real diagnostics at the right spans. TheExpr::Templatearm then calledretain_user_name_diagnostics, discarding everything except UnresolvedName. That filter predates thestring.fromdesugar: the old.to_string()wrapping could genuinely fail on synthetic spans, butstring.fromis total, so the filter only deleted the user's own errors and letTy::Errorreach MIR.Changes
retain_user_name_diagnosticshelper. The per-segment check still owns the one rule the tree cannot express (non-null interps), anchored on the original${...}spans.string.from<T>reported E0002 (cannot infer type parameter) when its argument had already failed to type.check_call_innernow suppressesCannotInferTypeParameterper parameter when the variable occurs in a param whose argument's recorded type carries an error-recovery sentinel (Error/Unknown); an argument only ever holds one through its own already-diagnosed failure. This also fixes the same double-report for any generic call with a broken argument, e.g.consume([1,2,3].len()).contains_*walkers ingenerics.rsonto a sharedcontains_ty_wherenode-predicate traversal;contains_typevar,contains_typevar_where, and the newcontains_error_recoveryare one-liners over it.Tests
Two new corpus projects, written first with baselines captured against the unfixed compiler:
diagnostic_errors/backtick_interp_type_errors: the ticket's literal repro plus chained calls, operator errors, unresolved names, and errors inside${for}/${if}segment bodies. Before: only the unresolved name reported. After: all seven diagnostics, each at its original span, no cascades.diagnostic_errors/generic_error_arg_no_cascade: errored-arg suppression, the bound-from-expected case, and a control pinning that a genuinely uninferable parameter (phantom<T>()) still reports E0002.Full
-p baml_testsrun (2834 tests) passes with zero changes to existing snapshots;-p baml_compiler2_tirand thebex_enginebacktick suites pass; clippy clean.Note:
enforce-baml-sizeis expected to fail (stale baseline, known team-wide issue since ~07-17).Summary by CodeRabbit
Bug Fixes
for/ifsegments).Tests
for/ifbodies.