Check return expressions against the function's return type - #28
Merged
Conversation
Fixes the second half of #23. `Expr::Return` was never constrained against the enclosing function's signature — the checker only unified the *body's* type with the return type, and only when that type wasn't void: let ty = self.check_expr(body, &func_decl.arena, decls); if func_decl.ret != mk_type(Type::Void) { self.eq(ty, func_decl.ret, ...) } So a return was checked only when it happened to sit in tail position, and never at all in a void function. Both gaps produced an internal compiler error rather than a diagnostic: f() { return 0 } // --check passes, JIT panics f(b: bool) -> i32 { if b { return 1.5 } // --check passes, JIT panics 0 } internal compiler error: cranelift IR verification failed: - inst23 (return v12): arguments of return must match function signature The VM backend accepted both without complaint, so the two backends disagreed on whether the program was valid. Track the enclosing return types in a stack and constrain each `Expr::Return` against the innermost one. A return expression now takes on that type rather than its operand's, so a tail-position return doesn't report the same mismatch twice. Lambdas push a fresh variable that the body and any inner returns unify with, since their return type isn't known up front. The parser stamped `Expr::Return` with the location *after* parsing its operand, which pointed the error at the following token — usually the closing brace. Capture the location of the `return` keyword instead, as Break and Continue already do. Two existing tests change: - checker/return_type_mismatch: same error, now located at the return. - generics/unsolved_typevar_error: this program is no longer ambiguous. The extra constraint from `return pool[idx]` is enough to infer T, and it now compiles and runs correctly on both backends — covered by the new generics/generic_global_return_infers. Rewritten to keep guarding the original hang with a case where nothing constrains T. Bare `return` (the first half of #23) is still a parse error; that needs `Expr::Return(Option<ExprID>)` and backend changes, so it's left for a follow-up. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BX9w4k8qecsH9LMgBP8hXc
Merged
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
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.
Fixes the second half of #23.
Expr::Returnwas never constrained against the enclosing function's signature. The checker only unified the body's type with the return type, and only when that type wasn't void:So a return was checked only when it happened to sit in tail position, and never at all in a void function. Both gaps produced an internal compiler error instead of a diagnostic:
The VM backend accepted both without complaint, so the two backends disagreed on whether the program was valid.
Fix
Track the enclosing return types in a stack and constrain each
Expr::Returnagainst the innermost one. Both cases are now ordinary type errors:A return expression takes on the enclosing return type rather than its operand's, so a tail-position return doesn't report the same mismatch twice (once from the new constraint, once from the body-level check). Lambdas push a fresh variable that the body and any inner returns unify with, since their return type isn't known up front.
The parser stamped
Expr::Returnwith the location after parsing its operand, which pointed the error at the following token — usually the closing brace. It now captures the location of thereturnkeyword, asBreakandContinuealready do.Tests
New:
checker/return_value_from_void,checker/early_return_type_mismatch,checker/early_return_ok,generics/generic_global_return_infers.Two existing tests change:
checker/return_type_mismatch— same error, now located at the return rather than the function signature.generics/unsolved_typevar_error— this program is no longer ambiguous. The extra constraint fromreturn pool[idx]is enough to inferT, and it now compiles and runs correctly on both backends (verified:pool⟨i32⟩andpool⟨f32⟩stay distinct). That program moved to the newgeneric_global_return_inferstest; the guard against the original hang is kept with a case where nothing constrainsT.cargo test --workspacepasses: 351 lib tests and 310 golden tests across all four backends.Not covered
Bare
return— the other half of #23 — is still a parse error (Expected expression), becauseparse_stmtunconditionally parses an operand andExpr::Return(ExprID)has no way to represent its absence. Fixing that needsExpr::Return(Option<ExprID>)plus changes in every backend that matches on it, so I left it for a follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01BX9w4k8qecsH9LMgBP8hXc