Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7461-layer1-migrate-urlnew.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **`new URL(input, base)` is the first lowering migrated onto the Layer 1 rooting API, and the move closed a window the hand-fix left.** `FnCtx` has no interior mutability, so #7459's borrow-carrying handle cannot be built on it — the shape that works against a `&mut`-only emitter is the combinator the runtime already settled on for layer 3: `call_rooted` emits the collecting call and roots its result in one step, so no unrooted register ever exists to be held across a later collection point. Migrating the path revealed that #7453's hand-written fix left `base_ptr` live and unrooted across `js_gc_temp_root_get` — a small window, but one nobody spotted in review, which is the argument for an API over a checklist. 11/11 URL gap tests byte-identical to the Node oracle; clean under forced evacuation. One lowering: this starts the migration, it does not finish Layer 1. (#7461)
16 changes: 13 additions & 3 deletions crates/perry-codegen/src/expr/url_main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,25 @@ pub(crate) fn lower(ctx: &mut FnCtx<'_>, expr: &Expr) -> Result<String> {
// would root an already-stale pointer.
let url_slot = super::temp_root::temp_root_push_i64(ctx, &url_ptr);
let base_v = lower_expr(ctx, base)?;
let base_ptr = ctx
.block()
.call(I64, "js_url_coerce_string", &[(DOUBLE, &base_v)]);
// Layer 1 migration (#7459): `call_rooted` emits the collecting
// call and roots its result in one step, so no unrooted
// register for `base_ptr` ever exists to be held across a later
// collection point. The window that made #7453 a bug is not
// expressible here.
let base_slot = crate::rooting::call_rooted(
ctx,
I64,
"js_url_coerce_string",
&[(DOUBLE, &base_v)],
);
let url_ptr = super::temp_root::temp_root_get_i64(ctx, &url_slot);
let base_ptr = base_slot.read(ctx);
let obj = ctx.block().call(
I64,
"js_url_new_with_base",
&[(I64, &url_ptr), (I64, &base_ptr)],
);
base_slot.release(ctx);
super::temp_root::temp_root_truncate(ctx, &url_slot);
obj
} else {
Expand Down
64 changes: 64 additions & 0 deletions crates/perry-codegen/src/rooting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -238,3 +238,67 @@ mod tests {
assert_eq!(len.0, "%r0");
}
}

// ---------------------------------------------------------------------------
// Applying the design to the REAL emitter.
//
// `FnCtx` has no interior mutability -- `ctx.block()` needs `&mut` -- so the
// borrow-carrying `Raw` above cannot be built on it directly: `root(self)`
// would need a second borrow while the handle still holds the first (the same
// E0499 the RFC's own API hits, see `Raw`'s doc).
//
// The shape that DOES work against a `&mut`-only emitter is the combinator, and
// it is the same one 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, so there is no window in
// which an unrooted register exists to be misused, and `read` re-reads through
// the slot every time.
//
// This is weaker than the borrow formulation -- it prevents the bug rather than
// detecting attempts to write it -- but it needs no emitter rewrite, which is
// what makes it migratable one call site at a time.
// ---------------------------------------------------------------------------

use crate::expr::FnCtx;
use crate::types::{I32, I64};

/// A slot holding a GC-managed pointer for the duration of a lowering.
#[derive(Debug, Clone)]
pub struct RootedSlot {
idx: String,
}

impl RootedSlot {
/// Re-read the slot. Called afresh at every use: the returned register is
/// only valid until the next emission that can collect, and re-reading is
/// cheaper than reasoning about whether one has happened.
pub fn read(&self, ctx: &mut FnCtx<'_>) -> String {
ctx.block()
.call(I64, "js_gc_temp_root_get", &[(I32, &self.idx)])
}

/// Release the slot. Call after the last [`RootedSlot::read`].
pub fn release(self, ctx: &mut FnCtx<'_>) {
ctx.block()
.call_void("js_gc_temp_root_truncate", &[(I32, &self.idx)]);
}
}

/// Emit a call that can collect and root its result in one step.
///
/// The point is what this function does NOT return: an unrooted register. A
/// caller cannot hold the result across a later collection point because it
/// never has the result -- only a slot -- which is what makes the #7453 shape
/// unwritable here rather than merely reviewable.
pub fn call_rooted(
ctx: &mut FnCtx<'_>,
ret_ty: crate::types::LlvmType,
callee: &str,
args: &[(crate::types::LlvmType, &str)],
) -> RootedSlot {
let reg = ctx.block().call(ret_ty, callee, args);
let idx = ctx
.block()
.call(I32, "js_gc_temp_root_push", &[(I64, &reg)]);
RootedSlot { idx }
}
Loading