Repository navigation
Add try..finally - #8931
Open
cknitt wants to merge 1 commit into
Open
Add try..finally#8931cknitt wants to merge 1 commit into
cknitt wants to merge 1 commit into
Conversation
Why:
JavaScript code cleans up with finally blocks: after an await, around
break and continue, and in generators, where cancellation is only seen
by finally. ReScript had no way to express them.
`try` takes `finally { ... }` after its body or after its catch. `finally`
stays an identifier: after a catch it only starts a finally block when a
brace follows. The block is a statement and must not break or continue out
of it. Supersedes #7903.
AST modified (Pexp_try):
Example:
let f = g =>
try g() catch {
| Not_found => 0
} finally {
Console.log("done")
}
Before (diagnostic): `finally` was not valid after a catch.
After (parse):
expression
Pexp_try
expression
Pexp_apply
expression
Pexp_ident "g"
…
[
<case>
pattern
Ppat_construct "Not_found"
expression
Pexp_constant PConst_int (0,None)
]
Some
expression
Pexp_apply
expression
Pexp_ident "Console.log"
…
Lambda added (Ltryfinally):
After (lambda):
(try
(try (apply g/1002 unit) with raw_exn/1062
…)
finally (seq (js_call[console.log] "done") unit))
JS added (try..catch..finally):
After (js):
function f(g) {
try {
return g();
} catch (raw_exn) {
let exn = Primitive_exceptions.internalToException(raw_exn);
if (exn.RE_EXN_ID === "Not_found") {
return 0;
}
throw exn;
} finally {
console.log("done");
}
}
Diagnostic added (break out of finally):
Example:
while true {
try {
()
} finally {
break
}
}
After (diagnostic):
`break` cannot leave a `finally` block. Only a loop inside the block can be broken out of.
Review: Why a separate Ltryfinally instead of an optional finally on
Ltrywith? Ltrywith keeps its meaning in every pass, a try without catch
needs no exception ident, and try..catch..finally is an Ltryfinally around
an Ltrywith that the JS backend prints as one statement.
Review: Why is the catch handler no longer in tail position? With a
finally, a self tail call in the handler would become a loop `continue`
that runs the finally before the recursive call instead of after it.
Review: Why reject break and continue in finally? JavaScript allows them,
but they silently drop a pending exception or return value.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF
Signed-off-by: Christoph Knittel <ck@cca.io>
Member
Author
|
@codex review |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #8931 +/- ##
==========================================
+ Coverage 81.13% 81.16% +0.02%
==========================================
Files 456 456
Lines 63750 63897 +147
==========================================
+ Hits 51724 51859 +135
- Misses 12026 12038 +12
🚀 New features to boost your workflow:
|
|
Codex Review: Didn't find any major issues. Swish! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
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. |
cknitt
marked this pull request as ready for review
October 20, 2026 17:54
rescript
@rescript/belt
@rescript/darwin-arm64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/runtime
@rescript/win32-x64
commit: |
|
Developer playground preview: https://rescript-lang.github.io/rescript/dev-playground/?version=pr-8931 |
This branch has not been deployed
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.
Adds
finallyblocks totry, with or without acatch. Closes #6974; supersedes #7903, reimplemented on current master.The output is a native JavaScript
try/catch/finallyblock:Why
finallyis how JavaScript code cleans up afterawait, aroundbreak/continue, and in generators. There, cancellation (gen.return(), saga cancellation, afor..ofthat breaks early) reaches the generator only throughfinally, never throughcatch. The workaround of catching, cleaning up and rethrowing stopped being correct oncebreak/continuelanded (#8348), because they skip the copied cleanup. This PR is groundwork for generators.Language
try e finally { … }andtry e catch { … } finally { … }. The finally block needs braces, as in JavaScript.finallyis not a keyword. After acatch, it only starts a finally block when a{follows. Existing code that uses afinallyidentifier after atrykeeps its meaning.breakandcontinuecannot leave afinallyblock. JavaScript allows it, but it silently drops a pending exception or return value (ESLint'sno-unsafe-finally). Loops inside the block can still use them. If early return (Add early return with provisional extension syntax #8656) lands,returnshould be rejected the same way.Implementation
Pexp_tryandTexp_trygain anexpression option. The case list is empty when there is no catch.Ltryfinally (body, finally). Try/catch/finally is anLtryfinallyaround anLtrywith, which the backend prints as a single JS statement. This leavesLtrywithunchanged in every pass, and a try without catch needs no dummy exception ident. Optimizer passes treat it liketry: never inlined, purity requires both parts to be pure, arity comes from the body.continuethat runs the finally before the recursive call instead of after it. Add try..finally syntax #7903 had this bug._res.finallyattribute, using the same encoding asfor..of..cmiand.cmtare bumped.Answers to the review questions on #7903
finallya keyword? No, see above.unifyintypecore: gone; the finally is typed withtype_statement, like loop bodies.} catch {and} finally {on the closing brace's line.Tests
tests/tests/src/try_finally_test.res, runtime:break/continue, including from aswitch, and loops inside the finallyast-mapping/TryExpressions.res), and parse errors (tryFinally.res).break_out_of_finally,continue_out_of_finally,finally_non_unit.ExnFinally.res, covering exceptions thrown through a finally and inside one, and exceptions caught before it.🤖 Generated with Claude Code
https://claude.ai/code/session_01Fc7LEuyEfynjaKCVEBPmTF