implement typed rethrows for higher-order functions#3343
Open
rossirpaulo wants to merge 26 commits intocanaryfrom
Open
implement typed rethrows for higher-order functions#3343rossirpaulo wants to merge 26 commits intocanaryfrom
rossirpaulo wants to merge 26 commits intocanaryfrom
Conversation
Add `throws: Box<Ty>` to `Ty::Function` across all compiler layers (parser, CST, AST, TIR, normalization, MIR, runtime type) so the type system can represent `() -> T throws E`. Defaults to `Never` when omitted. Includes covariant subtyping for throws and test coverage for the new syntax.
Preserve throw contracts when functions are used as values: free functions, class methods, and builtins now include their declared throws in the constructed Ty::Function. Lambda bodies infer throws from their expressions (or validate against explicit annotations). Adds comprehensive test coverage for lambda inference, explicit annotations, contract violations, nested lambdas, HOF patterns, function declarations, type aliases, and class field function types.
Implement Phase 4 of typed rethrows for higher-order functions: - Add StoredFunctionRequiresExplicitThrows diagnostic for when a throwing function is assigned to a stored position typed throws never - Enforce throws compatibility in lambda bidirectional checking - Resolve type aliases before lambda checking to handle type alias cases - Add comprehensive test coverage for class fields, local variables, type aliases, and return positions Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds first-class support for optional function-type "throws" clauses across parser, AST, lowering, TIR/MIR Ty representations, inference/builder, throws semantics/contract utilities, normalization/subtyping, diagnostics/LSP display, and a large test suite; updates builtin test registry signatures to include Changes
Sequence DiagramsequenceDiagram
participant Parser as Parser
participant AST as AST/TypeExpr
participant Lower as Type Lowering
participant TySys as Ty System
participant Inference as Type Inference
participant Throws as Throws Semantics
Parser->>AST: parse function type (optional "throws")
AST->>Lower: provide TypeExpr::Function (includes optional throws)
Lower->>TySys: lower_type_expr_with_fn_context (synthesize __throws_<param> if omitted)
TySys->>TySys: construct Ty::Function { params, ret, throws }
Inference->>TySys: infer_lambda_body -> (ret_ty, effective_throws_ty, expressions)
Inference->>Throws: collect_effective_throws(expressions, catch_residual_throws, aliases)
Throws->>Inference: return effective throw facts / concrete throws Ty
Inference->>Inference: validate declared vs effective throws -> emit diagnostics if mismatch
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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✅ 5 passed
Generated by |
Merging this PR will degrade performance by 18.49%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | bench_incremental_no_change |
5.1 ms | 5.8 ms | -13.13% |
| ❌ | WallTime | bench_incremental_modify_function |
6.7 ms | 7.6 ms | -11.95% |
| ❌ | WallTime | bench_scale_100_functions |
52.2 ms | 60.2 ms | -13.41% |
| ❌ | WallTime | bench_parse_only_simple |
28.5 µs | 34.9 µs | -18.49% |
| ❌ | WallTime | bench_scale_deep_nesting |
17 ms | 19.6 ms | -13.4% |
| ❌ | WallTime | bench_incremental_rename_type |
21 ms | 24.3 ms | -13.81% |
| ❌ | WallTime | bench_single_simple_file |
10.9 ms | 12.4 ms | -11.81% |
| ❌ | WallTime | bench_lexer_only_simple |
16.5 µs | 19.7 µs | -16.33% |
| ❌ | WallTime | bench_empty_project |
10.1 ms | 11.2 ms | -10.08% |
| ❌ | WallTime | bench_incremental_add_new_file |
8.4 ms | 9.4 ms | -10.13% |
Comparing rossirpaulo/typed-rethrows-hof (73cdd0f) with canary (e817f17)
Footnotes
-
105 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
rossirpaulo
changed the title
![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: 6
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_tests/src/compiler2_tir/mod.rs (1)
1113-1122:⚠️ Potential issue | 🟡 MinorUse the post-inference result in the declared-
throwsbranch too.This branch still renders
infers ...from the pre-inference throw graph only. Any function with an explicitthrowsand HOF-driven body effects will snapshot stale or missing inferred throws even though the post-inference result is already available.🩹 Minimal fix
- match &inferred_throws { - Some(inferred) => { - format!(" throws {declared} infers {inferred}") - } - None => format!(" throws {declared}"), - } + match post_inference_throws + .as_deref() + .or(inferred_throws.as_deref()) + { + Some(inferred) => { + format!(" throws {declared} infers {inferred}") + } + None => format!(" throws {declared}"), + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_tests/src/compiler2_tir/mod.rs` around lines 1113 - 1122, The declared-throws branch builds its message using a stale pre-inference throws snapshot; update the branch that handles sig.throws so it uses the post-inference inferred result instead of the old snapshot. Concretely, after calling lower_type_expr_in_ns to get declared, change the match on inferred_throws to reference the same post-inference inferred-throws variable used elsewhere (the post-inference binding), so the format!(" throws {declared} infers {inferred}") uses the up-to-date inferred_throws value rather than the pre-inference value; keep lower_type_expr_in_ns, sig.throws and the declared variable as-is.baml_language/crates/baml_compiler2_tir/src/builder.rs (1)
4079-4103:⚠️ Potential issue | 🟠 MajorBuiltin method function types drop declared
throws.This branch computes
throws_tysolely fromsynthetic_effect_varsand never consultssig.throws. Any builtin method with an explicit throws clause is therefore exposed asthrows neverunless it also happens to be effect-polymorphic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler2_tir/src/builder.rs` around lines 4079 - 4103, The current branch builds throws_ty solely from synthetic_effect_vars, ignoring any explicit throws in sig.throws; update the logic that computes throws_ty (the match on synthetic_effect_vars and the constructed Ty::Never / Ty::TypeVar / Ty::Union) to also incorporate sig.throws: if sig.throws is present, map each declared throw into the appropriate Ty (similar mapping used for synthetic_effect_vars), merge those mapped types with the synthetic-effect-derived types into a single union (or single type when only one), and only produce Ty::Never when both synthetic_effect_vars and sig.throws are empty; keep the returned BuiltinResolution::Method construction (params/ret/throws) unchanged apart from using this merged throws_ty. Ensure you still drop(diags) as before.
🧹 Nitpick comments (4)
baml_language/crates/baml_lsp2_actions/src/check.rs (1)
302-303: Add a focused unit test for this new error-to-diagnostic mapping.This new
TirTypeErrorarm should be covered by a local unit test fortir_type_error_to_diagnostic_idto guard against future enum/mapping drift.As per coding guidelines: "
**/*.rs: Prefer writing Rust unit tests over integration tests where possible".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_lsp2_actions/src/check.rs` around lines 302 - 303, Add a focused Rust unit test that constructs a TirTypeError::StoredFunctionRequiresExplicitThrows variant and asserts that calling tir_type_error_to_diagnostic_id on it returns DiagnosticId::TypeMismatch; place the test in the same module (e.g., a #[cfg(test)] mod tests) in check.rs so it’s a local unit test, give it a descriptive name like stored_function_requires_explicit_throws_maps_to_type_mismatch, create any minimal required dummy values to build the TirTypeError variant, call tir_type_error_to_diagnostic_id(error) and use assert_eq! to verify the result.baml_language/crates/baml_compiler_parser/src/parser.rs (1)
2014-2020: Add a parser unit test for function-typethrowssyntax.Please add a local parser test that asserts
(int) -> string throws MyErr(and a malformed variant) parses intoTHROWS_CLAUSEcorrectly.As per coding guidelines: "
**/*.rs: Prefer writing Rust unit tests over integration tests where possible".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler_parser/src/parser.rs` around lines 2014 - 2020, Add Rust unit tests in the same module to cover function-type throws parsing: write a test that feeds "(int) -> string throws MyErr" to the parser (using the existing parsing helper used in this file, e.g., calling Parser::new(...).parse_type() or the local test helper) and assert the resulting syntax tree contains a THROWS_CLAUSE node with the throws token and the identifier child for MyErr; also add a second test for a malformed case like "(int) -> string throws" that asserts a THROWS_CLAUSE node is still produced (but missing identifier) so the parser recovers; place tests in the module's #[cfg(test)] tests block alongside other parser unit tests.baml_language/crates/baml_compiler_syntax/src/ast.rs (1)
2763-2769: Consider merging with the existingimpl TypeExprblock.This adds a second
impl TypeExprblock while there's already a comprehensive one at lines 302-658 containing all other function-type accessors (is_function_type,function_type_params,function_return_type). Movingfunction_throws_type()near those related methods (around line 648) would improve discoverability and maintain consistency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler_syntax/src/ast.rs` around lines 2763 - 2769, The new method function_throws_type() is in a separate impl block for TypeExpr; move it into the existing impl TypeExpr that already contains is_function_type, function_type_params, and function_return_type so all function-type accessors are grouped together—specifically relocate the function_throws_type method into that impl (near the other function_* accessors) and remove the now-empty/duplicate impl block to keep discoverability and consistency.baml_language/crates/baml_tests/src/compiler2_tir/mod.rs (1)
1095-1111: Keeppost_inference_throwsasTyuntil the final render.Stringifying here forces the later
split(" | ")merge, which is lossy for any single thrown type whoseDisplayitself contains|. Keeping the structuredTyavoids reparsing and keeps snapshot output faithful.💡 Safer shape
- let post_inference_throws: Option<String> = { + let post_inference_throws: Option<baml_compiler2_tir::ty::Ty> = { if let Some(ref fb) = func_body_opt { if let baml_compiler2_hir::body::FunctionBody::Expr(ref body) = **fb { let ty = inference.effective_throws(db, pkg_id, body); if matches!(ty, baml_compiler2_tir::ty::Ty::Never { .. }) { None } else { - Some(ty.to_string()) + Some(ty) } } else { None } } else { None } };Then flatten/dedupe the
Tyfacts when assemblingthrows_str, instead of reparsingDisplay.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_tests/src/compiler2_tir/mod.rs` around lines 1095 - 1111, The code currently stringifies the inferred throw type into post_inference_throws (via ty.to_string()), which loses structure and causes later reparsing/splitting (split(" | ")) to be lossy when a type's Display contains '|'; change post_inference_throws to keep the structured baml_compiler2_tir::ty::Ty (i.e., Option<Ty>) instead of Option<String> in the block that inspects FunctionBody::Expr and calls inference.effective_throws, stop calling ty.to_string() there, and then update the later throws_str assembly logic to accept Option<Ty>, flatten and deduplicate Ty facts directly (rather than reparsing Display output) before final rendering to string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@baml_language/crates/baml_compiler2_mir/src/lower.rs`:
- Around line 167-182: In the Tir2Ty::Function -> Ty::Function arm, normalize
throws aliases that resolve to never by first converting the throws type with
convert_tir2_ty(...) and then checking the converted Ty for Ty::Never; if it is
Ty::Never return None (closed throws), otherwise wrap the converted Ty in
Some(Box::new(...)). Apply this change in the throws handling of the
Tir2Ty::Function match arm so alias-to-never gets treated the same as a direct
Tir2Ty::Never.
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 4489-4523: In report_type_mismatch, the stored-function throws
diagnostic is emitted too eagerly; before taking the Ty::Function throws branch
(involving expected_resolved/got_resolved and their throws fields), verify that
the function shapes (parameter types and return type) are already compatible
(e.g., compare params length and elementwise compatibility and return type
compatibility after resolve_alias_chain); only if the shapes match and
expected_throws is Ty::Never while actual_throws is not, emit
TirTypeError::StoredFunctionRequiresExplicitThrows, otherwise fall through to
emit the regular TypeMismatch diagnostic as before.
- Around line 2815-2845: infer_concrete_body_throws currently strips all
Ty::TypeVar and combine_effect_vars_with_body_throws unconditionally re-adds
every synthetic_effect_var when any callback exists, producing incorrect throws;
change the flow so infer_concrete_body_throws preserves concrete thrown types
from the body's effective_throws (do not drop non-concrete facts) and also
return which synthetic effect vars actually escape the body (i.e., which TypeVar
names appear in the body's effective facts), then update
combine_effect_vars_with_body_throws to only include those synthetic_effect_vars
that the body actually escapes and to merge concrete body_throws_facts with
those vars (never eliding concrete throws into Ty::Never), keeping the Union
semantics when there are multiple concrete/var throws; use the existing symbols
infer_concrete_body_throws, combine_effect_vars_with_body_throws,
synthetic_effect_vars and body_throws_facts to locate and implement this change.
- Around line 720-744: The code determines throws_ty from func_def.throws or
e_body but never validates the final lambda function type against the expected
callback type; when func_def.throws is present the function can claim narrower
throws/return types incorrectly. After computing ret_ty and throws_ty in
infer_lambda_body / the lambda builder (where ret_ty, e_body, throws_ty,
func_def are used), perform a final validation step that resolves the
constructed function type (including throws_ty and ret_ty) and checks it is a
subtype of the expected_resolved/expected callback type (use the existing
subtype/unify helper used elsewhere in this module), report a diagnostic via
self.context.report_at_span on mismatch, and fall back or adjust the returned
type accordingly so annotated throws/return types cannot be narrower than the
body or the expected callback type.
In `@baml_language/crates/baml_compiler2_tir/src/effective_throws.rs`:
- Around line 387-412: Expr::OptionalCall currently only recurses into the
callee and args and thus drops throws produced by actually invoking the optional
function; update the Expr::OptionalCall branch in
collect_effective_throws_from_expr to also call
collect_effective_throws_from_call (the same way Expr::Call does) so invocation
throws are included, and ensure the call-handling code can accept an optional
function type by unwrapping Ty::Optional(Box<Ty::Function>) (either before
calling collect_effective_throws_from_call or inside it) so f?.() and optional
method calls record escaping throws correctly.
In `@baml_language/crates/baml_type/src/lib.rs`:
- Around line 164-169: The Function variant of Ty now has a throws field but
both validate_runtime and the fmt::Display arm for Ty::Function still ignore it;
update validate_runtime's Ty::Function match to call validate_runtime() on
throws when Some(...) and update the fmt::Display match for Ty::Function to
include " throws <T>" in the printed output when throws.is_some() (build the
param list from params and append " throws <throws>" if present), referencing
the Ty::Function variant, the validate_runtime function and the fmt::Display
impl so the runtime validation and debug rendering include the throws type.
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_tir/src/builder.rs`:
- Around line 4079-4103: The current branch builds throws_ty solely from
synthetic_effect_vars, ignoring any explicit throws in sig.throws; update the
logic that computes throws_ty (the match on synthetic_effect_vars and the
constructed Ty::Never / Ty::TypeVar / Ty::Union) to also incorporate sig.throws:
if sig.throws is present, map each declared throw into the appropriate Ty
(similar mapping used for synthetic_effect_vars), merge those mapped types with
the synthetic-effect-derived types into a single union (or single type when only
one), and only produce Ty::Never when both synthetic_effect_vars and sig.throws
are empty; keep the returned BuiltinResolution::Method construction
(params/ret/throws) unchanged apart from using this merged throws_ty. Ensure you
still drop(diags) as before.
In `@baml_language/crates/baml_tests/src/compiler2_tir/mod.rs`:
- Around line 1113-1122: The declared-throws branch builds its message using a
stale pre-inference throws snapshot; update the branch that handles sig.throws
so it uses the post-inference inferred result instead of the old snapshot.
Concretely, after calling lower_type_expr_in_ns to get declared, change the
match on inferred_throws to reference the same post-inference inferred-throws
variable used elsewhere (the post-inference binding), so the format!(" throws
{declared} infers {inferred}") uses the up-to-date inferred_throws value rather
than the pre-inference value; keep lower_type_expr_in_ns, sig.throws and the
declared variable as-is.
---
Nitpick comments:
In `@baml_language/crates/baml_compiler_parser/src/parser.rs`:
- Around line 2014-2020: Add Rust unit tests in the same module to cover
function-type throws parsing: write a test that feeds "(int) -> string throws
MyErr" to the parser (using the existing parsing helper used in this file, e.g.,
calling Parser::new(...).parse_type() or the local test helper) and assert the
resulting syntax tree contains a THROWS_CLAUSE node with the throws token and
the identifier child for MyErr; also add a second test for a malformed case like
"(int) -> string throws" that asserts a THROWS_CLAUSE node is still produced
(but missing identifier) so the parser recovers; place tests in the module's
#[cfg(test)] tests block alongside other parser unit tests.
In `@baml_language/crates/baml_compiler_syntax/src/ast.rs`:
- Around line 2763-2769: The new method function_throws_type() is in a separate
impl block for TypeExpr; move it into the existing impl TypeExpr that already
contains is_function_type, function_type_params, and function_return_type so all
function-type accessors are grouped together—specifically relocate the
function_throws_type method into that impl (near the other function_* accessors)
and remove the now-empty/duplicate impl block to keep discoverability and
consistency.
In `@baml_language/crates/baml_lsp2_actions/src/check.rs`:
- Around line 302-303: Add a focused Rust unit test that constructs a
TirTypeError::StoredFunctionRequiresExplicitThrows variant and asserts that
calling tir_type_error_to_diagnostic_id on it returns
DiagnosticId::TypeMismatch; place the test in the same module (e.g., a
#[cfg(test)] mod tests) in check.rs so it’s a local unit test, give it a
descriptive name like
stored_function_requires_explicit_throws_maps_to_type_mismatch, create any
minimal required dummy values to build the TirTypeError variant, call
tir_type_error_to_diagnostic_id(error) and use assert_eq! to verify the result.
In `@baml_language/crates/baml_tests/src/compiler2_tir/mod.rs`:
- Around line 1095-1111: The code currently stringifies the inferred throw type
into post_inference_throws (via ty.to_string()), which loses structure and
causes later reparsing/splitting (split(" | ")) to be lossy when a type's
Display contains '|'; change post_inference_throws to keep the structured
baml_compiler2_tir::ty::Ty (i.e., Option<Ty>) instead of Option<String> in the
block that inspects FunctionBody::Expr and calls inference.effective_throws,
stop calling ty.to_string() there, and then update the later throws_str assembly
logic to accept Option<Ty>, flatten and deduplicate Ty facts directly (rather
than reparsing Display output) before final rendering to string.
🪄 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: 53c25173-4cf1-48ed-bfaa-0e18ef636d02
⛔ Files ignored due to path filters (79)
baml_language/crates/baml_tests/snapshots/__baml_std__/baml_tests____baml_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/builtin_io/baml_tests__builtin_io__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/catch_all_keyword/baml_tests__catch_all_keyword__04_tir.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/catch_throw_regressions/baml_tests__catch_throw_regressions__04_tir.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/function_type_throws/baml_tests__function_type_throws__01_lexer__array_map_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__basic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__catch_absorbs_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__chained_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__class_field_fn_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__compose_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__enum_class_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__explicit_param_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__fn_decl_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__fn_type_alias_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__hof_own_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__hof_rethrows.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__hof_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__lambda_throws_explicit.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__lambda_throws_infer.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__lambda_throws_violation.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__mixed_params.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__nested_lambda_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__returned_closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__stored_callback_enforcement.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__array_map_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__basic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__catch_absorbs_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__chained_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__class_field_fn_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__compose_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__enum_class_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__explicit_param_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__fn_decl_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__fn_type_alias_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__hof_own_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__hof_rethrows.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__hof_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__lambda_throws_explicit.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__lambda_throws_infer.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__lambda_throws_violation.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__mixed_params.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__nested_lambda_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__returned_closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__stored_callback_enforcement.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__array_map_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__basic.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__catch_absorbs_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__chained_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__class_field_fn_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__compose_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__enum_class_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__explicit_param_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__fn_decl_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__fn_type_alias_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__hof_own_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__hof_rethrows.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__hof_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__lambda_throws_explicit.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__lambda_throws_infer.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__lambda_throws_violation.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__mixed_params.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__nested_lambda_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__returned_closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__stored_callback_enforcement.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_advanced/baml_tests__lambda_advanced__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_basic/baml_tests__lambda_basic__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_errors/baml_tests__lambda_errors__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/namespaces_type_resolution/baml_tests__namespaces_type_resolution__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/null_handling/baml_tests__null_handling__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/type_annotation/baml_tests__type_annotation__04_tir.snapis excluded by!**/*.snap
📒 Files selected for processing (39)
baml_language/crates/baml_builtins2/baml_std/testing/registry.bamlbaml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/lib.rsbaml_language/crates/baml_compiler2_ast/src/lower_type_expr.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/effective_throws.rsbaml_language/crates/baml_compiler2_tir/src/generics.rsbaml_language/crates/baml_compiler2_tir/src/infer_context.rsbaml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/lib.rsbaml_language/crates/baml_compiler2_tir/src/lower_type_expr.rsbaml_language/crates/baml_compiler2_tir/src/normalize.rsbaml_language/crates/baml_compiler2_tir/src/ty.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_lsp2_actions/src/check.rsbaml_language/crates/baml_tests/projects/function_type_throws/array_map_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/basic.bamlbaml_language/crates/baml_tests/projects/function_type_throws/catch_absorbs_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/chained_hof.bamlbaml_language/crates/baml_tests/projects/function_type_throws/class_field_fn_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/compose_hof.bamlbaml_language/crates/baml_tests/projects/function_type_throws/enum_class_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/explicit_param_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/fn_decl_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/fn_type_alias_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/hof_own_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/hof_rethrows.bamlbaml_language/crates/baml_tests/projects/function_type_throws/hof_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/lambda_throws_explicit.bamlbaml_language/crates/baml_tests/projects/function_type_throws/lambda_throws_infer.bamlbaml_language/crates/baml_tests/projects/function_type_throws/lambda_throws_violation.bamlbaml_language/crates/baml_tests/projects/function_type_throws/mixed_params.bamlbaml_language/crates/baml_tests/projects/function_type_throws/nested_lambda_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/returned_closures.bamlbaml_language/crates/baml_tests/projects/function_type_throws/stored_callback_enforcement.bamlbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_type/src/lib.rs
baml_language/crates/baml_compiler2_tir/src/effective_throws.rs
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
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_tir/src/throw_inference.rs (1)
69-79:⚠️ Potential issue | 🟠 MajorPull dependency-exported aliases into this pre-inference alias map.
Line 69 only collects aliases from the current package, even though the dependency interfaces are already available on Lines 71-79. If a callback parameter is typed as an exported dependency alias to a function type,
function_throws_factscannot unwrap it here, socollect_direct_param_call_throwsdrops the callback's direct throws before the graph is analyzed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler2_tir/src/throw_inference.rs` around lines 69 - 79, The pre-inference alias map currently only includes local aliases from collect_type_aliases(db, pkg_items) so exported type aliases from dependencies (available in dep_interfaces) are missed; update the alias map to merge in dependency-exported aliases before running function_throws_facts/collect_direct_param_call_throws by iterating dep_interfaces (the (Name, &crate::package_interface::PackageInterface) pairs returned by package_dependencies + package_interface::package_interface) and pulling each dependency's exported type aliases into the same aliases structure so callback parameter types that reference exported dependency aliases resolve correctly during throw inference.
🧹 Nitpick comments (2)
baml_language/crates/baml_tests/projects/function_type_throws/fn_decl_throws.baml (1)
15-24: Consider avoiding constant inputs to strengthen rethrows assertions.Using
declared_may_fail(1)is valid, but parameterizing the input can make this fixture more resilient if path-sensitive effect narrowing is added later.Optional robustness tweak
-function caller() -> string { - declared_may_fail(1) +function caller(x: int) -> string { + declared_may_fail(x) } @@ -function safe_caller() -> string { - declared_may_fail(1) catch (e) { +function safe_caller(x: int) -> string { + declared_may_fail(x) catch (e) { _ => "caught" } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_tests/projects/function_type_throws/fn_decl_throws.baml` around lines 15 - 24, The test currently calls declared_may_fail(1) with a hard-coded constant which weakens rethrow/effect assertions; change caller and safe_caller to accept an input parameter and forward that parameter to declared_may_fail instead (update function signatures of caller and safe_caller and the call sites inside them to use the parameter), keeping the catch block in safe_caller intact; this makes the fixture use a variable input to declared_may_fail rather than the constant 1 and preserves the intended behaviors of declared_may_fail, caller, and safe_caller.baml_language/crates/baml_tests/projects/function_type_throws/optional_call_throws.baml (1)
13-15: LGTM!Integration test correctly exercises the rethrows path by supplying a throwing callback whose
throws stringtype is inferred from thethrow "boom"expression.Consider adding a companion test function that exercises
optional_call_caughtto verify the catch path end-to-end:💡 Optional: add catch-path integration test
function test_optional_call_with_throwing_callback() -> int? { optional_call_rethrows(() -> int { throw "boom" }) } + +function test_optional_call_caught_returns_null() -> int? { + optional_call_caught(() -> int { throw "boom" }) +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_tests/projects/function_type_throws/optional_call_throws.baml` around lines 13 - 15, Add a companion integration test that exercises the catch path by calling optional_call_caught with a throwing callback so the thrown string is caught end-to-end; create a new test function (e.g., test_optional_call_with_caught_callback) that invokes optional_call_caught(() -> int { throw "boom" }) and asserts the expected caught result or behavior, using the same style as test_optional_call_with_throwing_callback to ensure the catch branch is exercised for optional_call_caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@baml_language/crates/baml_compiler2_tir/src/inference.rs`:
- Around line 512-518: The check_throws_contract call is using a builder seeded
only with func_def.generic_params so inner lambdas lose enclosing generics and
thrown types lower to Unknown; update the code that calls
builder.check_throws_contract (the invocations using symbols builder,
check_throws_contract, lambda_body, func_def) to use a builder that includes the
enclosing generic parameter scope (i.e., merge or create a new builder seeded
with both the current func_def.generic_params and any outer generic params
visible to the lambda) before calling check_throws_contract so thrown types are
resolved with enclosing generics in scope; apply the same fix to the other
occurrence around the block that references the same symbols (the other call at
the later lines).
- Around line 169-188: The code in effective_throws currently rebuilds aliases
from only package_items via collect_type_aliases and passes that limited set
into crate::effective_throws::collect_effective_throws; replace that step so you
pass the full alias set instead of recomputing from package_items (do not call
collect_type_aliases(pkg_items) here). Locate the effective_throws function and
its call to crate::effective_throws::collect_effective_throws and change the
aliases argument to the global/full alias map you already maintain (e.g., the
module/global alias collection or the previously computed alias set on the
inference context) so function_throws_facts sees resolved aliases rather than
opaque Ty::TypeAlias. Ensure you remove the local collect_type_aliases use and
pass the existing full alias variable into collect_effective_throws.
In `@baml_language/crates/baml_compiler2_tir/src/throws_semantics.rs`:
- Around line 183-225: The shape check function
function_shape_matches_ignoring_outer_throws currently only matches raw
Ty::Function and misses cases where functions are wrapped in Ty::Optional;
update function_shape_matches_ignoring_outer_throws to first resolve aliases
(keep resolve_alias_chain) and then peel matching Ty::Optional layers on both
got_resolved and expected_resolved (unwrapping Box<Ty::Function> when both are
Optional) before the match, so that when both sides are Optional(Function{...})
you construct the two Function types with throws set to Ty::Never and call
normalize::is_subtype_of as before (preserve use of aliases and TyAttr
handling).
---
Outside diff comments:
In `@baml_language/crates/baml_compiler2_tir/src/throw_inference.rs`:
- Around line 69-79: The pre-inference alias map currently only includes local
aliases from collect_type_aliases(db, pkg_items) so exported type aliases from
dependencies (available in dep_interfaces) are missed; update the alias map to
merge in dependency-exported aliases before running
function_throws_facts/collect_direct_param_call_throws by iterating
dep_interfaces (the (Name, &crate::package_interface::PackageInterface) pairs
returned by package_dependencies + package_interface::package_interface) and
pulling each dependency's exported type aliases into the same aliases structure
so callback parameter types that reference exported dependency aliases resolve
correctly during throw inference.
---
Nitpick comments:
In
`@baml_language/crates/baml_tests/projects/function_type_throws/fn_decl_throws.baml`:
- Around line 15-24: The test currently calls declared_may_fail(1) with a
hard-coded constant which weakens rethrow/effect assertions; change caller and
safe_caller to accept an input parameter and forward that parameter to
declared_may_fail instead (update function signatures of caller and safe_caller
and the call sites inside them to use the parameter), keeping the catch block in
safe_caller intact; this makes the fixture use a variable input to
declared_may_fail rather than the constant 1 and preserves the intended
behaviors of declared_may_fail, caller, and safe_caller.
In
`@baml_language/crates/baml_tests/projects/function_type_throws/optional_call_throws.baml`:
- Around line 13-15: Add a companion integration test that exercises the catch
path by calling optional_call_caught with a throwing callback so the thrown
string is caught end-to-end; create a new test function (e.g.,
test_optional_call_with_caught_callback) that invokes optional_call_caught(() ->
int { throw "boom" }) and asserts the expected caught result or behavior, using
the same style as test_optional_call_with_throwing_callback to ensure the catch
branch is exercised for optional_call_caught.
🪄 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: af209f89-376b-4641-ba36-9b3981ad7f5f
⛔ Files ignored due to path filters (32)
baml_language/crates/baml_tests/snapshots/catch_return_type_mismatch/baml_tests__catch_return_type_mismatch__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__array_map_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__catch_absorbs_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__compose_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__fn_decl_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__fn_type_alias_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__hof_rethrows.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__lambda_throws_violation.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__optional_call_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__returned_closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__01_lexer__stored_callback_enforcement.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__array_map_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__catch_absorbs_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__compose_hof.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__fn_decl_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__fn_type_alias_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__hof_rethrows.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__lambda_throws_violation.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__optional_call_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__returned_closures.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__02_parser__stored_callback_enforcement.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__array_map_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__catch_absorbs_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__fn_decl_throws.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__hof_rethrows.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__lambda_throws_violation.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/function_type_throws/baml_tests__function_type_throws__10_formatter__optional_call_throws.snapis excluded by!**/*.snap
📒 Files selected for processing (21)
baml_language/crates/baml_compiler2_ast/src/disambiguate.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/effective_throws.rsbaml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/lib.rsbaml_language/crates/baml_compiler2_tir/src/throw_inference.rsbaml_language/crates/baml_compiler2_tir/src/throws_semantics.rsbaml_language/crates/baml_lsp2_actions/src/utils.rsbaml_language/crates/baml_tests/projects/function_type_throws/array_map_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/catch_absorbs_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/compose_hof.bamlbaml_language/crates/baml_tests/projects/function_type_throws/fn_decl_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/fn_type_alias_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/hof_rethrows.bamlbaml_language/crates/baml_tests/projects/function_type_throws/lambda_throws_violation.bamlbaml_language/crates/baml_tests/projects/function_type_throws/optional_call_throws.bamlbaml_language/crates/baml_tests/projects/function_type_throws/returned_closures.bamlbaml_language/crates/baml_tests/projects/function_type_throws/stored_callback_enforcement.bamlbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/baml_type/src/lib.rs
✅ Files skipped from review due to trivial changes (4)
- baml_language/crates/baml_compiler2_tir/src/lib.rs
- baml_language/crates/baml_tests/projects/function_type_throws/returned_closures.baml
- baml_language/crates/baml_compiler2_mir/src/lower.rs
- baml_language/crates/baml_tests/projects/function_type_throws/fn_type_alias_throws.baml
🚧 Files skipped from review as they are similar to previous changes (8)
- baml_language/crates/baml_tests/projects/function_type_throws/array_map_throws.baml
- baml_language/crates/baml_tests/projects/function_type_throws/lambda_throws_violation.baml
- baml_language/crates/baml_tests/projects/function_type_throws/catch_absorbs_throws.baml
- baml_language/crates/baml_tests/projects/function_type_throws/compose_hof.baml
- baml_language/crates/baml_tests/projects/function_type_throws/hof_rethrows.baml
- baml_language/crates/baml_tests/src/compiler2_tir/mod.rs
- baml_language/crates/baml_tests/projects/function_type_throws/stored_callback_enforcement.baml
- baml_language/crates/baml_compiler2_tir/src/builder.rs
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
baml_language/crates/baml_compiler2_tir/src/throws_semantics.rs (1)
266-291: Broaden this regression and run the library test suite.This pins the raw
Optional(Function)case, but the helper also depends on alias expansion. Please add aTy::TypeAlias -> Optional(Function)variant here too, and runcargo test --libbefore merge since the PR validation only lists targetedbaml_testsruns.As per coding guidelines, "Prefer writing Rust unit tests over integration tests where possible" and "Always run
cargo test --libif you changed any Rust code".baml_language/crates/baml_compiler2_tir/src/inference.rs (2)
77-78: Consider sharing the alias map instead of cloning it into every cached scope.
infer_scope_typesis Salsa-cached per scope, but this alias set is effectively file/package-wide and now gets cloned into bothTypeInferenceBuilderand everyScopeInference. On files with many lambda scopes, that duplicates the same dependency-export map in each cached result and makessalsa::Updatecompare more data than necessary. AnArc<HashMap<...>>or a dedicated alias query would keep the new behavior without the per-scope copy cost.Also applies to: 303-305, 682-688
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler2_tir/src/inference.rs` around lines 77 - 78, The aliases HashMap stored and cloned into every cached scope (the aliases field of ScopeInference and the data passed into TypeInferenceBuilder used by infer_scope_types) should be shared instead of deep-copied: change the type from HashMap<crate::ty::QualifiedTypeName, Ty> to Arc<HashMap<crate::ty::QualifiedTypeName, Ty>> (or return an Arc from a new dedicated query) and update all construction sites (where ScopeInference and TypeInferenceBuilder are created and where infer_scope_types assembles aliases) to clone the Arc rather than the map contents so Salsa comparisons and per-scope memory use only copy the Arc pointer.
251-262: Finish extracting lambda generic-scope assembly.You already have
extend_generic_params_with_enclosing_lambdas, but the full merge order still lives in both lambda branches. Since parameter annotation lowering andcheck_throws_contractboth rely on that exact visible generic set, folding the remaining class/function/current-lambda pieces into one helper would make this path much harder to regress.Also applies to: 503-530, 585-596
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_compiler2_tir/src/inference.rs` around lines 251 - 262, The current helper extend_generic_params_with_enclosing_lambdas only folds generic params from enclosing lambdas; finish extracting and centralize the full visible-generic assembly (including class, function, and current-lambda scopes) into a single helper used by parameter-annotation lowering and check_throws_contract to ensure consistent merge order; implement a new or extended helper (reuse/extend extend_generic_params_with_enclosing_lambdas and find_lambda_by_span) that takes generic_params: &mut Vec<Name>, body: &ExprBody, source_map: &AstSourceMap and an enclosing_lambda_spans slice and appends generics from enclosing lambdas, the current lambda, enclosing function/class scopes in the correct merge order, then replace the duplicated merge logic at the other sites (the blocks around the existing calls at ~lines 503-530 and 585-596) to call this single helper so both parameter annotation lowering and check_throws_contract observe the exact same visible generic set.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@baml_language/crates/baml_compiler2_tir/src/inference.rs`:
- Around line 77-78: The aliases HashMap stored and cloned into every cached
scope (the aliases field of ScopeInference and the data passed into
TypeInferenceBuilder used by infer_scope_types) should be shared instead of
deep-copied: change the type from HashMap<crate::ty::QualifiedTypeName, Ty> to
Arc<HashMap<crate::ty::QualifiedTypeName, Ty>> (or return an Arc from a new
dedicated query) and update all construction sites (where ScopeInference and
TypeInferenceBuilder are created and where infer_scope_types assembles aliases)
to clone the Arc rather than the map contents so Salsa comparisons and per-scope
memory use only copy the Arc pointer.
- Around line 251-262: The current helper
extend_generic_params_with_enclosing_lambdas only folds generic params from
enclosing lambdas; finish extracting and centralize the full visible-generic
assembly (including class, function, and current-lambda scopes) into a single
helper used by parameter-annotation lowering and check_throws_contract to ensure
consistent merge order; implement a new or extended helper (reuse/extend
extend_generic_params_with_enclosing_lambdas and find_lambda_by_span) that takes
generic_params: &mut Vec<Name>, body: &ExprBody, source_map: &AstSourceMap and
an enclosing_lambda_spans slice and appends generics from enclosing lambdas, the
current lambda, enclosing function/class scopes in the correct merge order, then
replace the duplicated merge logic at the other sites (the blocks around the
existing calls at ~lines 503-530 and 585-596) to call this single helper so both
parameter annotation lowering and check_throws_contract observe the exact same
visible generic set.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: e70b0143-2481-43ff-a505-3f31d76fb053
📒 Files selected for processing (2)
baml_language/crates/baml_compiler2_tir/src/inference.rsbaml_language/crates/baml_compiler2_tir/src/throws_semantics.rs
LSP tests and bytecode snapshots expected stale throws contract violation diagnostics and old optional-function-type formatting that changed with the typed rethrows for higher-order functions work. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_violation.baml (1)
19-19: Align top-of-file comments with the new no-diagnostics expectation.Line 19 is correct now, but Line 1 and Line 2 still describe this as a mismatch/unsupported case. Updating those comments will avoid confusion for future test maintenance.
♻️ Suggested comment update
-// throws enum type, but body throws variants — contract mismatch -// NOTE: enum variants in throws type position are not yet supported. +// throws enum type, body throws enum variants +// bare enum throws semantically covers thrown enum variants.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_violation.baml` at line 19, Top-of-file comment lines 1 and 2 still indicate a mismatch/unsupported diagnostics expectation but the test now expects no diagnostics (as shown by the marker at line 19); update those initial comments to reflect the new expectation (e.g., replace the old mismatch/unsupported comment text with a no-diagnostics expectation) so the file’s header matches the actual test marker and avoids future confusion.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_violation.baml`:
- Line 19: Top-of-file comment lines 1 and 2 still indicate a
mismatch/unsupported diagnostics expectation but the test now expects no
diagnostics (as shown by the marker at line 19); update those initial comments
to reflect the new expectation (e.g., replace the old mismatch/unsupported
comment text with a no-diagnostics expectation) so the file’s header matches the
actual test marker and avoids future confusion.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 83621e15-227e-48ba-a0ae-ffddc815235b
⛔ Files ignored due to path filters (8)
baml_language/crates/baml_tests/snapshots/__baml_std__/baml_tests____baml_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__baml_std__/baml_tests____baml_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/__testing_std__/baml_tests____testing_std____06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/lambda_basic/baml_tests__lambda_basic__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_expanded_unoptimized.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/tests/bytecode_format/snapshots/bytecode_format__bytecode_display_textual.snapis excluded by!**/*.snap
📒 Files selected for processing (8)
baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/function_throws.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_caller_sees_contract.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_caller_variant_contract.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_exact_match.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_extraneous.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_precise.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_violation.bamlbaml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_mixed.baml
✅ Files skipped from review due to trivial changes (6)
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_extraneous.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_variant_precise.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_enum_exact_match.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_mixed.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/throw/throws_caller_sees_contract.baml
- baml_language/crates/baml_lsp2_actions_tests/test_files/syntax/hover/function_throws.baml
Consolidate signature lowering (params, ret, throws, effect vars) into a shared `callable_boundary` module used by builder, inference, and throw_inference — eliminating duplicated logic. Only widen outward throws for callbacks that are actually invoked in the body, add diagnostic formatting that renders `__throws_` type vars as human-readable messages, and extend test coverage for under-declared rethrows, unused callbacks, stored callback enforcement, and generic class methods. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Enforce the A+B contract: callback parameters with implicit effect polymorphism that are never directly invoked now produce an actionable error. This closes a type soundness hole where throws were silently dropped. - Add UnusedCallbackEffectVar error variant in infer_context.rs - Emit diagnostic in inference.rs by capturing direct_callback_effect_vars - Add LSP error code mapping in check.rs - Update test files with ERROR comments for expected diagnostics - Fix stdlib functions with explicit throws annotations
Filter synthetic_display_vars in the TIR display to only show effect vars whose source callback params are directly invoked. This ensures the displayed throws type matches runtime behavior. - Make directly_invoked_callback_params public in callable_boundary.rs - Filter synthetic display vars in Branch 2 of throws display logic - Update TIR snapshots to reflect corrected display
…ollections Add targeted regression tests for two scenarios supported by the machinery but not directly exercised: - Generic stored callback fields (Handler<E> pattern) - Heterogeneous stored callback collection widening (Task<E>[]) Note: These tests currently show throws never because generic type parameters in stored callback fields are not yet instantiated during throws inference. The tests serve as regression coverage for when this is improved.
Phase 1: StructuralTy::Class and StructuralTy::Enum now carry type_args, enabling invariant subtype comparison for instantiated nominals (e.g. Handler<string> vs Handler<int> are distinct). Shared instantiate_alias helper funnels all six alias expansion sites. Generic inference helpers (contains_typevar, infer_bindings, erase_unresolved_typevars) recurse into nominal type_args. Phase 2: Cross-package class field and method resolution routed through PackageResolutionContext with generic substitution applied inside the methods. Builder delegates to res_ctx instead of bypassing PackageInterface. Fixed cross-namespace struct literal resolution fallback. New test fixtures for generic subtype rejection and generic function inference over nominal wrappers.
Add collect_member_field_call_throws to the pre-inference throw pipeline so that throws from typed member calls (h.run(), self.run(), task.run()) propagate to callers. Uses PackageResolutionContext for member lookup with generic substitution applied. test_string_handler now correctly infers throws string, test_int_handler infers throws int, and test_run_mixed_tasks infers throws int | string via transitive propagation.
Update display_type_expr to render type_args recursively for TypeExpr::Path (e.g. Handler<string> instead of Handler). Include generic_params in hover tooltips for class and function definitions so class Handler<E> and function unwrap<T> show their type parameters. Add LSP hover test infrastructure (CursorTest::type_info) and 11 tests covering generic class/function definitions, usage-site resolution, nested generics, optional/array generic params, and markdown rendering.
Twelve 04_tir snapshots updated with trivially larger serialization from the new Vec<StructuralTy> type_args on Class/Enum variants. MIR snapshots (04_5_mir) are unchanged — no scope leak.
Add throws field to TypeInfo::Function populated from either inferred transitive throw sets (via function_throw_sets TIR query) or the declared throws clause from the function signature. Hover now shows e.g. `function use_handler(h: Handler<string>) -> null throws string` for functions with inferred or declared throws. Functions that throw never omit the throws clause entirely. Add 4 new LSP hover tests covering: no-throws omission, declared throws, inferred throws from member calls, and union throws.
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.
Summary
Implements typed rethrows for higher-order functions across the compiler2 pipeline, then tightens the semantics so the feature behaves correctly in the cases the original draft still got wrong.
This branch now carries
throwsthrough function values, propagates direct callback effects through higher-order wrappers, keeps stored callback positions closed by default, and fixes the contract-checking and alias-propagation gaps that were still present in the earlier Phase 4 / Phase 5 implementation.What Changed
Shared Throws Semantics In TIR
baml_compiler2_tirand moved coverage logic there instead of duplicating it across builder-side checks and snapshot rendering.Tydata rather than reparsingTy::to_string().Correct Throws Contract Checking
throws ErrorKindincorrectly failed against thrown variants likeErrorKind.NotFound.throws contract violationerrors for broad enum declarationsextraneous throws declarationwarningsAlias-Aware Higher-Order Rethrows
type EnumThrower = () -> int throws ErrorKindfunction use_enum_thrower(f: EnumThrower) -> int { f() }Stored Callback Enforcement
Regression Suite Cleanup
compose_hofnow returns the closure explicitlyreturned_closuresnow returns closures explicitly instead of snapshotting missing-return errorsStdlib Boundary Policy
Architecture Notes
This stays aligned with
baml_language/ARCHITECTURE.md:User / Developer Impact
throws ErrorKindnow correctly covers thrown enum variants.throws never, but the error message is now reserved for the actual throws-only mismatch case.Validation
cargo test -p baml_tests function_type_throws -- --nocapturecargo test -p baml_tests testing_std -- --nocaptureSummary by CodeRabbit
New Features
() -> int throws string), shown in type display and propagated through calls, higher-order functions, optional calls, and catches.Tests