Fix tree-walker escaping function from match-in-loop tail#157
Merged
Conversation
…p tail
A `?match{}` expression at the tail of an `@` or `wh` loop body was
silently early-returning the enclosing function with the match arm's
value. Three personas independently reported this as "match arm
silently exits the outer loop" across the assessment runs.
Root cause: `Stmt::Match` in eval_stmt had a special-case
`if is_last { return BodyResult::Return(v) }` that fired whenever the
match was the final statement of any body, function or loop. The
function-body handler in `eval_body` already returns
`BodyResult::Value(last)` at line 1691 for the function caller to
convert into the function return, so the special-case was duplicating
that logic incorrectly. Inside a loop body it caused the match's value
to propagate as a Return through the loop and out of the function,
bypassing whatever came after.
The fix is one-line removal. With the special-case gone the match
yields `Value(v)` like any other expression statement: the loop body's
`last` accumulator captures it on each iteration, the loop returns
`Value(last)` to its caller, and function-tail match still works
because eval_body's existing tail handling does the conversion.
`is_last` was the only consumer of that parameter and the only caller
of `eval_stmt` is `eval_body`, so removed the plumbing entirely
rather than leaving dead args that invite the special-case to come
back. VM and Cranelift were always correct; this brings the
tree-walker into engine parity.
Cross-engine regression covers: match-as-tail-of-@-loop preserves the function's actual tail; match-as-tail-of-function still returns the match value; side-effect arms inside loops update accumulators correctly; cnt and brk inside match arms still propagate to the loop; match-as-tail-of-wh-loop matches @-loop behaviour. All five run against tree, vm, and cranelift. examples/match-in-loop.ilo demonstrates both shapes (match as loop tail, match as function tail) with run/out directives so examples_engines.rs exercises them per engine. Gives agents an in-context learning sample of the now-correct pattern alongside the harness-level coverage.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
Summary
A
?match{}expression at the tail of an@orwhloop body was silently early-returning the enclosing function with the match arm's value. Three personas independently reported this during assessment runs as "match arm silently exits the outer loop." VM and Cranelift were correct; only the tree-walker was wrong.Engine-divergent silent miscompilation is the worst possible failure mode for an AI-targeted language: the program verifies, runs, returns the wrong number with no error signal, and the agent burns retries debugging logic that isn't broken. Same severity class as the slc/mset bug PR #150 fixed.
Repro
--run-tree42❌5✓--run-vm5✓5✓--run-cranelift5✓5✓Root cause
Stmt::Matchineval_stmthad a special-caseif is_last { return BodyResult::Return(v) }that fired whenever the match was the final statement of any body — function or loop. The function-body handler ineval_bodyalready returnsBodyResult::Value(last)for the function caller to convert into the function return, so the special-case at Stmt::Match was duplicating that logic incorrectly. Inside a loop body it made the match's value propagate as a function-level Return, bypassing whatever came after the loop.What's in the diff
interpreter: tree-walker no longer escapes function from match-in-loop tail— one-line removal of theif is_lastbranch inStmt::Match'sValue(v)arm.is_lastwas the only consumer of that parameter andeval_bodywas the only caller ofeval_stmt, so removed the plumbing entirely rather than leaving dead args that invite the special-case to come back.tests + example: pin match-in-loop tail behaviour across engines— 6 cross-engine regression tests covering match-as-loop-tail, match-as-function-tail (sanity), side-effect arms,cnt/brkpropagation, and thewh-loop analogue.examples/match-in-loop.ilodemonstrates both loop-tail and function-tail shapes with-- run:/-- out:directives sotests/examples_engines.rsexercises them across every engine.Test plan
cargo test --release --features cranelift— 3419 passed, 0 failed, 74 ignored (3413 baseline + 6 new)cargo fmt --all -- --checkcleanis_lastplumbing removed,wh-loop test added, example expanded with function-tail sanity case, header comment trimmed)Follow-ups
The doc entry about
mset m k variablesilently storing wrong value when a0was previously bound was a stale-binary artefact of PR #150 — same root cause, already fixed there. Updatedilo_assessment_feedback.mdto reflect this in the Addressed section.The custom ARM64 JIT removal is in flight on a separate branch (
feature/remove-custom-jit) — independent of this PR, no conflict.