fix(cfg): catch and post-labeled-break segments wrongly unreachable (#57) - #67
Merged
Merged
Conversation
) Two segment-reachability bugs in code_path.zig caused no-unreachable false positives (the JS runner reconstructs reachability from the serialized seg_reachable table, so node_reachable — a separate loop-only pass — masked them). Bug 2 (try/catch): `first_throwable_called` was set only by `throw` and `return <expr>`, so `try { bar(); return; } catch {}` (a bare throwable call + argless return) marked the catch dead. Add a `throwable` event: the parser emits it for Call/Member/New/Yield expressions evaluated in a try body (gated on a new `in_try_body` flag — a single predictable bool check in addNode), and the CFG's new makeFirstThrowablePathInTryBlock makes the catch reachable from the try entry. try_context is now saved/cleared across enterCodePath/exitCodePath so a throwable in a nested function does NOT make the outer catch reachable. Bug 1 (labeled break): makeBreakLabeled only walked loop_context, so `break A` to a labeled BLOCK (`A: { break A; } foo()`) found no target and foo() was disconnected. Add a LabelBreakContext for non-loop labels: label_open(non-loop) pushes it, label_close pops and merges the break targets with the body's normal exit into the post-label segment. Labeled loops are unchanged. Validated against the serialized seg_reachable: catch reachable for call/yield/ member, still dead for an empty try and for nested-function throwables; foo() reachable after a labeled-block break, still dead after a returning block; labeled-loop break unchanged. Full suite green; TS conformance 17910/17913 · 1210/1223; babel 1928/1928 · 1548/1548; semantic sweep 0 crashes with byte-identical scope/symbol/ref/diagnostic totals.
Per review: errdefer-restore `in_try_body` so a recoverable parse error inside a
try body doesn't leak the flag into error-recovered code (harmless — the CFG
drops the spurious throwable events — but cleaner).
Tests: the `A: { if (x) break A; bar(); }` case was a false guard (the `bar()`
fall-through made foo reachable even on buggy main); replace with the
`{ if (x) break A; return; }` form where the break is the only path to foo
(0 pre-fix → 1 post-fix). Add nested-label (`A: B: { break A; }` / `break B`),
nested-block break, member-READ and `new` throwable cases — distinct code paths
the original tests didn't discriminate.
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 #57.
Two segment-reachability bugs in
code_path.zigcausedno-unreachablefalse positives. The JS runner reconstructs reachability from the serializedseg_reachabletable, sonode_reachable(a separate loop-only pass) masked these — they're only visible in the segment graph.Bug 2 — catch unreachable when the try body ends with
returnfirst_throwable_calledwas set only by explicitthrowandreturn <expr>. A bare throwable call (bar()) + an arglessreturnset it for neither, so the catch was marked dead. Fix: a newthrowableevent — the parser emits it for Call/Member/New/Yield expressions evaluated in a try body (gated on a newin_try_bodyflag; a single predictable bool check inaddNode), and the CFG'smakeFirstThrowablePathInTryBlockmakes the catch reachable from the try entry.try_contextis now saved/cleared acrossenterCodePath/exitCodePath, so a throwable in a nested function does NOT make the outer catch reachable.Bug 1 —
break <label>to a labeled blockmakeBreakLabeledonly walkedloop_context, so a break to a labeled block found no target. Fix: aLabelBreakContextfor non-loop labels —label_open(non-loop) pushes it,label_closepops and merges the break targets with the body's normal exit into the post-label segment. Labeled loops are unchanged.Validation (against the serialized
seg_reachable)try { return; }) and for a throwable in a nested function — matching ESLint.foo()reachable afterA: { break A; }; still dead after a block that always returns; labeled-loop break unchanged.