[robobun]

What

TranspilerStateGuard in src/runtime/api/JSTranspiler.rs is the RAII guard that Bun.Transpiler's scan, transformSync and scanImports use to point the long-lived Transpiler at a per-call stack Arena/Log and put the resting state back on every exit. It held two raw pointers into the owning JSTranspiler (transpiler: *mut Transpiler<'static> and restore_log: *mut Log), dereferenced the first through an unsafe accessor in Drop, and encoded "restore macro_context or not" as Option<Option<MacroContext>>. Each of the three host functions also open-coded the same swap-in block: its own unsafe { detach_lifetime_ref(&arena) }, its own with_mut swap, a struct literal building the guard, and a comment arguing that the guard is declared after the arena and therefore drops first.

// before
struct TranspilerStateGuard {
    transpiler: *mut Transpiler::Transpiler<'static>,
    prev_arena: &'static Arena,
    restore_log: *mut bun_ast::Log,
    prev_macro_context: Option<Option<JSAst::Macro::MacroContext>>,
}

// after
struct TranspilerStateGuard<'a> {
    owner: &'a JSTranspiler,
    _arena: core::marker::PhantomData<&'a Arena>,
    prev_arena: &'static Arena,
    macro_context: MacroRestore, // Leave | Restore(Option<MacroContext>)
}

fn enter(owner: &'a JSTranspiler, arena: &'a Arena, log: *mut Log, macro_context: MacroRestore)
    -> (Self, &'static Arena)

enter performs the single remaining detach_lifetime_ref and the swap-in; Drop restores through the safe JsCell::with_mut closure on owner.transpiler and matches on MacroRestore. The 3 call sites each collapse from 17 to 19 lines to one TranspilerStateGuard::enter(self, &arena, &raw mut log, MacroRestore::..) line (transformSync still takes the previous macro_context itself and passes it as MacroRestore::Restore(prev)). The guard's transpiler_mut and restore_log_ptr helpers and both raw fields are deleted; JSTranspiler::config_log_ptr keeps its one remaining caller in Drop. Removes 4 unsafe blocks and adds 1 (net 3 fewer); one file, +76/-105 lines.

Why

The guard's type now answers what it restores and on whose behalf: it borrows the JSTranspiler it will write back to and the arena it installed, so the requirement that previously lived in three comments (the guard must die before the arena it points the transpiler at) is enforced by dropck, and the two possible macro-context outcomes are the two named variants of MacroRestore instead of a nested Option. The Log is deliberately not borrowed, because scanImports passes &mut log to the scanner while the guard is live; that one remaining ordering requirement is stated on the struct. It is zero-cost: the guard is a stack-only value (48 bytes before, 40 after, since PhantomData is zero-sized and MacroRestore has the same 24-byte layout as Option<Option<MacroContext>>), with_mut is #[inline(always)] and performs the same UnsafeCell projection the raw dereference did, and the swap-in and restore bodies are the same statements as before, executed once per call instead of being triplicated in the source.

Part of a series of small type-system hardening changes; each PR stands alone.

Verification

cargo check and cargo clippy are clean for the touched crates. Debug build succeeds. bun bd test test/bundler/transpiler/transpiler.test.js test/js/bun/transpiler/transpiler-tsconfig-uaf.test.ts test/js/bun/transpiler/transpiler-error-gc-uaf.test.ts: 191 pass, 1 skip, 21 todo, 0 fail (213 tests across 3 files).

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 15 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f318846a-281b-461e-8a59-44129f9aec06

📥 Commits

Reviewing files that changed from the base of the PR and between 97e21e5 and d08bce0.

📒 Files selected for processing (1)

• src/runtime/api/JSTranspiler.rs

---

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

LGTM — mechanical refactor that consolidates three open-coded swap-in blocks into TranspilerStateGuard::enter and drops raw pointers for a borrowed owner.

What was reviewed:

• Traced Drop before/after: owner.config_log_ptr() yields the same pointer each caller previously stored in restore_log, and with_mut on owner.transpiler performs the same UnsafeCell projection as the old raw-pointer deref.
• Checked transform_sync's split of macro_context.take() into a separate with_mut before enter — no observable intermediate state, same net effect.
• Verified declaration order at all three call sites still drops _restore before log/arena; the new 'a borrow of arena now makes dropck enforce the arena half.
• MacroRestore::{Leave,Restore} maps 1:1 onto the old None/Some(prev) arms; the deinit-before-restore of the fresh context is preserved verbatim.

Extended reasoning...

Overview

Single-file refactor of src/runtime/api/JSTranspiler.rs (+76/-105). TranspilerStateGuard — the RAII guard that swaps a per-call Arena/Log into the long-lived Transpiler for scan, transformSync, and scanImports — is reworked from two raw pointers (*mut Transpiler<'static>, *mut Log) plus Option<Option<MacroContext>> to a lifetime-parameterized struct that borrows &'a JSTranspiler and phantom-borrows &'a Arena, with a named MacroRestore enum. A new enter() constructor absorbs the swap-in logic that was triplicated at each call site; Drop restores via owner.transpiler.with_mut(..) and owner.config_log_ptr() instead of dereferencing stored raw pointers. Net -3 unsafe blocks.

Security risks

None. This is internal RAII plumbing for Bun.Transpiler; no input parsing, auth, crypto, or externally-reachable surface changes. The unsafe surface strictly shrinks.

Level of scrutiny

Moderate — it touches lifetime-laundering and Drop ordering around a stack Arena, which is memory-safety-relevant. But the change is behavior-preserving by construction: every statement in the old Drop and swap-in bodies reappears in the new enter/Drop, just routed through &'a JSTranspiler + JsCell::with_mut instead of a stored *mut. I traced each of the three call sites against the diff:

• scan / scan_imports: old prev_macro_context: None → MacroRestore::Leave; restore_log: self.config_log_ptr() at construction → self.owner.config_log_ptr() at drop (same field projection, same value). Declaration order (arena, log, _restore) unchanged, so reverse-drop still restores before the locals free — and now the 'a borrow of arena in enter makes dropck reject a reordering.
• transform_sync: previously did t.macro_context.take() inside the same with_mut closure as the arena/log swap; now it's a separate with_mut immediately before enter. Two closure-scoped projections back-to-back with no JS re-entry between them — identical resulting state. _ast_scope was already declared before _restore here, unchanged.

The one thing dropck does not enforce (the log local, passed as *mut because scan_imports needs &mut log while the guard is live) is explicitly documented on the struct, and declaration order at all three sites is correct.

Other factors

The PR description reports the existing transpiler suite (191 pass across transpiler.test.js and the two UAF regression tests) passes on the debug build. No behavioral change means no new test is required per REVIEW.md. No CODEOWNERS entry covers this path. No prior human reviewer comments on the timeline. The change follows the repo's stated preferences: dedup within the diff, named enum over nested Option, fewer unsafe blocks with a SAFETY comment on the one that remains.