Fix #178: recompute usesJsFn after the reachability sweep (no orphan JsFn.res) - #197
Open
jagguji wants to merge 2 commits into
Open
Fix #178: recompute usesJsFn after the reachability sweep (no orphan JsFn.res)#197jagguji wants to merge 2 commits into
jagguji wants to merge 2 commits into
Conversation
…JsFn.res) The #191 reachability sweep already drops orphan records stranded by a discarded speculative build. But a bare `Function` sets `shared.usesJsFn` while such a build is in progress, and `JsFn.t` is a hand-authored raw node (not a keyed registry entry), so the key-based sweep can't see it: dropping the owner record left `usesJsFn` set and emitted an unreferenced `JsFn.res`. Reproduced: `makeBox<T>(): Box<T>` flags its return-only generic, the `Box` record (with `cb: Function`) is swept, but `JsFn.res` was still written referenced by nothing. Now `usesJsFn` is recomputed from the survivors (roots + remaining entries) after the sweep, so the emitted file set stays orphan-free. A legitimately referenced `JsFn.t` still keeps its `JsFn.res`. Fixture `speculative-jsfn-orphan` asserts the complete emitted file set (only DemoBindings.res — no JsFn.res, no orphan *Types.res). All 118 goldens compile; benchmark unchanged. Fixes #178 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Benchmark: ✅ PASS
|
commit: |
…ivor scan The post-sweep usesJsFn recompute scanned roots + entries.flatMap(entryChildTypes), but entryChildTypes's record branch omits e.indexValue — the `@set_index` value type. A SURVIVING record whose only JsFn.t is a Function-typed index signature (`[k: string]: Function` -> `@set_index …Set: (rec, string, JsFn.t)`) was therefore not scanned, so usesJsFn was wrongly cleared and JsFn.res dropped, leaving the @set_index external dangling — uncompilable. (Same class as the #195 setter/static gap: the survivor surface was narrower than what the emitter produces.) Surgical fix: add each entry's indexValue to the survivor set (kept out of entryChildTypes, which feeds deps/home/SCC and would move the bench). Fixture jsfn-function-index-reachable guards it — JsFn.res stays in the emitted file set and the output compiles. All 119 goldens compile; benchmark unchanged.
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.
Summary
#178's primary defect — speculative
@unboxedclassification stranding orphan shared records — was already fixed by the reachability sweep shipped in #191 (which is exactly the mechanism #178's own comment proposed). Its repro no longer leaks:{left} | {right}now becomes an opaqueLeaked.tmodule with both records referenced viafrom*constructors.This PR closes the one residual gap #178 explicitly called out: the
usesJsFn/JsFn.resorphan.The gap
A bare
Functionmaps toJsFn.tand setsshared.usesJsFn = true.JsFn.tis a hand-authored raw node, not a keyed registry entry, so the key-based sweep can't see it. When the type owning thatFunctionfield is swept as an orphan,usesJsFnstayed set andcli.mjsstill emitted an unreferencedJsFn.res.Reproduced on
main:The fix
After the sweep drops unreachable entries, recompute
usesJsFnfrom the survivors (roots + remaining entries): scan for aJsFn.traw node and clear the flag when none remain. A legitimately-referencedJsFn.tstill keeps itsJsFn.res.Validation
speculative-jsfn-orphanasserts the complete emitted file set (onlyDemoBindings.res— noJsFn.res, no orphan*Types.res), which is exactly what the issue asked for.npm testgreen; all 118 goldens compile;npm run bench— all 10 packages identical (surgical fix).register(cb: Function)) still emits and referencesJsFn.res.Also confirmed the other item from #178's comment —
@set_indexvalue types — is already covered (their refs go intoentry.deps, which the sweep walks).Fixes #178
🤖 Generated with Claude Code