fix(lang): support return as a diverging expression in catch/match arms#3881
Conversation
…arms
Writing a braceless `return` as a `catch`/`match` arm value
(`_ => return 0`) used to fail: `return` was a statement, so the
expression parser rejected it and emitted a cascade of misleading
downstream errors (B-246):
error: Expected expression, found return
error: Float literal values are not supported: 0.0
error: Expected '=>' after catch pattern, found ','
error: operator `Add` cannot be applied to ...
Only the first pointed at the real problem; the rest were recovery
artifacts.
`return` now also parses as a `RETURN_EXPR` — a diverging expression of
type `never`, mirroring `throw`. `never` unifies with any arm type, so
the arm type-checks without special-casing, while the returned value is
still checked against the function's declared return type (identical
typing to the block form `=> { return x }`). Statement-position `return`
is unchanged (still a `RETURN_STMT`); the expression path only fires when
`return` is reached through expression parsing, so no existing snapshots
move.
The formatter wraps a braceless arm body into a block (as it already does
for `throw`) and appends the `;` a block-position `return` needs, so the
output round-trips (idempotent) for both catch and match arms.
Tests:
- ns_catch_arm_return: executable catch/match-arm cases (runtime behavior)
- compiles/catch_arm_return: clean compile + pins the formatter wrap
- diagnostic_errors/catch_arm_braceless_return: a wrong-typed braceless
`return` now yields a single clear `type mismatch`, not the cascade
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds ChangesExpression-position return in catch/match arms
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
⏭️ 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):
|
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_tests/projects/diagnostic_errors/catch_arm_braceless_return/repro.baml`:
- Around line 23-26: The repro in caller() is not exhaustive, so it can trigger
an extra non-exhaustive/throws diagnostic in addition to the intended
return-type mismatch on return e. Update the catch arm handling in caller() by
either adding a fallback arm in the catch expression or narrowing callee() so it
throws only Errors.AuthenticationError, ensuring the fixture isolates the
return-type mismatch alone.
🪄 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: 4813c0ad-b63c-46cd-8b18-477812b47f11
⛔ Files ignored due to path filters (16)
baml_language/crates/baml_tests/snapshots/baml_src/_root.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/baml_src/catch_arm_return.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__01_lexer__catch_arm_return.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__02_parser__catch_arm_return.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__04_5_mir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__06_codegen.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/compiles/catch_arm_return/baml_tests__compiles__catch_arm_return__10_formatter__catch_arm_return.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_arm_braceless_return/baml_tests__diagnostic_errors__catch_arm_braceless_return__01_lexer__repro.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_arm_braceless_return/baml_tests__diagnostic_errors__catch_arm_braceless_return__02_parser__repro.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_arm_braceless_return/baml_tests__diagnostic_errors__catch_arm_braceless_return__03_hir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_arm_braceless_return/baml_tests__diagnostic_errors__catch_arm_braceless_return__04_tir.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_arm_braceless_return/baml_tests__diagnostic_errors__catch_arm_braceless_return__05_diagnostics.snapis excluded by!**/*.snapbaml_language/crates/baml_tests/snapshots/diagnostic_errors/catch_arm_braceless_return/baml_tests__diagnostic_errors__catch_arm_braceless_return__10_formatter__repro.snapis excluded by!**/*.snap
📒 Files selected for processing (16)
baml_language/crates/baml_compiler2_ast/src/ast.rsbaml_language/crates/baml_compiler2_ast/src/lower_expr_body.rsbaml_language/crates/baml_compiler2_hir/src/builder.rsbaml_language/crates/baml_compiler2_mir/src/lower.rsbaml_language/crates/baml_compiler2_tir/src/builder.rsbaml_language/crates/baml_compiler2_tir/src/throws_analysis.rsbaml_language/crates/baml_compiler2_visualization/src/control_flow/from_ast.rsbaml_language/crates/baml_compiler_parser/src/parser.rsbaml_language/crates/baml_compiler_syntax/src/ast.rsbaml_language/crates/baml_compiler_syntax/src/syntax_kind.rsbaml_language/crates/baml_fmt/src/ast/expressions.rsbaml_language/crates/baml_tests/baml_src/ns_catch_arm_return/catch_arm_return.bamlbaml_language/crates/baml_tests/projects/compiles/catch_arm_return/catch_arm_return.bamlbaml_language/crates/baml_tests/projects/diagnostic_errors/catch_arm_braceless_return/repro.bamlbaml_language/crates/baml_tests/src/compiler2_tir/mod.rsbaml_language/crates/tools_onionskin/src/compiler.rs
Binary size checks passed✅ 7 passed
Generated by |
CodeRabbit: the `caller()` repro caught only one of `Errors`' four variants, so the fixture emitted an `extraneous throws` warning alongside the intended return-type mismatch. Make the catch arm exhaustive (`throws string` + a `string =>` arm) so the fixture isolates exactly one diagnostic — the `type mismatch: expected bool, got string` on `return e` — which is the point the regression is meant to pin (no cascade, single clear error).
Latest canary (catch/match-arm `return` support, #3881). Clean merge with no conflicts; workspace compiles and 750 affected-crate tests pass.
Fixes B-246.
Problem
Writing a braceless
returnas acatch/matcharm value fails becausereturnis a statement, not an expression. The compiler emits a cascade of misleading errors where only the first identifies the real problem:The last three are downstream parse failures from recovery misalignment, not real bugs — they read as "this expression has four bugs."
Fix
Adopt the Rust model the ticket recommends:
returnnow also parses as aRETURN_EXPR— a diverging expression of typenever, exactly mirroring howthrowalready works. Sinceneverunifies with any arm type, the arm type-checks without special-casing, while the returned value is still checked against the function's declared return type (identical typing to the block form=> { return x }).throwwas already a diverging expression and worked braceless in arm position; this makesreturnsymmetric.Pipeline (mirrors
throwat every stage)parse_return_exprproducesRETURN_EXPR, recognized inparse_primary_expr. Statement-positionreturnis still taken byparse_stmtfirst, so it stays aRETURN_STMTand no existing snapshots move.Expr::Return { value: Option<ExprId> }; value extraction shared withStmt::Return.never(+ check value against the declared return type, + defer-escape), and lower to a function-exit return (not routed throughcatch_context, so it returns from the function rather than being caught).throw) and appends the;a block-positionreturnneeds, so output round-trips (idempotent) for bothcatchandmatcharms.Tests
baml_src/ns_catch_arm_return— executable catch- and match-arm cases asserting runtime behavior (early-return fires on throw; non-throw path flows normally; mixed arm types).projects/compiles/catch_arm_return— clean compile + pins the formatter wrap for both arm kinds.projects/diagnostic_errors/catch_arm_braceless_return— a wrong-typed bracelessreturnnow yields a single cleartype mismatch: expected bool, got Errors, identical to the block form, instead of the four-error cascade.Full
baml_testslib suite (1703) green;baml_srcsuite (1954 tests) green; parser/fmt/defer/exceptions suites green; clippy clean on all touched crates.Summary by CodeRabbit
New Features
returnin expression positions, includingmatchandcatcharms, with correct control-flow behavior.returnarm bodies are now formatted appropriately when wrapped into blocks.Bug Fixes
returnexpressions.returncatch arms.Tests
returnused as arm values.