Fix compiled generator/async-generator try/catch routing (#628, #632); lock #617, #543#683
Merged
Merged
Conversation
…; lock #617, #543 with tests #632 (sync + async): a throw/rethrow escaping a catch/finally body did a real IL throw that bypassed an enclosing flag-based try's caughtException flag, so the outer catch never ran and the exception escaped the state machine. Route the value into the enclosing flag-based try's capture local + present flag and branch to its cleanup — running the finally(s) strictly inside that try first — via a new EmitThrowIntoEnclosingTry helper (the catch-side analog of the existing finally routing). The enclosing try is the one already tracked by _tryBodyContext / _currentTryExceptionLocal (saved before a try body, restored before its catch/finally, so during a handler it identifies the enclosing try). A new ScopeDepth field + FinallyFramesInside computes the intervening-finally chain even when that try has no finally of its own. Applied at all three propagation sites per emitter: the lexical EmitThrow handler case, the external throw() injection (EmitRoutedThrow), and the catch-less post-finally rethrow (an uncaught exception leaving a try/finally must reach an enclosing flag-based catch, not escape MoveNext). Chain-empty stores directly; chain-nonempty (a finally may yield/await) holds the value in <>pendingException and moves it via a routing terminal. #628 (async analog of #619): port the exceptionPresentLocal boolean gate to AsyncGeneratorMoveNextEmitter. A thrown/rejected null/undefined captures as a null CLR reference, which the value-nullness Brfalse gate misread as "no exception", skipping the catch and dropping the post-finally rethrow. The present flag records presence independent of the value; the #617 await-rejection routing now sets it too. #617 and the compiled side of #543 were already fixed on main (the issues were stale/unclosed): #617's rejected-await-in-try routing and #543's generator catch identity both behave correctly. Add regression tests that lock the behavior — new GeneratorErrorIdentityTests for #543, and the compiled re-entrancy test now asserts `instanceof TypeError` in-body. Found and filed two out-of-scope gaps: #675 (a real exception escaping a nested real-IL try/catch inside a flag-based catch body still escapes an enclosing flag-based try — a distinct mechanism, not a direct handler throw) and #676 (interpreter property access on undefined throws a raw "Runtime Error" string instead of a guest TypeError). Tests: full SharpTS.Tests suite green (12714 passed). New cross-mode regression tests in GeneratorTryFinallyTests (#632), AsyncGeneratorTryFinallyTests (#628 + #632, CompiledOnly per the #564 eager-drain ordering), GeneratorErrorIdentityTests (#543), plus IL-verification guards. Closes #628, closes #632, closes #617, closes #543.
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
Fixes a cluster of compiled-mode generator / async-generator
try/catchbugs, all in the flag-based state-machine emission used when ayield/awaitcrosses a protected region. The interpreter already handled every case; these bring compiled mode to parity with Node.throw/rethrow from inside a catch/finally body isn't caught by an enclosing flag-basedtrynull/undefinedinto a flag-basedtryskips the catchawaitinside atryisn't caught by that try's catchmain; locked with testsinstanceof TypeErrorisfalsefor an error caught inside a compiled generator bodymain(compiled); locked with tests#632 — handler-body throw routing (the substantive fix)
EmitThrowrouted a handler-bodythrowonly through enclosing finally frames and then did a real ILthrow, which bypasses an outer flag-based try'scaughtException/present flag — so the outer catch never ran and the exception escapedMoveNext/MoveNextAsync.The fix routes the value into the enclosing flag-based try's capture local + present flag and branches to its cleanup, running the finally(s) strictly inside that try first — a new
EmitThrowIntoEnclosingTry, the catch-side analog of the existing return/throw finally routing._tryBodyContext(sync) /_currentTryExceptionLocalet al. (async): these are saved before a try body and restored before its catch/finally, so while emitting a handler they identify the enclosing try.ScopeDepth(=_exitScopes.Countat try-body start) +FinallyFramesInsidecomputes the intervening-finally chain correctly even when the enclosing try has no finally of its own (so no stack marker).EmitThrowhandler case, the externalit.throw()injection (EmitRoutedThrow), and the catch-less post-finally rethrow (an uncaught exception leaving atry/finallymust reach an enclosing flag-based catch, not escape the state machine).<>pendingExceptionacross the finallys and move it via a routing terminal.#628 — present-flag gate (async analog of #619)
A thrown/rejected
null/undefinedcaptures as a null CLR reference, which the old value-nullnessBrfalsegate misread as "no exception". Ported the dedicatedexceptionPresentLocalboolean gate (catch gate, segment-skip, post-finally rethrow) intoAsyncGeneratorMoveNextEmitter; the #617 await-rejection routing now sets it too.#617 / #543 — already fixed, now locked
Both behave correctly on
mainbut the issues were never closed and lacked targeted coverage. Added regression tests (newGeneratorErrorIdentityTestsfor #543; the compiled re-entrancy test now assertsinstanceof TypeErrorin-body) and updated the stale#543deferral comment.Gaps found and filed (out of scope here)
try/catchinside a flag-based catch body still escapes an enclosing flag-based try (a distinct mechanism: an exception already in flight, vs. a directthrowstatement, which this PR fixes). Needs catch/finally bodies wrapped in capture-and-route sync segments.undefined/nullthrows a raw"Runtime Error"string instead of a guestTypeError(broader, not generator-specific; surfaced while confirming instanceof TypeError is false for errors caught inside a compiled generator body #543).Testing
SharpTS.Testssuite green: 12714 passed, 0 failed, 0 skipped.GeneratorTryFinallyTests(Compiled generator: a throw from inside a catch body is not caught by an enclosing flag-based try/catch (sync + async) #632 — rethrow, intervening finally, catch-less try/finally, throw-from-finally, two nested finallys, externalthrow()at a catch-body yield),AsyncGeneratorTryFinallyTests(Compiled async generator: throwing null/undefined into a flag-based try/catch skips the catch (async analog of #619) #628 + Compiled generator: a throw from inside a catch body is not caught by an enclosing flag-based try/catch (sync + async) #632,CompiledOnlyper the Interpreted async generator: body side effects run out of order vs for-await-of consumer (eager drain) #564 eager-drain ordering),GeneratorErrorIdentityTests(instanceof TypeError is false for errors caught inside a compiled generator body #543), plus ILVerify (CompileAndVerifyOnly) guards for every new shape.Closes #628, closes #632, closes #617, closes #543.