Repository navigation
🐛 fix: Blame a failure a boxed body returns on the line that failed - #164
Conversation
A failure inside a lambda was reported, by the compiled program, at the line that called it: a lambda passed to lick was blamed on the whole call. The body did record the failing line, but it handed its result back through meow.Returning, which always put the program back at the caller's position. That only holds where a failure panics; a boxed body hands its failure back as a value, so any lambda, and any function whose result is boxed, had its failure moved onto the calling line. The playground reported the line that failed. Returning now follows the rule meow.Call already keeps: it goes back to the caller only when the value is not an unhandled Furball. Closes #163 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthrough
ChangesFailure Position Handling
Priority: ➖ Normal Estimated code review effort: 2 (Simple) | ~10 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to No actionable issue remains; the change is mergeable after normal checks. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to Failures returned from boxed function bodies are now reported at the failing line rather than the calling line. The change does not appear to expand access or privileges, but its effect on shared diagnostic state warrants a design-level check. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. A rabbit checks the caller’s place, Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6791115437
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if _, failed := AsFurball(value); failed { | ||
| return v |
There was a problem hiding this comment.
When the caller catches this Furball with ~>, GagOr turns the call into a successful value but never restores here, so any later failure in the same statement is blamed on the callee. For example, if g fails on line 2, nya(g() ~> 0.0, to_float("x")) on line 4 now reports the to_float("x") failure at line 2; before this change the compiled backend correctly reported line 4. The recovery path needs to restore the position captured before evaluating the caught expression.
Useful? React with 👍 / 👎.
2 participants
Overview
Closes #163
A failure inside a lambda was reported, by the compiled program, at the line that called the lambda. A lambda passed to
lickwas blamed on the whole call:Why
The generated body already recorded the failing line. It then handed its result back through
meow.Returning(__caller, v), which always put the program back at the caller's position. Its doc comment assumed "a call that fails never reaches it". That holds where a failure panics (a typed function). A boxed body, though, hands its failure back as a value, an unhandled Furball, and that goes throughReturninglike any result. So it was not only lambdas: any function whose result is boxed, such asmeow f() Preturning a kitty, had its failure moved onto the calling line.The change
Returningnow follows the rulemeow.Callalready keeps: it goes back to the caller only when the value is not an unhandled Furball. The change is one runtime function. No generated code changes, so no golden moves.A lambda that succeeds, or one that caught its own failure with
~>, still leaves the program at the call site. A failure later in the same statement is still blamed on that statement, and both cases are pinned.Verification
go test ./...passesgo vet ./...passescompiler/lambda_position_test.goruns 7 programs on both the compiled binary and the interpreter. It requires the same output and the same failure, position included. Cases: a lambda called by name, a lambda passed tolick, the last line of a longer lambda, a lambda called inside a typed function, a named function returning a kitty, a failure after a lambda that succeeded, and a failure after a lambda that caught its own. The five failure cases fail on the compiled side without the fix; the two success cases pass either way.runtime/meowrt/position_test.gounit tests forReturning: it goes back to the caller for a boxed and for a native result, stays put for an unhandled Furball, and goes back for a caught one. The Furball case fails without the fix.gofmt -lis clean, and staticcheck, gocritic and misspell are clean on the changed files.hello,fibonacci,fizzbuzz,list_ops) runImpact
Returninginruntime/meowrt/position.goA
meowbinary run outside this checkout builds against the published runtime module, so it picks this up with the next release that carries the runtime, as it did forSeedin #159.AI Session
Checklist
🤖 Generated with Claude Code
https://claude.ai/code/session_01Scc7p1RecmFoikhcXwnTMu
Generated by Claude Code
Summary by CodeRabbit