feat(codegen): Layer 1 prototype — rooting by construction, with the RFC's API corrected - #7459
Merged
Merged
Conversation
…RFC's API corrected The RFC (docs/src/internals/rfc-rooting-by-construction.md) proposes three types and one rule, and rests on a claim it cannot check on paper: that the borrow checker rejects the #7341 bug shape. This prototypes the types and turns that claim into two doctests that cargo test executes. It also found a defect in the RFC's proposed API. It spells the constructor let obj = e.emit_call(...).root(&mut e, &mut frame); which does not compile: the returned handle already borrows the emitter, so asking for a second, mutable borrow is E0499. Corrected here by having Raw carry the shared reborrow and root(self) consume it. Anyone starting the migration from the RFC as written would have hit this on line one. Validation, and each part is checked rather than asserted: * the bug shape is rejected -- compile_fail doctest * pinned to E0499, so it cannot pass on an unrelated typo * sabotage-tested: rewriting the bad example into the good one makes that doctest FAIL, so it is not vacuously green * the correct form compiles, and emits root_store BEFORE the window and root_load AFTER it -- the ordering that every #7341 fix turned on Prototype only: not wired into any lowering path. The migration cost is the RFC's open question and is unchanged by this.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a public ChangesRooting API
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Codegen
participant RootingEmitter
participant Rooted
participant Raw
Codegen->>RootingEmitter: emit_collecting(callee)
RootingEmitter-->>Raw: return raw value
Codegen->>Raw: root()
Raw-->>Rooted: create rooted slot
RootingEmitter->>RootingEmitter: record root store and collecting operation
Codegen->>Rooted: get(emitter)
Rooted-->>Raw: return fresh raw borrow
RootingEmitter->>RootingEmitter: record root load
Possibly related PRs
✨ 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. Comment |
proggeramlug
pushed a commit
that referenced
this pull request
Aug 5, 2026
First production lowering on the layer 1 discipline, and it closed a window the hand-fix in #7453 left behind. FnCtx has no interior mutability -- ctx.block() needs &mut -- so the borrow-carrying Raw from #7459 cannot be built on it: root(self) would need a second borrow while the handle holds the first. The shape that works against a &mut-only emitter is the combinator, which is what the runtime settled on for layer 3 (RuntimeHandle::across_*): never hand out an unrooted handle at all. call_rooted emits the collecting call and roots its result in one step. Emitted window, #7453 vs now: before after coerce url coerce url root url root url mk() <- user call mk() coerce base <- UNROOTED coerce base read url root base <- closed new_with_base(url, base_raw) read url read base new_with_base(read, read) base_ptr was live and unrooted across js_gc_temp_root_get in the hand-written version. Small window, but a window -- and I did not see it when writing that fix by hand, which is the argument for the API. 11/11 URL gap tests byte-identical to node; repro clean under PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1.
proggeramlug
added a commit
that referenced
this pull request
Aug 5, 2026
* feat(codegen): migrate UrlNew onto the Layer 1 rooting API First production lowering on the layer 1 discipline, and it closed a window the hand-fix in #7453 left behind. FnCtx has no interior mutability -- ctx.block() needs &mut -- so the borrow-carrying Raw from #7459 cannot be built on it: root(self) would need a second borrow while the handle holds the first. The shape that works against a &mut-only emitter is the combinator, which is what the runtime settled on for layer 3 (RuntimeHandle::across_*): never hand out an unrooted handle at all. call_rooted emits the collecting call and roots its result in one step. Emitted window, #7453 vs now: before after coerce url coerce url root url root url mk() <- user call mk() coerce base <- UNROOTED coerce base read url root base <- closed new_with_base(url, base_raw) read url read base new_with_base(read, read) base_ptr was live and unrooted across js_gc_temp_root_get in the hand-written version. Small window, but a window -- and I did not see it when writing that fix by hand, which is the argument for the API. 11/11 URL gap tests byte-identical to node; repro clean under PERRY_GC_HEAP_LIMIT=8 PERRY_GC_FORCE_EVACUATE=1. * docs: changelog fragment for 7461 --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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.
Layer 1 of the engine plan (#7294). The plan says this layer is "largely built"; the RFC says otherwise about the part that matters, and rests on a claim it could not check on paper — that the borrow checker actually rejects the bug shape. This prototypes the three types and turns that claim into doctests
cargo testexecutes.It found a defect in the RFC's API
The RFC spells the constructor:
That does not compile.
emit_calltakes&mut selfand returns a handle carrying that borrow, soroot(&mut e)asks for a second mutable borrow —E0499. Anyone starting the migration from the RFC as written hits this on line one.Corrected by having
Rawcarry the shared reborrow, soroot(self)consumes the handle without re-borrowing:The claim, now executable
js_url_coerce_string→ lowerbase→ second coercion → use the first pointer is #7453, merged earlier today. Under these types it is a compile error:and the correct code is the shortest way out of that error.
Each part is checked, not asserted:
compile_faildoctestE0499, so it cannot pass on an unrelated typo (a barecompile_failpasses for any error, which would make it a gate that cannot fail)root_storebefore the window androot_loadafter it — the ordering that every gc(layer 3): from-space quarantine catches 55 stale dereferences across the gap suite — the instrument is in CI but aimed at one synthetic fixture #7341 fix turned on, and the thingtemp_root.rscan only state in prose today--doc: 2 passed.--lib rooting: 2 passed.Scope, stated plainly
Prototype. Not wired into any lowering path, and it does not make Layer 1 done. The migration — threading these types through
perry-codegen's emitter — is the RFC's open question and is unchanged by this. What changes is that the design is now known to work rather than believed to, and one concrete API error is out of the way before anyone starts.Summary by CodeRabbit
New Features
Tests