Next.js 16 standalone app-router bring-up — walls 1–5 (codegen/runtime/HIR fixes)#5438
Merged
Conversation
added 30 commits
June 17, 2026 06:33
…dle target (v0.5.1173)
Next.js patchSetHeaderWithCookieSupport calls
Object.defineProperty(res, PATCHED_SET_HEADER, {value:true}). Perry models the
http ServerResponse as a native HANDLE (small pointer-tagged id), not a heap
object, so js_object_define_property threw 'called on non-object', aborting the
request (HTTP 500). Localized via backtrace: define_property <- patch_set_header
<- Server.handleRequestImpl.
FIX: a native-handle target no longer throws. String-key data descriptors route
through the handle property-set dispatch (handle[key] round-trips); symbol keys /
accessors degrade to a best-effort no-op (the framework patch is idempotent).
Returns the target instead of throwing. Verified on real Next.js: defineProperty
TypeError gone, request advances to a deeper invariant (separate wall).
…parent ctor (v0.5.1174) class NodeNextRequest extends _index.BaseNextRequest: super() was a no-op, so BaseNextRequest's ctor (sets this.url/this.method) never ran → req.url undefined → 'Invariant: url can not be undefined'. The named member-extends HIR arm set only static extends_name and relied on the parent being in codegen's class table (module-order-dependent); only the .default arm routed through extends_expr. FIX: route named member-extends through extends_expr unconditionally (like .default + the unknown-Ident arm) so super() runs the parent ctor at runtime via js_fetch_or_value_super (tolerates native/closure/classref/builtin parents). Static extends/extends_name kept when resolvable; colliding-name native case (class Agent extends http.Agent) still handled by the parent_name==name arm. Verified real Next.js: BaseNextRequest ctor runs, req.url/method resolve, invariant gone, request advances into routing/render.
…ijacked on user-class instances (v0.5.1175)
…module parent dropped construction args
A no-own-ctor class with heritage inherits the spec default ctor
constructor(...args){super(...args)}. When the parent is a cross-module
class resolved at runtime as a dynamic parent (class Child extends
_mod.Parent {} -> extends_expr + js_register_class_parent_dynamic), the
synthesized standalone ctor dropped every construction arg, so the parent
ctor ran with garbage params. Next.js route matchers (PagesRouteMatcher,
AppPageRouteMatcher, AppRouteRouteMatcher) never received their definition
-> RouteMatcher(definition) left this.definition undefined -> reload()
threw 'Cannot read properties of undefined (reading pathname)' -> HTTP 500.
- artifacts.rs: synthesized_ctor_param_count helper resolves the forwarding
arity from the nearest ancestor ctor; falls back to a fixed band (8) when
a non-native cross-module parent's arity is unavailable (auto-optimize
compiles nested modules with empty imported_classes). Native parents keep 0.
- string_pool.rs: register CLASS_CONSTRUCTORS total_params from the same
helper (ctor_arity_overrides) so the registered arity matches the emitted
forwarding-ctor signature (was 0 for no-own-ctor classes).
- method.rs: route a dynamic-parent (extends_expr) synthesized super through
js_fetch_or_value_super instead of the wrong-prefix inline static call.
… on class instances + runtime require(.json) Wall 52: `.match(arg)` was folded to String.prototype.match based only on the ARG being possibly-regex, never checking the RECEIVER is a string. A class instance with a user `match(pathname)` method (Next.js RouteMatcher) called through a shared call site `m.match(p)` (m: any) compiled to js_string_match(m_as_string, p), reinterpreting the instance pointer as a string and returning null — so DefaultRouteMatcherManager never matched the App-Router root '/'. Fix: only fold to StringMatch/StringMatchAll when the receiver is statically a string; Any/class receivers fall through to the runtime dispatcher (which handles string .match via is_string() and class methods). Mirrors the wall-50 normalize fix. Wall 53: runtime `require(absolutePath.json)` (Next.js loads manifests via `require(this.middlewareManifestPath)`) threw MODULE_NOT_FOUND — the CJS wrapper's require only handles compile-time-static specifiers. Added a .json disk fallback: new js_require_json_disk runtime fn (readFileSync + JSON.parse, throws MODULE_NOT_FOUND when absent) reached via a __perry_require_json_disk intrinsic (globals.rs -> __perry_runtime NativeMethodCall -> codegen FFI), called from the wrapper require for absolute .json paths. .js/.node runtime require stays unsupported (would need eval).
…e registry (Next.js wall 54, part 1) Next.js/turbopack load page + chunk modules via a runtime `require(path)` computed from a manifest, not a static specifier. Perry resolves static requires at compile time, so a runtime absolute-path require threw MODULE_NOT_FOUND even for modules it had compiled. Adds a path->module-exports registry: every compiled CJS module self-registers its exports under its canonical absolute source path at init (cjs wrapper tail), and the wrapper's require resolves an unmatched absolute path through the registry before the .json disk read / MODULE_NOT_FOUND throw. - runtime: js_register_path_module / js_require_path_module (canonicalized key) - intrinsics: __perry_register_path_module / __perry_require_path_module (globals.rs -> __perry_runtime NativeMethodCall -> codegen FFI) - wrap.rs: self-register at module init + path lookup in require Verified: a module statically required (compiled) is also resolvable via require(absolutePath.js) with identical exports ref, matching node. This is the foundation; compile-time discovery of the dynamically-required module set (part 2) is still needed for the page modules to be in the registry.
`is_in_node_modules` used `canonical.contains("node_modules")` — a substring
match that misclassifies a non-node_modules .js whose FILENAME contains
"node_modules" (e.g. turbopack's bundled chunks
`.next/server/chunks/ssr/node_modules_next_dist_…._.js`) as a node_modules JS
module, force-routing it to the removed V8 runtime -> hard 'JavaScript runtime
(V8) support has been removed' build error. Check for a real `node_modules/`
directory component instead so such files compile natively (Next.js wall 54
groundwork: lets the turbopack server chunks AOT-compile).
…t eagerly at module init A class declared inside a function/arrow body (e.g. turbopack chunk factories) had its `static x = ...` field initializers emitted into the enclosing MODULE's init, so they ran eagerly when the chunk module was loaded — even though the factory closure was never called. React-SSR turbopack chunks have nested classes whose static initializers run arbitrary code (`static #a = this.EMPTY = new z(...)`); running them eagerly at module load threw `TypeError: Cannot convert undefined or null to object` while loading a 209KB chunk. Add `Class.is_nested` (set when lowered at scope_depth>0 / inside a block scope) and skip nested classes in init_static_fields_early/late. The function-body decl path (body_stmt.rs) already emits static-field init into the function body for non-pre-hoisted classes, so they now initialize lazily when the declaration is evaluated — matching JS.
…server page/chunk modules by path
A Next.js standalone server loads its page/route/chunk modules from
`<entry_dir>/.next/server/**` via a path computed at request time
(`require(getPagePath(...))`, turbopack `R.c("chunkpath")`) — never via a
static import — so the import walk never reached them and they were not
AOT-compiled. The server reached request handling but every render threw
`Error: Cannot find module '.next/server/app/page.js'`.
Discovery: when the entry sits beside a `.next/server` directory,
collect_modules seeds every `.next/server/**/*.js` as an additional
compile root (75 modules in the demo; 669 -> 747). They self-register
under their absolute path at init (part 1's
`__perry_register_path_module`).
Lazy loading: the `.next` modules stay Deferred (eager-initing turbopack
chunks at startup runs React-SSR code before the server is ready). The
entry's `main` records each module's `<prefix>__init` address by path
(new `js_register_path_init` + `MODULE_PATH_INIT_REGISTRY`); the first
runtime `require(absolutePath)` whose exports aren't registered yet
triggers that init lazily (which self-registers exports and may
recursively require its chunk dependencies in `R.c()` order). All
registry locks are released before calling init to allow re-entry.
With this the server boots and the page-load path fires correctly on
request (verified: the trigger reaches `loadComponents` -> `requirePage`
-> the page module's init). Executing the turbopack chunk/page code
itself is the next wall (a SIGSEGV in the generated React-SSR code).
Gated entirely on the `.next/server` sibling directory, so non-Next.js
builds are byte-identical. Runtime additions are additive.
… it to undefined
SetAdd wrote js_set_add's return value back to the variable's storage to
"see the possibly-realloc'd pointer" — but js_set_add ALWAYS returns the
same SetHeader pointer (ensure_capacity reallocs only the internal
elements buffer, never the header). The writeback was vestigial (copied
from the array-push realloc pattern) and actively wrong for a
boxed/mutable closure capture: it called js_closure_set_capture_f64,
overwriting the capture SLOT (which holds a box pointer) with the Set
value, so the next read dereferenced the Set-as-box and got undefined.
Repro (CJS module, the Next.js turbopack runtime shape):
const loaded = new Set();
function lrp(p){ loaded.add(p); } // loaded captured by closure
module.exports = (s) => ({ c: (p) => lrp(p) });
// caller: const R = require(mod)('x'); R.c('a'); R.c('b');
After the first R.c, `loaded` read as undefined; the second R.c
SIGSEGV'd in js_set_add. This silently cleared the turbopack runtime's
module-level `loadedChunks` Set, crashing Next.js page/chunk loading.
Fix: drop the writeback entirely — the set is mutated in place and the
header pointer is stable; GC moves are handled by root rewriting of the
variable slot, not here. Map.set was already correct (no writeback).
`{ 900(e,t,r){} }` — an object-literal method shorthand with a numeric
key — fell through `lower_method_prop`'s key match to the `_ => Ok(None)`
arm, so the method was dropped entirely: `obj[900]` returned undefined
and the key never appeared in `Object.keys`. `{ 900: function(){} }`
(numeric key, value form) and `{ "900"(){} }` (string-keyed shorthand)
both worked, so this only hit the numeric-key + shorthand combination.
webpack/turbopack module-factory tables use exactly this shape
(`var r = { 900(e,t,r){…}, 781(e){…} }` then `r[900].call(…)`), so a
bundle's numeric-keyed factories vanished — e.g. Next.js's
`app-page-turbo.runtime.prod.js` entry `a(900)`.
Add the `PropName::Num` arm mirroring the KeyValue / closed-shape paths
(`number_to_js_key`), so the key stringifies per spec (`{900(){}}` has
own key "900").
… per-object flag
`PROPERTY_DESCRIPTORS` / `ACCESSOR_DESCRIPTORS` are keyed by raw object
address. When an object is freed and its address is reused by a fresh
allocation, a stale `(addr, key)` descriptor entry from the previous
tenant would be read back for the new object — reporting a plain `{}`'s
property as a getter-only accessor or `writable: false` and falsely
throwing "Cannot assign to read only property" on an ordinary write.
Each descriptor install already sets `OBJ_FLAG_HAS_DESCRIPTORS` in the
object's GcHeader (via `note_descriptor_target`, for `GC_TYPE_OBJECT`),
and a fresh allocation's `_reserved` is zeroed. Gate the hot get/set
descriptor lookups on a new `object_has_descriptors()` helper so a fresh
object with the flag clear skips the stale side-table read entirely:
- `field_set_by_name`: accessor short-circuit + per-property writable check
- `field_get_set`: plain-object accessor get/set short-circuits
- `proxy::own_set_descriptor`: the [[Set]] walk used once a descriptor on
Object.prototype disables the plain-object fast path process-wide
(closures keep consulting the side tables — they don't carry the flag).
Addresses the stale-descriptor-on-address-reuse class of false read-only
throws.
…ained member-init
A simple-ident var bound to a chained member-assignment whose RHS is an
object literal miscompiles in the full-bundle context: the constructed
object's own field reads back as 0 when the construction flows directly
into both the member store and the binding. A directly-bound object
literal (`var x = {exports:{}}`) is fine, so hoist the construction to
its own `Let` and feed the member-set and the binding from that temp —
a semantics-preserving rewrite of `var i = n[e] = {exports:{}}` into
`var tmp = {exports:{}}; n[e] = tmp; var i = tmp;`.
This unblocks Next.js's app-page-turbo webpack runtime: its
`__webpack_require__` does `var i = cache[e] = {exports:{}}` then passes
`i.exports` to the module factory; the corrupted `exports = 0` made
React's `exports.Fragment = …` throw "Cannot assign to read only
property 'Fragment'". Together with the descriptor-flag fix
(prior commit) the page module now loads past the Fragment wall.
The is_nested field on perry_hir::Class (nested-class eager-static-init fix) was missed on the inliner's test-helper Class literal, breaking `cargo test -p perry-transform`. Add is_nested: false.
A fresh var/let/const NAME = ... binding inherited a native-instance tag
(FormData/Response/Headers/http.Server/...) that an UNRELATED earlier
binding of the same NAME registered. ctx.native_instances is module-global,
keyed by name, last-match-wins; a minified webpack bundle that binds a local
i via new FormData() in one factory and reuses var i = { exports:{} } as the
require-cache object in another leaked the FormData tag into the second i.
The plain read i.exports then lowered to a FormData NativeMethodCall (no such
member -> folds to 0), so React's exports.Fragment = ... ran against 0 and
threw "Cannot assign to read only property 'Fragment'" -- the long-standing
Next.js app-page-turbo wall.
Fix: push a tombstone (empty module) for the name before the native-instance
registration checks in the simple-ident var-decl path; a genuinely-native
init re-registers after it (last-match-wins keeps the real tag), and
lookup_native_instance resolves a tombstoned name to None so the read/call
lowers as an ordinary property access. The tombstone lives in the
scope-truncated native_instances vec, so it does not leak past the binding's
function.
Validated: Next.js app-page-turbo bundle no longer throws the Fragment error;
native http server (createServer/listen/close/address) and EventEmitter still
dispatch correctly; perry-hir tests pass.
lower_fn_decl did `lookup_func(name).unwrap_or_else(|| fresh_func())` but the fresh_func() branch never called register_func, so a function declaration whose name was not pre-hoisted (common for deeply-nested decls inside closures, e.g. minified webpack-bundled modules) had its own name unresolvable in its body. A self-reference or member read on the function name then lowered to js_global_get_or_throw_unresolved -> "ReferenceError: <name> is not defined". Register the name when it was not already pre-registered; the registration lives in the enclosing scope's function table (scope-truncated at exit), so it is visible for recursion and sibling references without leaking. perry-hir tests pass.
lower_method_prop lowered object-method bodies via lower_block_stmt (a plain
block: no function-declaration name pre-registration), while every other
function-body path (lower_fn_decl/lower_nested_fn_decl/lower_fn_expr) uses
lower_fn_body_block_stmt, which Phase-1-hoists nested function-declaration
names so forward references resolve. Minified webpack bundles emit module
factories as object METHOD shorthand (`{"./mod"(e,t,r){ ...; r.d(t,{x:()=>o});
... function o(){} }}`), so the export getter `()=>o` is a forward reference
to a `function o` declared later in the same method body. Without Phase-1
hoisting that reference lowered to an unresolved global -> "ReferenceError: o
is not defined" on the Next.js page-module load. Route method/accessor bodies
through lower_fn_body_block_stmt (both call sites). perry-hir tests pass.
A `var` declared LATER in a function body but referenced by a closure created
EARLIER must be captured by reference (the closure sees the later assignment).
predefine_var_bindings_in_function_body already hoists+boxes such `var`s, but
their box was never added to the function-entry prealloc set — only let/const
forward-captures (Phase 1.6) were. So the earlier closure captured a
TAG_UNDEFINED snapshot instead of the live box, and saw `undefined` after the
`var` was assigned. This is the webpack/turbopack ESM re-export shape used in
Next.js' react-server.node.js: `r.d(t,{x:()=>n.x}); var n=r("…")` — the export
getter `()=>n.x` is created before `var n` is assigned. Extend
pre_register_forward_captured_lets to also add forward-captured `var` ids to
forward_boxed_ids (prealloc). Repro: `function f(){const g=()=>n; var n=42;
return g()}` returned undefined, now 42. perry-hir tests pass.
Phase-1 function-declaration hoisting in lower_fn_body_block_stmt reused ANY
existing local of the same name via lookup_local, which searches every scope.
A nested `function a` therefore reused an OUTER-scope `a` binding's local/box
instead of shadowing it with a fresh one. When the outer binding is a
closure-captured variable, the nested declaration overwrites the captured box
at runtime: in the Next.js app-page turbopack bundle the webpack chunk's
`function a` require is captured by an inner IIFE, and react-dom's module-local
`function a` (React's prod error formatter) reused that same box — so after
react-dom was required, `a("…/superstruct/index.cjs")` invoked React's error
formatter instead of the require, returning the string "Minified React error
#…/superstruct/index.cjs". That string flowed through `a.n` and a later 0-arg
call threw "value is not a function" during app-page module init (HTTP 500).
Add lookup_local_in_current_scope (locals at/after the current enter_scope
mark) and use it for the hoisting reuse decision so a same-named outer local is
shadowed (fresh local + box) per JS scoping. A sibling `var`/`function` in the
SAME body still merges. perry-hir tests pass; var-forward-capture + nested
re-export repros unaffected.
Identifier resolution checked lookup_local (which searches every scope) before
lookup_class, so a nested `class a` whose name also exists as a binding in an
OUTER scope resolved to that outer binding instead of the class. In the Next.js
app-page turbopack bundle a webpack chunk-level `a` (`a=()=>{}`, undefined when
the module factory runs at init time) is captured into p-timeout's module,
which declares `class a extends Error` (its TimeoutError) and exports it via
`e.exports.TimeoutError=a` — that read resolved to the outer `undefined`, so
`new r.TimeoutError` threw "undefined is not a constructor". When the name is a
class declared in the current function body (forward_class_names, lexically
saved/restored per body) and there is no current-scope local of that name,
resolve the identifier to a ClassRef so it shadows the outer local. A sibling
param/var/let in the same scope still wins. perry-hir tests pass; class-shadow
repro fixed.
…em to import_helpers)
The Fragment fix (e3665b330) tombstoned a leaked native-instance tag when a
VAR-DECL rebinds a name that an earlier `new FormData()`/`new Response()` (etc.)
in another factory registered (native_instances is module-global, name-keyed,
last-match-wins). Function PARAMETERS were not covered: a minified
`function(e){…}` whose param `e` collides with an earlier `e = new Response()`
inherited the stale `"e"→native` tag, so `e.<method>` lowered to a
NativeMethodCall on the stale module → folded named reads to 0. In the Next.js
app-page bundle superstruct's `enums(e){ let r=e.map(x=>a(x)).join(); … }` saw
`e.map`/`e.length`/`e.constructor` ALL read 0 (while `e[0]` and
`Array.prototype.map.call(e)` worked — the array was fully valid) → `e.map(…)`
went through number-method dispatch → 0 → `.join` on 0 → "(number).join is not
a function" during app-page module init. Add `shadow_native_instance_if_present`
and call it at every parameter binding site (fn decl / fn expr / arrow / object
method / class method / private method / nested fn decl) so a param shadows any
live same-named native-instance tag, exactly like a var-decl. perry-hir tests
pass (158); class-shadow + superstruct repros unaffected.
…odule
Class references are name-keyed (`Expr::New { class_name }` / `ClassRef(name)`
resolve by name at codegen), and the body-stmt dedup skips a `class X` whose
name is already registered. In a minified single-module bundle (Next.js
app-page) many DISTINCT classes share a name across nested webpack/ncc factory
scopes — `class s`x3, `class a`x5, etc. superstruct's `Struct` (`class s`, 6
members) was the 3rd `class s`, so the dedup SKIPPED it and `enums()`'s
`new s({...})` bound to an unrelated `class s` (16 members) whose ctor read a
property on undefined -> "Cannot read properties of undefined (reading 'b')"
during app-page module init.
Add scope-local class-name aliasing: when a function body (or function-
expression body) declares `class X` while an outer/prior `class X` is already
registered, rename this body's X to a unique `X$<n>` and record `X -> X$<n>` in
a `class_renames` map (saved/restored per body, mirroring `forward_class_names`)
so the declaration and every in-body reference (`new X`, `ClassRef`, dedup)
bind to the lexically-correct class. Detection runs in BOTH Phase-1.5 class
scans — `lower_fn_body_block_stmt` AND `lower_fn_expr` (the cjs/ncc
`(function(e){…class s{…}…})(t)` IIFE path, where Struct lives). Functions are
unaffected (already id-based via `FuncRef(FuncId)`); non-colliding classes are
unaffected (alias is identity), so blast radius is limited to duplicate-named
classes that were previously broken. perry-hir tests pass (158); superstruct +
class-shadow repros pass; app-page bundle advances past the wall.
…a real handle Next.js' standalone server sets globalThis.AsyncLocalStorage = AsyncLocalStorage (node-environment-baseline.js) then constructs storage via new maybeGlobalAsyncLocalStorage() (async-local-storage.js). The dynamic callee misses the static `new AsyncLocalStorage()` codegen arm, so js_new_function_construct fell through to a class_id=0 empty object whose .getStore read back undefined -> `TypeError: getStore is not a function` at server startup (io-utils.js io()). Add an async_hooks arm to js_new_function_construct mirroring the #4995 EventEmitter dispatch: a new JS_NATIVE_ASYNC_HOOKS_CONSTRUCT pointer, registered by perry-stdlib at startup, routes a bound async_hooks.AsyncLocalStorage / AsyncResource ctor value to the real stdlib/runtime handle constructor. Also bind AsyncLocalStorage method-VALUE reads (als.getStore / als.run as values, not zero-arg calls) for statically-typed receivers (expr_member.rs arm + dispatch_async_local_storage_property) so the read returns the callable bound method rather than invoking getStore() with no args. Verified: server now passes the getStore startup wall (advances to a later new-Function wall in depd/send); static and dynamic ALS run/getStore match Node.
…le time depd's `wrapfunction` (bundled in `send` -> Next.js) builds its deprecation wrapper with a runtime-constructed body, so the eval classifier bucketed it RuntimeUnknown and (in defer mode, PERRY_ALLOW_EVAL) compiled it to a throw-on-call value. `send` invokes the wrapper eagerly at module init, so the Next.js standalone server crashed at startup (`new Function() cannot run in an ahead-of-time compiled binary`) before printing its banner. The runtime `js_function_ctor_from_strings` already recognizes this exact template and returns the wrapped fn. Recognize the same shape at HIR-lowering time (5 verbatim param-name string-literal args + the three body marker substrings collected from the concatenation skeleton) and let it PROCEED to that recognizer instead of deferring. Any other runtime-unknown body still defers. NO general eval/interpreter. Verified: with this + the async-hooks dynamic-new fix, the Next.js server now boots and prints its banner (advances to a later path-dispatch wall).
…ty read
The CJS-require shim lowers `require("path")` to
`PropertyGet { NativeModuleRef("path"), "default" }` (a property read), not a
bare `NativeModuleRef`. The devirtualized native-module dispatch bucket is
installed (js_nm_install_<mod>) only by the bare-NativeModuleRef value lowering
in static_field_meta, so the require-then-`.default.<method>()` shape never
registered the bucket: nm_dispatch_lookup fell to its None arm and every method
call returned undefined (Next.js' `_path.default.join(...)` -> undefined ->
distDir/buildIdPath undefined -> fs ERR_INVALID_ARG_TYPE at server startup).
Properties (path.sep) worked because they resolve via the constants path, not
the method dispatcher.
Emit nm_install_symbol(module) in the NativeModuleRef property-read arm too,
mirroring the bare-ref path. Idempotent at runtime; also keeps the bucket's
handlers alive against the auto-optimize dead-strip.
Verified: `require("path")`-only (no static import) .default.join now returns
the joined path; the Next.js server advances past the filesystem-setup wall.
Per ECMA-262 SerializeJSONProperty, JSON.stringify(value, replacer) must call
the replacer with `this` = the holder (the containing object/array; for the
root, the `{ "": value }` wrapper). Perry's call_replacer invoked the closure
via js_closure_call2 with no receiver, so `this` was wrong for every key
(verified: a 4-key object gave root/isA/wrapper all false vs Node's
wrapper/root/isA/root). Replacers that depend on the holder broke — notably
React's Flight reply encoder keys its dedup Maps by `this`
(referenceMap.get(this)), and `this[key] instanceof Date` checks.
Set the implicit `this` to the holder around the replacer call (mirrors the
reviver path's internalize_json_property), threading the holder through
call_replacer: the object/array being walked for nested keys, and a freshly
built `{ "": value }` wrapper (GC-safe, like apply_reviver) for the root.
Verified byte-identical to Node across the JSON.stringify suite (replacer
this-binding, array/function replacers, pretty-print, Map-cycle replacer).
…rototype, not self
js_object_get_prototype_of fell through to its `return obj_value` self-prototype
fallback for native-module namespace objects (the `{__module__}`-tagged
NATIVE_MODULE_CLASS_ID object that `require("path")` etc. produce), so
Object.getPrototypeOf(mod) === mod. Turbopack's `interopEsm` walks the import's
prototype chain — `for (cur = raw; !LEAF_PROTOTYPES.includes(cur); cur =
getProto(cur))` — to mirror getters for every inherited key; a self-returning
getProto makes that loop never advance, so it allocates export getters
(`createGetter` = `()=>obj[key]`) forever. That was the Next.js 16 standalone
startup runaway: after the banner the server span at 100% CPU, RSS climbing
~27MB/s, never reaching `✓ Ready`.
Return %Object.prototype% (a LEAF_PROTOTYPE) for NATIVE_MODULE_CLASS_ID objects
in both heap-pointer branches so the chain walk terminates. Verified: the
native-module proto chain now ends at Object.prototype→null (no self-loop), and
the Next.js standalone server boots to a stable 193MB RSS and serves requests
(now a normal HTTP 500 `cacheSignal is not a function` render error, not a hang).
…osure_ptr guard, rest-param construct ABI, url string-method gating - native/mod.rs + native_call_method.rs + stdlib_ffi.rs: null-safe native-method fallback (cacheSignal e.indexOf 0.0-sentinel mis-classification) - closure/dynamic_props.rs: is_closure_ptr heap-range guard (SIGSEGV on mis-boxed 0x400000000) - string_pool.rs + class_constructors.rs x2: register class-ctor rest params in CLOSURE_REST_REGISTRY + pack rest array before positional caps in both replay paths (member-new super-spread 0x400000000) - expr_member.rs + native_module.rs: skip unimplemented-module gate for String.prototype methods on module-named locals (url.endsWith) wall#5 (readFileSync ENOENT) NOT included — Perry can't propagate the runtime throw through Next's manifest loader without crashing; needs deeper async-exception work.
# Conflicts: # crates/perry-hir/src/lower/context.rs # crates/perry-hir/src/lower_decl/block.rs
added 13 commits
June 18, 2026 10:12
…ndex to undefined, not SIGSEGV NaN-boxed receivers whose tag isn't POINTER (0x7FFD) or STRING (0x7FFF) are primitives (INT32 0x7FFE, BIGINT 0x7FFA, undefined/null/bool 0x7FFC). Indexing them is `undefined` per JS (`(983055)[0] === undefined`). Previously the int payload was treated as a heap pointer and the GcHeader at raw-8 was read → wild deref → SIGSEGV (Next.js app-page-turbo render crash on a NaN-boxed-int receiver 0xf000f). Reject non-pointer/string NaN-boxed receivers up front (cross-platform); also guard the raw-pointer path with is_valid_obj_ptr.
… longer mis-resolve to node modules)
native_modules_index is module-global + first-match-wins with no scope
tracking, so a local binding (param or `const`) named the same as a
registered native module (`url`, `util`, …) resolved to the node MODULE
everywhere — firing the unimplemented-API gate on its ordinary methods.
Next.js app-page-turbo hit this en masse: 88x `url.push`, 84x `util.destroy`,
and the render-blocking `url.o` deferred throw (a local `o` arrow, a local
`url` array, undici's own `util`, etc.).
Add a per-scope module-shadow stack mirroring the native-INSTANCE shadowing
(shadow_native_instance / truncate): push at param + var-decl binding sites,
truncate in exit_scope. The var-decl case is conditional — it does NOT shadow
a decl whose init is `require('<node-core>')` (the real module binding).
Drops the app-page-turbo unimplemented-API gates 640 -> 28 and clears the
url.o render throw.
# Conflicts: # crates/perry-runtime/src/module_require.rs
…_id (LocalId parallel to #5143) compute_max_func_id scans class field/static-field initializers and the extends expression (the #5143 FuncId fix), but the parallel LocalId scan in compute_max_local_id did not — a field-initializer closure's param/body LocalIds were invisible, so the generator/async transform could synthesize state/done/sent LocalIds that COLLIDE with a field-init local and corrupt unrelated codegen (e.g. a module-binding/capture read resolving wrong). A too-high max is always safe; a missed id collides. Adds a regression test.
…nstead of throwing A class object (OBJECT_TYPE_CLASS) reaching js_native_call_value — e.g. `new s.RequestCookies(headers)` where the dynamic callee resolves (through a webpack lazy-export getter) to a class object but the construct site lowered to a call — hit `func_ptr.is_null()` and threw `value is not a function`. The construct path (js_new_function_construct) already handles class objects; the call path did not. Route a class object in the call path to construction (its only sensible meaning). Unblocks the Next.js app-page-turbo render past the RequestCookies wall.
…ue-call
Refines the prior class-object-in-call-path fix. In the Next.js app-page-turbo
bundle, `new s.RequestCookies(headers)` lowers as `new (s.RequestCookies())(headers)`:
the member callee evaluates as a 0-arg value-call and the OUTER `new` carries the
args. Constructing at that 0-arg inner call (prior behavior) built the instance
with no args -> the constructor read 'cookie' off undefined headers
('Cannot read properties of undefined (reading get)'). Instead, return the class
object on a 0-arg value-call so the outer `new` constructs it with the real args;
a value-call WITH args still constructs directly. Clears both the RequestCookies
value-not-fn (W2) and undefined.get (W4) walls in the Next.js render.
…mpty string)
A failed readFileSync returned an empty string instead of throwing. Callers that
try/catch a missing file then fall back (e.g. Next.js loadManifest returns {}
when an optional manifest like subresource-integrity-manifest.json is absent)
never saw the error — the empty string flowed into JSON.parse('') and threw a
misleading 'Unexpected end of JSON input'. Throw a Node-shaped fs error
(code/errno/syscall/path) — a real catchable JS exception, not the null-deref the
prior empty-string workaround guarded against — so try/catch behaves as node.
…tructed anonymous class expressions
`new class { m() { return outer } }()` registered a synthesized class whose
constructor takes one param per captured enclosing-scope local (method bodies
rewritten to read `this.__perry_cap_<id>`), but the directly-constructed
anonymous form passed only the user's args to the New — cap params received
`undefined`, so methods reading a captured local saw `undefined`. Mirror the
named-class `new C()` path: append each capture as `LocalGet(id)`. Refs Next.js
bundled tracer (getActiveScopeSpan -> trace.getSpan on undefined destructured trace).
# Conflicts: # crates/perry-hir/src/lower/context.rs # crates/perry-hir/src/lower/expr_call/regex_string.rs # crates/perry-hir/src/lower/lowering_context.rs
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR implements Next.js "wall 54 (part 2)" deferred-module path-based lazy initialization end-to-end, adds ChangesNext.js Wall 54 + Nested Statics + Shadowing + Ctor Rest + AsyncLocalStorage + Descriptor Gating + Runtime Fixes
Sequence Diagram(s)sequenceDiagram
participant compile_entry
participant js_register_path_init
participant Runtime
participant js_require_path_module
participant prefix__init
participant js_register_path_module
rect rgba(173, 216, 230, 0.5)
note over compile_entry: Startup: entry module main()
compile_entry->>js_register_path_init: (path_ptr, byte_len, &prefix__init)
end
rect rgba(144, 238, 144, 0.5)
note over Runtime: Later: require(absolutePath)
Runtime->>js_require_path_module: path_value (NaN-boxed)
js_require_path_module->>js_require_path_module: check PATH_MODULES cache
js_require_path_module->>prefix__init: transmute + call (on cache miss)
prefix__init->>js_register_path_module: (path_value, module.exports)
js_require_path_module->>Runtime: return cached exports
end
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
crates/perry-runtime/src/object/field_get_set.rs (1)
1079-1099:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate the data-descriptor lookup too.
The accessor table is now protected by
object_has_descriptors, but the followingget_property_attrslookup is still unconditional. SincePROPERTY_DESCRIPTORShas the same raw-address keying, a reused object address can still make this indexed setter silently drop a write as non-writable.🐛 Proposed fix
- if ACCESSORS_IN_USE.with(|c| c.get()) && super::object_has_descriptors(obj as usize) { + let has_descriptors = super::object_has_descriptors(obj as usize); + if ACCESSORS_IN_USE.with(|c| c.get()) && has_descriptors { if let Some(acc) = get_accessor_descriptor(obj as usize, name) { if acc.set != 0 { let closure = (acc.set & crate::value::POINTER_MASK) as *const crate::closure::ClosureHeader; if !closure.is_null() { @@ return; } } - if let Some(attrs) = get_property_attrs(obj as usize, name) { - if !attrs.writable() { - return; + if has_descriptors { + if let Some(attrs) = get_property_attrs(obj as usize, name) { + if !attrs.writable() { + return; + } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/field_get_set.rs` around lines 1079 - 1099, The get_property_attrs call is currently unconditional, but like the accessor descriptor lookup, the PROPERTY_DESCRIPTORS table uses raw-address keying which can cause stale data from reused object addresses. Wrap the get_property_attrs invocation with the same gate condition used for get_accessor_descriptor: check that ACCESSORS_IN_USE is true and object_has_descriptors returns true for the object before calling get_property_attrs.crates/perry-runtime/src/proxy.rs (1)
999-1032:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't skip descriptors for array proxy targets.
object_has_descriptorsis only set forGC_TYPE_OBJECT, but this proxy invariant check handles generic targets. An array with an own non-writable/accessor descriptor skips lines 1012-1021 and can fall through toobj_value_has_own_keyaswritable: true, letting proxysetviolate the target descriptor.🐛 Possible fix direction
- if header.obj_type == crate::gc::GC_TYPE_OBJECT { + if matches!( + header.obj_type, + crate::gc::GC_TYPE_OBJECT | crate::gc::GC_TYPE_ARRAY + ) { let header = header as *const crate::gc::GcHeader as *mut crate::gc::GcHeader; (*header)._reserved |= crate::gc::OBJ_FLAG_HAS_DESCRIPTORS; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/proxy.rs` around lines 999 - 1032, The condition checking `object_has_descriptors(obj_ptr)` on line 1012 only covers GC_TYPE_OBJECT targets and misses array proxy targets that may have their own non-writable or accessor descriptors. This causes arrays with non-writable/accessor descriptors to skip the descriptor lookup block and incorrectly fall through to obj_value_has_own_key which returns writable: true, violating proxy invariants. Modify the condition to also check descriptor tables for array targets, or restructure the logic so descriptor checks are performed regardless of whether the object_has_descriptors flag is set, ensuring both the accessor descriptor and property attributes lookups are consulted for all proxy target types before falling through to the obj_value_has_own_key check.crates/perry-runtime/src/object/field_set_by_name.rs (1)
959-982:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMirror the non-writable check in the wide-key hit path.
When
keys_index_lookuphits, this function writes the slot and returns before reaching the new gatedPROPERTY_DESCRIPTORScheck. A wide object can still overwrite an existingwritable: falseproperty once the sidecar index is active.🐛 Proposed fix
if let Some(i) = keys_index_lookup(obj, keys, name_bytes, key_hash) { let i = i as usize; if is_frozen { let key_str = key_to_str_for_diag(key); crate::error::throw_immutable_write(0, &key_str); } + if PROPERTY_ATTRS_IN_USE.with(|c| c.get()) + && super::object_has_descriptors(obj as usize) + { + if let Some(ref k) = incoming_key_str { + if let Some(attrs) = get_property_attrs(obj as usize, k) { + if !attrs.writable() { + crate::error::throw_immutable_write(0, k); + } + } + } + } if i < alloc_limit { js_object_set_field(obj, i as u32, JSValue::from_bits(value.to_bits())); } else {Also applies to: 1117-1124
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/object/field_set_by_name.rs` around lines 959 - 982, The wide-key hit path (when keys_index_lookup returns Some(i)) is missing a check for non-writable properties via PROPERTY_DESCRIPTORS before writing to the field. This allows wide objects to overwrite properties with writable: false once the sidecar index is active. Add a non-writable property descriptor check in the keys_index_lookup hit path (alongside the existing is_frozen check) to prevent writing to non-writable properties, and apply the same fix to the similar code block at lines 1117-1124 as noted in the comment.crates/perry-hir/src/lower/expr_call/array_only_methods.rs (1)
464-478:⚠️ Potential issue | 🟠 Major | ⚡ Quick winExtend the new mutator bail-out to chained receivers.
Line 1117 now relies on
recv_is_class, but theast::Expr::Callreceiver path at Lines 464-478 still excludespush. A call likefactory().push(x)can therefore still lower to nativearray.push_singleon a class/object result and reinterpret its header as anArrayHeader.Proposed fix
let is_overlapping = matches!( method_name, "find" | "findIndex" | "findLast" | "findLastIndex" | "map" | "filter" | "some" | "every" | "forEach" | "reduce" | "reduceRight" - | "join" + | "join" + | "push" + | "pop" + | "shift" + | "unshift" );Also applies to: 1109-1117
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/lower/expr_call/array_only_methods.rs` around lines 464 - 478, The is_overlapping check in the matches! block starting at line 464 excludes the "push" method, which causes chained receiver calls like factory().push(x) to incorrectly lower to native array.push_single on class/object results. Add "push" to the list of method names in the is_overlapping matches! expression to ensure the mutator bail-out logic is consistently applied across all call receiver paths, matching the behavior now implemented at line 1117 that relies on recv_is_class.crates/perry-hir/src/lower/context.rs (1)
1447-1465:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRestore native-module shadows when block scopes exit too.
The new shadow stack is only truncated in
exit_scope, but block-scopedlet/constbindings are removed bypop_block_scope. A block-localconst url = ...will keepurlhidden fromlookup_native_modulefor the rest of the function after the block exits.Proposed fix
- pub(crate) fn push_block_scope(&mut self) -> (usize, usize) { + pub(crate) fn push_block_scope(&mut self) -> (usize, usize, usize) { self.inside_block_scope += 1; - (self.locals.len(), self.functions.len()) + ( + self.locals.len(), + self.functions.len(), + self.module_shadow_stack.len(), + ) } - pub(crate) fn pop_block_scope(&mut self, mark: (usize, usize)) { + pub(crate) fn pop_block_scope(&mut self, mark: (usize, usize, usize)) { debug_assert!( self.inside_block_scope > 0, "pop_block_scope without matching push" ); self.inside_block_scope = self.inside_block_scope.saturating_sub(1); - let (locals_mark, functions_mark) = mark; + let (locals_mark, functions_mark, module_shadow_mark) = mark; + self.module_shadow_stack.truncate(module_shadow_mark);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/lower/context.rs` around lines 1447 - 1465, The native-module shadow stack is only being restored in the exit_scope method, but block-scoped let/const bindings are removed by pop_block_scope which does not perform this restoration. Apply the same shadow stack restoration logic from exit_scope to the pop_block_scope method - specifically, add code to pop from scope_module_shadow_marks and truncate the module_shadow_stack to the saved mark value, ensuring that block-local bindings no longer shadow native module lookups after the block exits.crates/perry-hir/src/lower_decl/block.rs (1)
1009-1025:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse the resolved class name before re-registering captures.
Line 1010 restores
ctx.class_renames, then Line 1024 looks up captures by the raw AST name. For a body class renamed bymaybe_rename_colliding_class, captures are recorded under the resolved name, so this pass skipsappend_new_args_stmtandRegisterClassCaptures.Proposed fix
}; ctx.forward_class_names = saved_forward_class_names; - ctx.class_renames = saved_class_renames; // Re-register capture snapshots for classes declared in this body at @@ let mut re_regs: Vec<Stmt> = Vec::new(); for stmt in &block.stmts { if let ast::Stmt::Decl(ast::Decl::Class(class_decl)) = stmt { - let cname = class_decl.ident.sym.to_string(); + let cname = ctx.resolve_class_name(class_decl.ident.sym.as_str()); if let Some(captured) = ctx.lookup_class_captures(&cname) { @@ } } } + ctx.class_renames = saved_class_renames; // Undefined-initialised entry slots for hoisted `var`s declared inAlso applies to: 1066-1066
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/lower_decl/block.rs` around lines 1009 - 1025, The code on line 1024 performs a lookup using the raw AST class name (cname derived from class_decl.ident.sym), but class captures are recorded under the resolved class name after renaming. Since ctx.class_renames was restored on line 1010, you need to resolve the class name by checking ctx.class_renames to get the actual renamed class name, then use that resolved name when calling ctx.lookup_class_captures instead of the raw AST name. Apply the same fix to line 1066 which has the identical issue.
🧹 Nitpick comments (5)
crates/perry-stdlib/src/common/dispatch.rs (1)
290-304: ⚡ Quick winDeduplicate the AsyncLocalStorage method-name allowlist.
The same method set (
run/getStore/enterWith/exit/disable) is now maintained in multiple places (method dispatch + property dispatch). A shared helper/const avoids divergence bugs later.♻️ Suggested refactor
+const ASYNC_LOCAL_STORAGE_METHODS: &[&str] = + &["run", "getStore", "enterWith", "exit", "disable"]; + +#[inline] +fn is_async_local_storage_method(name: &str) -> bool { + ASYNC_LOCAL_STORAGE_METHODS.contains(&name) +} + unsafe fn dispatch_async_local_storage_method( handle: i64, method: &str, args: &[f64], ) -> Option<f64> { - if !matches!( - method, - "run" | "getStore" | "enterWith" | "exit" | "disable" - ) { + if !is_async_local_storage_method(method) { return None; } ... } unsafe fn dispatch_async_local_storage_property(handle: i64, property: &str) -> Option<f64> { - if !matches!( - property, - "run" | "getStore" | "enterWith" | "exit" | "disable" - ) { + if !is_async_local_storage_method(property) { return None; } ... }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-stdlib/src/common/dispatch.rs` around lines 290 - 304, The AsyncLocalStorage method-name allowlist (run, getStore, enterWith, exit, disable) is duplicated across multiple functions and can diverge over time. Create a shared constant or helper function that defines this allowed method set once, and refactor both the dispatch_async_local_storage_method and dispatch_async_local_storage_property functions to reference this shared definition instead of maintaining separate copies of the method names.crates/perry-runtime/src/module_require.rs (1)
261-270: 💤 Low valueConsider a debug assertion on
path_lenfor defensive safety.The cast
path_len as usizefromi64will silently wrap negative values to a hugeusize, causing undefined behavior infrom_raw_parts. While the safety contract places responsibility on the caller, a debug assertion could catch codegen bugs early in development without release build overhead.🛡️ Suggested defensive assertion
pub unsafe extern "C" fn js_register_path_init(path_ptr: *const u8, path_len: i64, init_addr: i64) { + debug_assert!(path_len >= 0, "path_len must be non-negative"); let slice = std::slice::from_raw_parts(path_ptr, path_len as usize);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-runtime/src/module_require.rs` around lines 261 - 270, In the js_register_path_init function, add a debug assertion to verify that path_len is non-negative before it is cast to usize and used in the from_raw_parts call. The assertion should check that path_len is greater than or equal to 0, placed before the slice creation to catch potential undefined behavior from negative values being silently cast to large usize values.crates/perry-codegen/src/expr/property_get.rs (1)
603-616: 💤 Low valueDuplicate
nm_install_symbolcall in the common path.The new install call at lines 614-616 is necessary to cover the
process.versionearly-return path. However, for all other properties, this install call is followed by another identical call at lines 639-641. Since the install function is idempotent (atomic store), this is correct but emits redundant code in the generated IR.Consider removing the second call at lines 639-641 since the new early call now covers all paths:
♻️ Optional cleanup
let prop_len_str = property.len().to_string(); - // The value read of a native-module callable export (`const f = - // util.inherits`) mints a BOUND_METHOD closure that, when invoked - // indirectly, dispatches through the per-module `NM_DISPATCH_REGISTRY` - // populated by `js_nm_install_<module>()`. The *direct* call form - // (`util.inherits(a, b)`) is statically lowered to the runtime extern - // and never touches the registry, so a module reached ONLY via this - // value-read path would leave the registry empty and the indirect call - // would resolve to `undefined` (winston/readable-stream's - // `require('inherits')` → `util.inherits` value → `inherits(Sub, Base)` - // silently skipped, breaking the ES5 super-chain). Emit the install - // here so the value-read path's later dispatch finds the module fn. - if let Some(install_sym) = crate::nm_install::nm_install_symbol(module_name) { - ctx.block().call_void(install_sym, &[]); - } return Ok(ctx.block().call(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/expr/property_get.rs` around lines 603 - 616, The code now contains a duplicate call to crate::nm_install::nm_install_symbol within the same function. The first call (with the check for Some(install_sym)) handles all code paths including the early return for process.version, making the second identical call to nm_install_symbol redundant. Locate and remove the second call to nm_install_symbol that occurs later in the function (around the location mentioned in the comment at lines 639-641) since the earlier call now covers all execution paths and the install operation is idempotent, eliminating the need for the duplicate invocation.crates/perry-hir/src/stable_hash/tests.rs (1)
282-282: ⚡ Quick winAdd a direct hash regression check for
is_nested.At Line 282, the fixture includes
is_nested, but this test still doesn’t verify that flipping onlyis_nestedchanges the hash. A small follow-up assertion would lock in the new stable-hash contract.Suggested test addition
m_class.classes.push(Class { id: 1, name: "C".to_string(), @@ aliases: vec![], is_nested: false, }); - assert_ne!(base_hash, hash_module(&m_class)); + let class_hash = hash_module(&m_class); + assert_ne!(base_hash, class_hash); + + let mut m_class_nested = m_class; + m_class_nested.classes[0].is_nested = true; + assert_ne!(class_hash, hash_module(&m_class_nested));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/stable_hash/tests.rs` at line 282, In the test at line 282 where the fixture includes `is_nested: false`, add a regression check assertion after computing the initial hash. Create a second fixture that is identical except with `is_nested` toggled to true, compute its hash, and add an assertion verifying that the two hashes are different. This ensures that changes to the `is_nested` field are properly reflected in the stable hash calculation and prevents future regressions where this field might accidentally be ignored.crates/perry-hir/src/destructuring/var_decl.rs (1)
1938-1938: 💤 Low valueConsider using a unique suffix for the temp local name.
The hardcoded name
"__nx_member_init"works correctly sincedefine_localallocates unique IDs, but multiple occurrences of this pattern in the same scope would produce locals with identical names in HIR dumps/debug output. A minor improvement would be to append a counter or usectx.next_local_id()in the name for clearer diagnostics.This is purely cosmetic and doesn't affect correctness.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-hir/src/destructuring/var_decl.rs` at line 1938, The hardcoded local variable name "__nx_member_init" in the ctx.define_local call should be made unique for better debugging visibility in HIR dumps. Instead of using a static string, append a unique suffix (such as a counter from the context or ctx.next_local_id() if available) to the temporary variable name so that multiple instances of this pattern produce locals with distinguishable names in debug output, making diagnostics clearer without affecting runtime correctness.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/perry-codegen/src/codegen/artifacts.rs`:
- Around line 419-428: The constructor emission logic uses extends_name to check
for class heritage, but the ctor_arity_overrides registration uses extends_expr,
causing an inconsistency where the registered total_params may not match the
actual emitted function signature. Identify both the ctor emission site (around
the extends_name check near line 419 in the extends_name branch) and the
ctor_arity_overrides registration site (mentioned as also applying to lines
1648-1675), then ensure both locations use the same heritage predicate (either
extends_name or extends_expr) consistently so that registered total_params and
synthesized forwarding arity remain synchronized.
In `@crates/perry-codegen/src/codegen/helpers.rs`:
- Around line 808-814: The guard checking `c.is_nested` at lines 808-814
prevents nested class static-field initializers from running at module init in
the primary path, but the later `__perry_static_init_` fallback code path does
not have this same guard. Find where the `__perry_static_init_` fallback
processes classes (likely iterating over static blocks or initializers) and
apply the same `is_nested` check to skip nested classes there as well, ensuring
that nested classes with static blocks cannot execute at module init through
either code path.
In `@crates/perry-codegen/src/codegen/method.rs`:
- Around line 503-561: The dynamic parent super call initialization block is
currently nested inside a condition that only checks for extends_name, making it
unreachable for classes with extends_expr but no static parent name. Ensure that
the outer condition wrapping this entire block (which contains the
builtin_parent_runtime check and the class.extends_expr.is_some() check) is
placed such that the dynamic super initialization executes for classes with
extends_expr, allowing the synthesized constructor to properly initialize fields
and call super with the correct parent reference instead of skipping both steps.
In `@crates/perry-codegen/src/codegen/string_pool.rs`:
- Around line 583-590: The position closure in the constructor rest parameter
lookup using position(|p| p.is_rest) incorrectly matches both actual rest
parameters and Perry's synthesized arguments parameter, causing incorrect rest
registration for constructors with both rest parameters and synthetic arguments.
Modify the position closure in the class.constructor.params iteration to add an
additional condition that excludes the synthetic arguments parameter from
matching, ensuring only actual rest parameters are recorded in ctor_rest_regs.
Apply the same fix to the other occurrence mentioned at lines 700-705.
In `@crates/perry-hir/src/lower_decl/block.rs`:
- Around line 85-108: The current implementation in the block that processes
var_decl.decls only collects top-level variable declarations and ignores var
declarations nested within control flow blocks like if/for/try statements. Since
all var declarations are function-scoped and already predefined, nested var
declarations must also be included in the forward-capture prealloc pass to
ensure earlier closures capture the live box rather than an undefined snapshot.
Modify the loop that iterates over var_decl.decls to recursively traverse and
collect var declarations from nested scopes, then add their identifiers to the
same forward_boxed_ids and var_hoisted_ids logic.
In `@crates/perry-hir/src/lower/context.rs`:
- Around line 411-416: The maybe_rename_colliding_class function generates class
aliases in the format X$0 without verifying that the generated alias doesn't
collide with existing registered classes. Modify the function to generate
candidate aliases in a loop, where each iteration checks both lookup_class for
the candidate and the values already in class_renames to ensure no collision
occurs. Continue incrementing the counter until a safe candidate is found that
doesn't collide with any registered or previously renamed class, then insert
that safe candidate into class_renames.
In `@crates/perry-hir/src/lower/expr_function.rs`:
- Around line 229-230: The function-expression parameter shadowing at lines
229-230 (in the context of define_local and shadow_native_instance_if_present)
only shadows native instances but not native module aliases, which can cause
parameter names like `url` or `util` to incorrectly resolve to Node.js modules
instead of the parameter. After the call to
ctx.shadow_native_instance_if_present for the param_name, also add a call to
shadow native module aliases using the appropriate shadow method (similar to
what is done in function-declaration/private/catch paths). Apply this same fix
to the second location mentioned at lines 603-604.
In `@crates/perry-hir/src/lower/expr_object.rs`:
- Around line 243-244: After defining a local parameter binding with
ctx.define_local() in the object method/accessor parameter handling, you need to
shadow both native instances and native module aliases to prevent parameters
from incorrectly resolving through native-module aliases during body lowering.
In addition to the ctx.shadow_native_instance_if_present() call, also add a
corresponding call to shadow native module aliases. This fix needs to be applied
at both locations where object-literal parameters are processed (around the
ctx.define_local() calls for param_name in the method and accessor parameter
handling blocks).
In `@crates/perry-runtime/src/closure/dispatch.rs`:
- Around line 1397-1407: The special case handling for class objects in this
branch (checking func_ptr.is_null() and is_class_object_value) is being applied
to all value-call scenarios, not just new-expression callee resolution. This
causes direct calls like C() to incorrectly return the class object instead of
throwing an error. Remove or refactor this entire conditional block and instead
pass explicit construct/callee-resolution intent information from the lowering
phase so the code can distinguish between actual new-expression callee
resolution and direct function calls to class objects, or route the affected new
sites to js_new_function_construct directly instead of through this generic
dispatch path.
In `@crates/perry-runtime/src/closure/dynamic_props.rs`:
- Around line 322-333: The `is_valid_obj_ptr` guard at line 331 is insufficient
on Linux-like targets because it only performs a broad range check (>= 0x1000)
rather than real heap-membership validation, allowing aligned unmapped addresses
like 0x4_0000_0000 to bypass the check and reach the unsafe magic read at *(ptr
+ 12), causing potential SIGSEGV. Replace or supplement the is_valid_obj_ptr
call with an actual arena or allocation-membership predicate that definitively
verifies the pointer belongs to a valid heap allocation before attempting to
dereference the ClosureHeader.
In `@crates/perry-runtime/src/fs/mod.rs`:
- Around line 303-309: The current code is calling std::fs::read a second time
just to capture an error, which can result in different errors due to filesystem
race conditions and loses the original error details from
read_file_bytes_with_options. Modify the read_file_bytes_with_options function
to return a Result type (Result<Vec<u8>, io::Error>) instead of just the bytes,
then in the error handling path, extract the error directly from that Result and
pass it to build_fs_error_value, eliminating the redundant filesystem call and
preserving the original error information.
In `@crates/perry-runtime/src/json/replacer.rs`:
- Around line 456-462: The root holder object is being created with the
post-toJSON transformed value instead of the original root value. In the
call_replacer invocation with replacer, empty_key_f64, and value_after_to_json
as arguments, change the root_holder parameter to use the original root value
instead of value_after_to_json. This ensures the this context in the replacer
function sees the original value in the root wrapper, while the replacer itself
still operates on the post-toJSON transformed value. Apply this fix to both
occurrences mentioned: the main location around lines 456-462 and the additional
location at lines 1302-1307.
In `@crates/perry-runtime/src/object/class_constructors.rs`:
- Around line 535-554: Replace the `lookup_closure_rest` call to return full
rest metadata instead of just the index, capturing information about whether the
rest slot is ordinary rest, synthetic arguments, or rest-and-arguments. Update
the rest parameter handling logic starting with the loop that populates
final_args and the rest_arr creation to use this metadata to determine whether
arguments should be packed from all call args or from the tail, ensuring the
correct number of arguments are included based on the specific rest type,
particularly when handling synthetic arguments objects.
In `@crates/perry-runtime/src/object/object_ops.rs`:
- Around line 1388-1403: Move the descriptor validation that normally occurs via
validate_property_descriptor to execute before the native handle fast path in
the code block with dispatch and the early return statement. Ensure that invalid
descriptors are caught and validated before the early return of obj_value, and
only allow validated accessor or symbol descriptors to proceed through this fast
path without throwing errors, preventing invalid descriptors from bypassing
validation checks.
- Around line 1371-1380: The handle classification logic in the handle_id
assignment uses a hardcoded boundary check `p < 0x10000` instead of the
canonical handle classifier used elsewhere in the runtime. Replace the
conditional `if p >= 1 && p < 0x10000` with a call to the standard
`addr_class::is_small_handle(p)` function to ensure consistent handle
classification across all runtime paths and properly recognize native
handle-band values.
In `@crates/perry-runtime/src/object/polymorphic_index.rs`:
- Around line 49-52: The match statement in the polymorphic_index function
currently filters tag values with cases for 0x7FFD and 0x7FFF (heap string
pointers) but lacks handling for the inline short-string (SSO) tag before the
default fallback case. Add a new match arm for the short-string tag value that
appears before the underscore catch-all case, which should materialize the
string and route it to the appropriate string indexing logic instead of
returning the undefined value. This ensures SSO strings like "abc" are properly
indexed rather than falling through to the primitive rejection path.
- Around line 78-89: The is_valid_obj_ptr call in the guard before reading the
GC header is insufficient because it only checks a broad address range on
Linux-like targets, allowing small bogus values like 0xf000f to pass through and
cause a wild memory read at raw - GC_HEADER_SIZE. Replace the is_valid_obj_ptr
check with a proper GC or allocation-membership predicate that actually verifies
the pointer belongs to a valid heap allocation, not just checking if it falls
within a broad address range, to prevent small bogus pointers from reaching the
header dereference operation.
---
Outside diff comments:
In `@crates/perry-hir/src/lower_decl/block.rs`:
- Around line 1009-1025: The code on line 1024 performs a lookup using the raw
AST class name (cname derived from class_decl.ident.sym), but class captures are
recorded under the resolved class name after renaming. Since ctx.class_renames
was restored on line 1010, you need to resolve the class name by checking
ctx.class_renames to get the actual renamed class name, then use that resolved
name when calling ctx.lookup_class_captures instead of the raw AST name. Apply
the same fix to line 1066 which has the identical issue.
In `@crates/perry-hir/src/lower/context.rs`:
- Around line 1447-1465: The native-module shadow stack is only being restored
in the exit_scope method, but block-scoped let/const bindings are removed by
pop_block_scope which does not perform this restoration. Apply the same shadow
stack restoration logic from exit_scope to the pop_block_scope method -
specifically, add code to pop from scope_module_shadow_marks and truncate the
module_shadow_stack to the saved mark value, ensuring that block-local bindings
no longer shadow native module lookups after the block exits.
In `@crates/perry-hir/src/lower/expr_call/array_only_methods.rs`:
- Around line 464-478: The is_overlapping check in the matches! block starting
at line 464 excludes the "push" method, which causes chained receiver calls like
factory().push(x) to incorrectly lower to native array.push_single on
class/object results. Add "push" to the list of method names in the
is_overlapping matches! expression to ensure the mutator bail-out logic is
consistently applied across all call receiver paths, matching the behavior now
implemented at line 1117 that relies on recv_is_class.
In `@crates/perry-runtime/src/object/field_get_set.rs`:
- Around line 1079-1099: The get_property_attrs call is currently unconditional,
but like the accessor descriptor lookup, the PROPERTY_DESCRIPTORS table uses
raw-address keying which can cause stale data from reused object addresses. Wrap
the get_property_attrs invocation with the same gate condition used for
get_accessor_descriptor: check that ACCESSORS_IN_USE is true and
object_has_descriptors returns true for the object before calling
get_property_attrs.
In `@crates/perry-runtime/src/object/field_set_by_name.rs`:
- Around line 959-982: The wide-key hit path (when keys_index_lookup returns
Some(i)) is missing a check for non-writable properties via PROPERTY_DESCRIPTORS
before writing to the field. This allows wide objects to overwrite properties
with writable: false once the sidecar index is active. Add a non-writable
property descriptor check in the keys_index_lookup hit path (alongside the
existing is_frozen check) to prevent writing to non-writable properties, and
apply the same fix to the similar code block at lines 1117-1124 as noted in the
comment.
In `@crates/perry-runtime/src/proxy.rs`:
- Around line 999-1032: The condition checking `object_has_descriptors(obj_ptr)`
on line 1012 only covers GC_TYPE_OBJECT targets and misses array proxy targets
that may have their own non-writable or accessor descriptors. This causes arrays
with non-writable/accessor descriptors to skip the descriptor lookup block and
incorrectly fall through to obj_value_has_own_key which returns writable: true,
violating proxy invariants. Modify the condition to also check descriptor tables
for array targets, or restructure the logic so descriptor checks are performed
regardless of whether the object_has_descriptors flag is set, ensuring both the
accessor descriptor and property attributes lookups are consulted for all proxy
target types before falling through to the obj_value_has_own_key check.
---
Nitpick comments:
In `@crates/perry-codegen/src/expr/property_get.rs`:
- Around line 603-616: The code now contains a duplicate call to
crate::nm_install::nm_install_symbol within the same function. The first call
(with the check for Some(install_sym)) handles all code paths including the
early return for process.version, making the second identical call to
nm_install_symbol redundant. Locate and remove the second call to
nm_install_symbol that occurs later in the function (around the location
mentioned in the comment at lines 639-641) since the earlier call now covers all
execution paths and the install operation is idempotent, eliminating the need
for the duplicate invocation.
In `@crates/perry-hir/src/destructuring/var_decl.rs`:
- Line 1938: The hardcoded local variable name "__nx_member_init" in the
ctx.define_local call should be made unique for better debugging visibility in
HIR dumps. Instead of using a static string, append a unique suffix (such as a
counter from the context or ctx.next_local_id() if available) to the temporary
variable name so that multiple instances of this pattern produce locals with
distinguishable names in debug output, making diagnostics clearer without
affecting runtime correctness.
In `@crates/perry-hir/src/stable_hash/tests.rs`:
- Line 282: In the test at line 282 where the fixture includes `is_nested:
false`, add a regression check assertion after computing the initial hash.
Create a second fixture that is identical except with `is_nested` toggled to
true, compute its hash, and add an assertion verifying that the two hashes are
different. This ensures that changes to the `is_nested` field are properly
reflected in the stable hash calculation and prevents future regressions where
this field might accidentally be ignored.
In `@crates/perry-runtime/src/module_require.rs`:
- Around line 261-270: In the js_register_path_init function, add a debug
assertion to verify that path_len is non-negative before it is cast to usize and
used in the from_raw_parts call. The assertion should check that path_len is
greater than or equal to 0, placed before the slice creation to catch potential
undefined behavior from negative values being silently cast to large usize
values.
In `@crates/perry-stdlib/src/common/dispatch.rs`:
- Around line 290-304: The AsyncLocalStorage method-name allowlist (run,
getStore, enterWith, exit, disable) is duplicated across multiple functions and
can diverge over time. Create a shared constant or helper function that defines
this allowed method set once, and refactor both the
dispatch_async_local_storage_method and dispatch_async_local_storage_property
functions to reference this shared definition instead of maintaining separate
copies of the method names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 1a58d3ca-dab6-4117-ba4c-5cbfb808ef4e
📒 Files selected for processing (75)
crates/perry-codegen/src/codegen/artifacts.rscrates/perry-codegen/src/codegen/entry.rscrates/perry-codegen/src/codegen/helpers.rscrates/perry-codegen/src/codegen/method.rscrates/perry-codegen/src/codegen/mod.rscrates/perry-codegen/src/codegen/opts.rscrates/perry-codegen/src/codegen/string_pool.rscrates/perry-codegen/src/collectors/class_accessors.rscrates/perry-codegen/src/collectors/this_as_value.rscrates/perry-codegen/src/expr/bigint_set.rscrates/perry-codegen/src/expr/property_get.rscrates/perry-codegen/src/lower_call/native/mod.rscrates/perry-codegen/src/lower_call/property_get.rscrates/perry-codegen/src/runtime_decls/objects.rscrates/perry-codegen/src/runtime_decls/stdlib_ffi.rscrates/perry-codegen/tests/constructor_recursion.rscrates/perry-codegen/tests/large_object_barriers.rscrates/perry-codegen/tests/macos_bundle_chdir_gate.rscrates/perry-codegen/tests/native_proof_buffer_views.rscrates/perry-codegen/tests/native_proof_regressions.rscrates/perry-codegen/tests/shadow_slot_hygiene.rscrates/perry-codegen/tests/static_symbol_hygiene.rscrates/perry-codegen/tests/typed_feedback.rscrates/perry-codegen/tests/typed_shape_descriptor.rscrates/perry-codegen/tests/typed_shape_descriptors.rscrates/perry-hir/src/destructuring/var_decl.rscrates/perry-hir/src/ir/decl.rscrates/perry-hir/src/lower/context.rscrates/perry-hir/src/lower/expr_call/array_only_methods.rscrates/perry-hir/src/lower/expr_call/globals.rscrates/perry-hir/src/lower/expr_call/local_array_methods.rscrates/perry-hir/src/lower/expr_call/native_module.rscrates/perry-hir/src/lower/expr_call/regex_string.rscrates/perry-hir/src/lower/expr_function.rscrates/perry-hir/src/lower/expr_member.rscrates/perry-hir/src/lower/expr_new.rscrates/perry-hir/src/lower/expr_object.rscrates/perry-hir/src/lower/lower_expr.rscrates/perry-hir/src/lower/lowering_context.rscrates/perry-hir/src/lower/module_decl.rscrates/perry-hir/src/lower_decl/block.rscrates/perry-hir/src/lower_decl/body_stmt.rscrates/perry-hir/src/lower_decl/body_stmt/nested_fn_decl.rscrates/perry-hir/src/lower_decl/class_decl.rscrates/perry-hir/src/lower_decl/class_members.rscrates/perry-hir/src/lower_decl/fn_decl.rscrates/perry-hir/src/lower_decl/private_members.rscrates/perry-hir/src/monomorph/specialize.rscrates/perry-hir/src/stable_hash/decls.rscrates/perry-hir/src/stable_hash/tests.rscrates/perry-runtime/src/closure/dispatch.rscrates/perry-runtime/src/closure/dynamic_props.rscrates/perry-runtime/src/fs/mod.rscrates/perry-runtime/src/json/replacer.rscrates/perry-runtime/src/lib.rscrates/perry-runtime/src/module_require.rscrates/perry-runtime/src/object/class_constructors.rscrates/perry-runtime/src/object/class_registry.rscrates/perry-runtime/src/object/field_get_set.rscrates/perry-runtime/src/object/field_set_by_name.rscrates/perry-runtime/src/object/mod.rscrates/perry-runtime/src/object/native_call_method.rscrates/perry-runtime/src/object/object_ops.rscrates/perry-runtime/src/object/polymorphic_index.rscrates/perry-runtime/src/proxy.rscrates/perry-runtime/src/value/handle.rscrates/perry-runtime/src/value/mod.rscrates/perry-runtime/src/value/tags.rscrates/perry-stdlib/src/common/dispatch.rscrates/perry-transform/src/generator/id_scan.rscrates/perry-transform/src/inline/mod.rscrates/perry/src/commands/compile.rscrates/perry/src/commands/compile/cjs_wrap/wrap.rscrates/perry/src/commands/compile/collect_modules.rscrates/perry/src/commands/compile/init_order.rs
Comment on lines
419
to
+428
| } else if class.extends_name.is_some() { | ||
| // Walk ancestors for the first one with a ctor; adopt its | ||
| // params (cleared of ids — they'll be fresh). | ||
| let mut found_params: Vec<perry_hir::Param> = Vec::new(); | ||
| let mut cur = class.extends_name.clone(); | ||
| while let Some(pname) = cur { | ||
| // v0.5.760: also consult `opts.imported_classes` for | ||
| // cross-module parent ctors. Pre-fix the loop fell | ||
| // through to the next ancestor when `class_table`'s | ||
| // entry for an imported class returned a stub with | ||
| // `constructor: None` (stubs always have None) — even | ||
| // though the source module did have a real ctor/effect. | ||
| // Result: `class Child extends Parent { x = | ||
| // "y" }` (no own ctor, parent in another module) had | ||
| // its synthesized ctor with ZERO params, so the user's | ||
| // `new Child("arg")` lost the arg before reaching | ||
| // Parent_constructor. Explicit zero-arg ctors and | ||
| // field-initializer ctors still stop the walk even with | ||
| // zero adopted params. Refs #420. | ||
| let imported_ctor = opts | ||
| .imported_classes | ||
| .iter() | ||
| .find(|i| i.local_alias.as_deref().unwrap_or(&i.name) == pname.as_str()) | ||
| .filter(|ic| { | ||
| ic.constructor_param_count > 0 | ||
| || ic.has_own_constructor | ||
| || ic.has_instance_fields | ||
| }); | ||
| if let Some(pclass) = class_table.get(pname.as_str()) { | ||
| if let Some(pctor) = &pclass.constructor { | ||
| found_params = pctor.params.clone(); | ||
| break; | ||
| } | ||
| if let Some(imported_ctor) = imported_ctor { | ||
| for i in 0..imported_ctor.constructor_param_count { | ||
| found_params.push(perry_hir::Param { | ||
| id: 0xFFFF_0000 + i as u32, | ||
| name: format!("__forward_arg{}", i), | ||
| ty: perry_types::Type::Any, | ||
| default: None, | ||
| decorators: Vec::new(), | ||
| is_rest: false, | ||
| arguments_object: None, | ||
| }); | ||
| } | ||
| break; | ||
| } | ||
| cur = pclass.extends_name.clone(); | ||
| } else if let Some(stub) = imported_class_stubs.iter().find(|c| c.name == pname) | ||
| { | ||
| // Imported stub — params not in HIR; use effectful | ||
| // ctor metadata as a synthetic count of unnamed args. | ||
| if let Some(imported_ctor) = imported_ctor { | ||
| for i in 0..imported_ctor.constructor_param_count { | ||
| found_params.push(perry_hir::Param { | ||
| id: 0xFFFF_0000 + i as u32, | ||
| name: format!("__forward_arg{}", i), | ||
| ty: perry_types::Type::Any, | ||
| default: None, | ||
| decorators: Vec::new(), | ||
| is_rest: false, | ||
| arguments_object: None, | ||
| }); | ||
| } | ||
| } else { | ||
| cur = stub.extends_name.clone(); | ||
| continue; | ||
| } | ||
| break; | ||
| } else { | ||
| break; | ||
| } | ||
| } | ||
| // No own ctor + heritage → JS spec default ctor | ||
| // `constructor(...args) { super(...args) }`. Synthesize forwarding | ||
| // params matching the closest ancestor ctor's arity (incl. | ||
| // cross-module parents) via the shared helper — its result is | ||
| // ALSO what gets registered into CLASS_CONSTRUCTORS below, so the | ||
| // emitted signature and the runtime `total_params` always agree | ||
| // (Next.js wall 51). The actual `super(...)` is emitted by the | ||
| // compile_method post-init step from these params positionally. | ||
| let n = synthesized_ctor_param_count( |
There was a problem hiding this comment.
Use the same heritage predicate for ctor emission and registration.
ctor_arity_overrides now registers an extends_expr-only class with the synthesized forwarding arity, but the actual standalone ctor only gets those params when extends_name is set. That can register a nonzero total_params for a zero-arg LLVM function and drop dynamic parent forwarding.
Proposed fix
- } else if class.extends_name.is_some() {
+ } else if class.extends_name.is_some() || class.extends_expr.is_some() {
// No own ctor + heritage → JS spec default ctor
// `constructor(...args) { super(...args) }`. Synthesize forwardingAlso applies to: 1648-1675
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/codegen/artifacts.rs` around lines 419 - 428, The
constructor emission logic uses extends_name to check for class heritage, but
the ctor_arity_overrides registration uses extends_expr, causing an
inconsistency where the registered total_params may not match the actual emitted
function signature. Identify both the ctor emission site (around the
extends_name check near line 419 in the extends_name branch) and the
ctor_arity_overrides registration site (mentioned as also applying to lines
1648-1675), then ensure both locations use the same heritage predicate (either
extends_name or extends_expr) consistently so that registered total_params and
synthesized forwarding arity remain synchronized.
Comment on lines
+808
to
+814
| // Next.js wall 54: a nested class's static-field initializers must run | ||
| // when the enclosing function evaluates the class, not at module init. | ||
| // Running a side-effectful one eagerly (e.g. `static #a = new Self()`) | ||
| // both mistimes it and can crash before user code. | ||
| if c.is_nested { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Skip nested classes in the static-block fallback too.
This guard prevents nested static fields from running at module init, but the later __perry_static_init_ fallback still walks nested classes. A nested class with static { ... } that is not already represented in hir.init can still execute at module init and hit the same out-of-scope/local-timing bug.
Proposed fix
for c in &hir.classes {
+ if c.is_nested {
+ continue;
+ }
for sm in &c.static_methods {
if !sm.name.starts_with("__perry_static_init_") {
continue;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // Next.js wall 54: a nested class's static-field initializers must run | |
| // when the enclosing function evaluates the class, not at module init. | |
| // Running a side-effectful one eagerly (e.g. `static #a = new Self()`) | |
| // both mistimes it and can crash before user code. | |
| if c.is_nested { | |
| continue; | |
| } | |
| for c in &hir.classes { | |
| if c.is_nested { | |
| continue; | |
| } | |
| for sm in &c.static_methods { | |
| if !sm.name.starts_with("__perry_static_init_") { | |
| continue; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/codegen/helpers.rs` around lines 808 - 814, The
guard checking `c.is_nested` at lines 808-814 prevents nested class static-field
initializers from running at module init in the primary path, but the later
`__perry_static_init_` fallback code path does not have this same guard. Find
where the `__perry_static_init_` fallback processes classes (likely iterating
over static blocks or initializers) and apply the same `is_nested` check to skip
nested classes there as well, ensuring that nested classes with static blocks
cannot execute at module init through either code path.
Comment on lines
+503
to
+561
| if builtin_parent_runtime.is_none() && class.extends_expr.is_some() { | ||
| if let Some(cid) = | ||
| ctx.class_ids.get(&class.name).copied().filter(|c| *c != 0) | ||
| { | ||
| let undef_lit = crate::nanbox::double_literal(f64::from_bits( | ||
| crate::nanbox::TAG_UNDEFINED, | ||
| )); | ||
| let mut lowered_args: Vec<String> = | ||
| Vec::with_capacity(method.params.len()); | ||
| for p in &method.params { | ||
| if let Some(slot) = ctx.locals.get(&p.id).cloned() { | ||
| lowered_args.push(ctx.block().load(DOUBLE, &slot)); | ||
| } else { | ||
| lowered_args.push(undef_lit.clone()); | ||
| } | ||
| } | ||
| let parent_val = ctx.block().call( | ||
| DOUBLE, | ||
| "js_get_dynamic_parent_value", | ||
| &[(crate::types::I32, &cid.to_string())], | ||
| ); | ||
| let (args_ptr, args_len) = if lowered_args.is_empty() { | ||
| ("null".to_string(), "0".to_string()) | ||
| } else { | ||
| let buf_reg = | ||
| ctx.func.alloca_entry_array(DOUBLE, lowered_args.len()); | ||
| for (i, a_val) in lowered_args.iter().enumerate() { | ||
| let slot = ctx.block().gep( | ||
| DOUBLE, | ||
| &buf_reg, | ||
| &[(I64, &format!("{}", i))], | ||
| ); | ||
| ctx.block().store(DOUBLE, a_val, &slot); | ||
| } | ||
| let ptr_reg = ctx.block().next_reg(); | ||
| ctx.block().emit_raw(format!( | ||
| "{} = getelementptr [{} x double], ptr {}, i64 0, i64 0", | ||
| ptr_reg, | ||
| lowered_args.len(), | ||
| buf_reg | ||
| )); | ||
| (ptr_reg, lowered_args.len().to_string()) | ||
| }; | ||
| let this_box = match ctx.this_stack.last().cloned() { | ||
| Some(slot) => ctx.block().load(DOUBLE, &slot), | ||
| None => undef_lit.clone(), | ||
| }; | ||
| let _ = ctx.block().call( | ||
| DOUBLE, | ||
| "js_fetch_or_value_super", | ||
| &[ | ||
| (DOUBLE, &parent_val), | ||
| (DOUBLE, &this_box), | ||
| (crate::types::PTR, &args_ptr), | ||
| (I64, &args_len), | ||
| ], | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
Make the dynamic-parent path reachable for extends_expr-only classes.
The new dynamic super(...args) block still sits inside a no-own-ctor branch that only checks extends_name, and field initialization also treats extends_expr as a base class. For a dynamic parent with no static name, the synthesized ctor skips super and initializes self fields as if it had no heritage.
Proposed fix
- let init_mode = if class.extends_name.is_some() {
+ let has_heritage = class.extends_name.is_some() || class.extends_expr.is_some();
+ let init_mode = if has_heritage {
crate::lower_call::FieldInitMode::AncestorsOnly
} else {
crate::lower_call::FieldInitMode::All
};
@@
- if class.constructor.is_none() && class.extends_name.is_some() {
+ if class.constructor.is_none() && has_heritage {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/codegen/method.rs` around lines 503 - 561, The
dynamic parent super call initialization block is currently nested inside a
condition that only checks for extends_name, making it unreachable for classes
with extends_expr but no static parent name. Ensure that the outer condition
wrapping this entire block (which contains the builtin_parent_runtime check and
the class.extends_expr.is_some() check) is placed such that the dynamic super
initialization executes for classes with extends_expr, allowing the synthesized
constructor to properly initialize fields and call super with the correct parent
reference instead of skipping both steps.
Comment on lines
+583
to
+590
| // #wall3: record the rest-param position (in USER params) so the runtime | ||
| // bundles trailing args at the dynamic member-new dispatch path. | ||
| if let Some(rest_idx) = class | ||
| .constructor | ||
| .as_ref() | ||
| .map(|c| c.params.len() as u32) | ||
| .unwrap_or(0); | ||
| ctor_triples.push(( | ||
| cid, | ||
| format!("{}__{}_constructor", module_prefix, class_name), | ||
| ctor_params, | ||
| )); | ||
| .and_then(|c| c.params.iter().position(|p| p.is_rest)) | ||
| { | ||
| ctor_rest_regs.push((ctor_symbol.clone(), rest_idx)); |
There was a problem hiding this comment.
Preserve synthetic arguments metadata for constructor rest registration.
position(|p| p.is_rest) also matches Perry’s synthesized arguments param (arguments_object.is_some()), but this always emits js_register_closure_rest. Dynamic construction of constructor(a) { arguments } would bundle only the tail after a, not all original call args; constructor(...rest) { arguments } also needs the rest-and-arguments path.
Suggested direction
- if let Some(rest_idx) = class
- .constructor
- .as_ref()
- .and_then(|c| c.params.iter().position(|p| p.is_rest))
- {
- ctor_rest_regs.push((ctor_symbol.clone(), rest_idx));
+ if let Some(constructor) = class.constructor.as_ref() {
+ // Mirror the user-function wrapper split:
+ // - ordinary user rest -> js_register_closure_rest
+ // - synthesized `arguments` -> js_register_closure_synthetic_arguments
+ // - user rest + synthesized `arguments` -> js_register_closure_rest_and_arguments
+ // Store the runtime registration kind alongside `rest_idx`.
}Also applies to: 700-705
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-codegen/src/codegen/string_pool.rs` around lines 583 - 590, The
position closure in the constructor rest parameter lookup using position(|p|
p.is_rest) incorrectly matches both actual rest parameters and Perry's
synthesized arguments parameter, causing incorrect rest registration for
constructors with both rest parameters and synthetic arguments. Modify the
position closure in the class.constructor.params iteration to add an additional
condition that excludes the synthetic arguments parameter from matching,
ensuring only actual rest parameters are recorded in ctor_rest_regs. Apply the
same fix to the other occurrence mentioned at lines 700-705.
Comment on lines
+85
to
+108
| } else { | ||
| // `var` bindings are already predefined + boxed by | ||
| // `predefine_var_bindings_in_function_body`, but their box is | ||
| // NOT in the prealloc set. A closure created EARLIER in the body | ||
| // that references a `var` declared LATER (`r.d(t,{x:()=>n.x}); | ||
| // var n=r("…")` — the webpack ESM re-export shape in Next.js' | ||
| // react-server.node.js) must capture the *live* box, not a | ||
| // TAG_UNDEFINED snapshot. Add forward-captured `var` ids to the | ||
| // prealloc set so codegen allocates the box at function entry. | ||
| for decl in &var_decl.decls { | ||
| let mut binding_idents: Vec<(String, u32)> = Vec::new(); | ||
| collect_pat_forward_idents(&decl.name, &mut binding_idents); | ||
| for (name, _span_lo) in binding_idents { | ||
| if !seen_closure_refs.contains(&name) { | ||
| continue; | ||
| } | ||
| if let Some(id) = ctx.lookup_local(&name) { | ||
| if !forward_boxed_ids.contains(&id) { | ||
| ctx.var_hoisted_ids.insert(id); | ||
| forward_boxed_ids.push(id); | ||
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Include nested var declarations in the forward-capture prealloc pass.
This branch only sees top-level var statements. A later var inside if/for/try is still function-scoped and already predefined, so an earlier closure reading it can still capture an unallocated/undefined box.
Proposed fix
- } else {
- // `var` bindings are already predefined + boxed by
- // `predefine_var_bindings_in_function_body`, but their box is
- // NOT in the prealloc set. A closure created EARLIER in the body
- // that references a `var` declared LATER (`r.d(t,{x:()=>n.x});
- // var n=r("…")` — the webpack ESM re-export shape in Next.js'
- // react-server.node.js) must capture the *live* box, not a
- // TAG_UNDEFINED snapshot. Add forward-captured `var` ids to the
- // prealloc set so codegen allocates the box at function entry.
- for decl in &var_decl.decls {
- let mut binding_idents: Vec<(String, u32)> = Vec::new();
- collect_pat_forward_idents(&decl.name, &mut binding_idents);
- for (name, _span_lo) in binding_idents {
- if !seen_closure_refs.contains(&name) {
- continue;
- }
- if let Some(id) = ctx.lookup_local(&name) {
- if !forward_boxed_ids.contains(&id) {
- ctx.var_hoisted_ids.insert(id);
- forward_boxed_ids.push(id);
- }
- }
- }
- }
}
}
+ // `var` is function-scoped even when it appears inside nested statements.
+ // Use the same recursive collector as the predefine pass so forward
+ // captures of later `if (...) { var n = ... }` / loop declarations are
+ // preallocated too.
+ let mut var_names = Vec::new();
+ collect_var_binding_names_from_stmt(stmt, &mut var_names);
+ var_names.sort();
+ var_names.dedup();
+ for name in var_names {
+ if !seen_closure_refs.contains(&name) {
+ continue;
+ }
+ if let Some(id) = ctx.lookup_local(&name) {
+ if !forward_boxed_ids.contains(&id) {
+ ctx.var_hoisted_ids.insert(id);
+ forward_boxed_ids.push(id);
+ }
+ }
+ }
// Record closures introduced by THIS statement for subsequent decls.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-hir/src/lower_decl/block.rs` around lines 85 - 108, The current
implementation in the block that processes var_decl.decls only collects
top-level variable declarations and ignores var declarations nested within
control flow blocks like if/for/try statements. Since all var declarations are
function-scoped and already predefined, nested var declarations must also be
included in the forward-capture prealloc pass to ensure earlier closures capture
the live box rather than an undefined snapshot. Modify the loop that iterates
over var_decl.decls to recursively traverse and collect var declarations from
nested scopes, then add their identifiers to the same forward_boxed_ids and
var_hoisted_ids logic.
Comment on lines
+535
to
+554
| let rest_idx = crate::closure::lookup_closure_rest(ctor_ptr as *const u8) | ||
| .map(|ri| ri as usize) | ||
| .filter(|ri| *ri < user_params); | ||
| if let Some(ri) = rest_idx { | ||
| for i in 0..ri { | ||
| if !args_ptr.is_null() && i < args_len { | ||
| final_args.push(*args_ptr.add(i)); | ||
| } else { | ||
| final_args.push(undef); | ||
| } | ||
| } | ||
| let mut rest_arr = crate::array::js_array_alloc(0); | ||
| if !args_ptr.is_null() { | ||
| let mut i = ri; | ||
| while i < args_len { | ||
| rest_arr = crate::array::js_array_push_f64(rest_arr, *args_ptr.add(i)); | ||
| i += 1; | ||
| } | ||
| } | ||
| final_args.push(crate::value::js_nanbox_pointer(rest_arr as i64)); |
There was a problem hiding this comment.
Replay constructors using the full rest metadata, not only the index.
lookup_closure_rest loses whether the registered slot is ordinary rest, synthetic arguments, or rest-and-arguments. Once constructor symbols can include arguments_object params, this packs arguments from the tail instead of all call args and can omit the extra synthetic slot after a user rest before captures are appended.
Suggested direction
- let rest_idx = crate::closure::lookup_closure_rest(ctor_ptr as *const u8)
- .map(|ri| ri as usize)
- .filter(|ri| *ri < user_params);
+ // Use the same full rest-kind metadata as dynamic closure dispatch, then
+ // build constructor user args accordingly before appending captures:
+ // - ordinary rest: fixed args + tail array
+ // - synthetic `arguments`: fixed args + full arguments array
+ // - rest-and-arguments: fixed args + tail array + full arguments arrayAlso applies to: 615-634
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/object/class_constructors.rs` around lines 535 -
554, Replace the `lookup_closure_rest` call to return full rest metadata instead
of just the index, capturing information about whether the rest slot is ordinary
rest, synthetic arguments, or rest-and-arguments. Update the rest parameter
handling logic starting with the loop that populates final_args and the rest_arr
creation to use this metadata to determine whether arguments should be packed
from all call args or from the tail, ensuring the correct number of arguments
are included based on the specific rest type, particularly when handling
synthetic arguments objects.
Comment on lines
+1371
to
+1380
| let handle_id = if jv.is_pointer() { | ||
| let p = jv.as_pointer::<u8>() as usize; | ||
| if p >= 1 && p < 0x10000 { | ||
| Some(p) | ||
| } else { | ||
| None | ||
| } | ||
| } else { | ||
| None | ||
| }; |
There was a problem hiding this comment.
Use the canonical handle classifier here.
The p < 0x10000 cutoff misses native handle-band values that other runtime paths classify with addr_class::is_small_handle(...). Handles at/above 0x10000 will skip this best-effort path and still throw “called on non-object”.
Proposed fix
- if p >= 1 && p < 0x10000 {
+ if crate::value::addr_class::is_small_handle(p) {
Some(p)
} else {
None
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| let handle_id = if jv.is_pointer() { | |
| let p = jv.as_pointer::<u8>() as usize; | |
| if p >= 1 && p < 0x10000 { | |
| Some(p) | |
| } else { | |
| None | |
| } | |
| } else { | |
| None | |
| }; | |
| let handle_id = if jv.is_pointer() { | |
| let p = jv.as_pointer::<u8>() as usize; | |
| if crate::value::addr_class::is_small_handle(p) { | |
| Some(p) | |
| } else { | |
| None | |
| } | |
| } else { | |
| None | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/object/object_ops.rs` around lines 1371 - 1380, The
handle classification logic in the handle_id assignment uses a hardcoded
boundary check `p < 0x10000` instead of the canonical handle classifier used
elsewhere in the runtime. Replace the conditional `if p >= 1 && p < 0x10000`
with a call to the standard `addr_class::is_small_handle(p)` function to ensure
consistent handle classification across all runtime paths and properly recognize
native handle-band values.
Comment on lines
+1388
to
+1403
| let dval = if desc_has_field(descriptor_value, b"value") { | ||
| Some(f64::from_bits( | ||
| desc_read_field(descriptor_value, b"value").bits(), | ||
| )) | ||
| } else { | ||
| None | ||
| }; | ||
| if let Some(v) = dval { | ||
| let name_ptr = | ||
| (ks as *const u8).add(std::mem::size_of::<crate::StringHeader>()); | ||
| let name_len = (*ks).byte_len as usize; | ||
| dispatch(hid as i64, name_ptr, name_len, v); | ||
| } | ||
| } | ||
| } | ||
| return obj_value; |
There was a problem hiding this comment.
Validate the descriptor before the native-handle early return.
This branch returns before the normal descriptor object check and validate_property_descriptor, so invalid descriptors on native handles can be accepted instead of throwing. Move the descriptor validation ahead of the handle fast path, then no-op only validated accessor/symbol descriptors.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/object/object_ops.rs` around lines 1388 - 1403, Move
the descriptor validation that normally occurs via validate_property_descriptor
to execute before the native handle fast path in the code block with dispatch
and the early return statement. Ensure that invalid descriptors are caught and
validated before the early return of obj_value, and only allow validated
accessor or symbol descriptors to proceed through this fast path without
throwing errors, preventing invalid descriptors from bypassing validation
checks.
Comment on lines
+49
to
+52
| match (obj_handle as u64) >> 48 { | ||
| 0x7FFD | 0x7FFF => (obj_handle as u64) & 0x0000_FFFF_FFFF_FFFF, | ||
| _ => return f64::from_bits(crate::value::TAG_UNDEFINED), | ||
| } |
There was a problem hiding this comment.
Handle inline short strings before rejecting NaN-boxed primitives.
The tag filter accepts heap string pointers (0x7FFF) but rejects the inline short-string tag path. If an SSO string reaches this polymorphic index helper, "abc"[0] returns undefined instead of indexing the string. Add a short-string branch that materializes/routes to the string index path before the primitive fallback.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/object/polymorphic_index.rs` around lines 49 - 52,
The match statement in the polymorphic_index function currently filters tag
values with cases for 0x7FFD and 0x7FFF (heap string pointers) but lacks
handling for the inline short-string (SSO) tag before the default fallback case.
Add a new match arm for the short-string tag value that appears before the
underscore catch-all case, which should materialize the string and route it to
the appropriate string indexing logic instead of returning the undefined value.
This ensures SSO strings like "abc" are properly indexed rather than falling
through to the primitive rejection path.
Comment on lines
+78
to
+89
| // #wall5-render: `obj[idx]` where `obj` is a mis-boxed / non-heap value | ||
| // (e.g. a small bogus pointer like 0xf000f produced upstream) must NOT | ||
| // dereference the GcHeader at `raw-8` — that's a wild read → SIGSEGV. The | ||
| // `raw < 0x1000` guards above are too weak (0xf000f passes). Typed-array / | ||
| // buffer / string receivers were already handled before this point, so a | ||
| // value reaching here that isn't a valid arena/old-gen object pointer is | ||
| // not indexable → `undefined` (matches JS `(5)[0]` etc.). Mirrors the | ||
| // is_closure_ptr heap-range guard (wall #2). Next.js app-page-turbo's | ||
| // `u_i_24_6.get` indexed such a value during the app render → crash. | ||
| if !crate::value::addr_class::is_valid_obj_ptr(raw as *const u8) { | ||
| return f64::from_bits(crate::value::TAG_UNDEFINED); | ||
| } |
There was a problem hiding this comment.
Do not rely on the broad address-range check before reading the GC header.
Line 87 still accepts small bogus raw values like the documented 0xf000f on Linux-like targets because is_valid_obj_ptr only checks a broad address range there. That means line 95 can still read raw - GC_HEADER_SIZE from unmapped memory. Use a real GC/allocation-membership predicate before the header read.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/perry-runtime/src/object/polymorphic_index.rs` around lines 78 - 89,
The is_valid_obj_ptr call in the guard before reading the GC header is
insufficient because it only checks a broad address range on Linux-like targets,
allowing small bogus values like 0xf000f to pass through and cause a wild memory
read at raw - GC_HEADER_SIZE. Replace the is_valid_obj_ptr check with a proper
GC or allocation-membership predicate that actually verifies the pointer belongs
to a valid heap allocation, not just checking if it falls within a broad address
range, to prevent small bogus pointers from reaching the header dereference
operation.
added 2 commits
June 19, 2026 08:57
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/perry-codegen/src/codegen/string_pool.rs (1)
759-766:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUndefined variable
blk— compilation error.The
ctor_rest_regsloop referencesblk.call_void(...)butblkis not defined at this point. All other registration loops were updated to use the chunker pattern (chunker.roll_if_full()+chunker.current_block()), but this loop was missed during the refactoring.Proposed fix
ctor_rest_regs.sort_unstable(); for (ctor_symbol, rest_idx) in ctor_rest_regs { + chunker.roll_if_full(); + let blk = chunker.current_block(); let func_ref = format!("@{}", ctor_symbol); blk.call_void( "js_register_closure_rest", &[(PTR, &func_ref), (I32, &rest_idx.to_string())], ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/perry-codegen/src/codegen/string_pool.rs` around lines 759 - 766, The `ctor_rest_regs` loop is attempting to call `blk.call_void(...)` but the variable `blk` is not defined, causing a compilation error. This loop needs to be refactored to use the chunker pattern that was applied to other registration loops. Update the loop to call `chunker.roll_if_full()` before the operation to ensure the block is ready, then replace the undefined `blk` reference with `chunker.current_block()` to get the active block from the chunker, matching the pattern used in the surrounding registration code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/perry-codegen/src/codegen/string_pool.rs`:
- Around line 759-766: The `ctor_rest_regs` loop is attempting to call
`blk.call_void(...)` but the variable `blk` is not defined, causing a
compilation error. This loop needs to be refactored to use the chunker pattern
that was applied to other registration loops. Update the loop to call
`chunker.roll_if_full()` before the operation to ensure the block is ready, then
replace the undefined `blk` reference with `chunker.current_block()` to get the
active block from the chunker, matching the pattern used in the surrounding
registration code.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 020d0f1f-6763-41f9-a240-0a13f0b7b0f2
📒 Files selected for processing (4)
crates/perry-codegen/src/codegen/string_pool.rscrates/perry-hir/src/lower_decl/body_stmt.rscrates/perry-runtime/src/object/class_registry.rscrates/perry/src/commands/compile.rs
🚧 Files skipped from review as they are similar to previous changes (3)
- crates/perry/src/commands/compile.rs
- crates/perry-runtime/src/object/class_registry.rs
- crates/perry-hir/src/lower_decl/body_stmt.rs
added 5 commits
June 19, 2026 09:10
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.
Next.js 16 standalone app-router bring-up — codegen/runtime/HIR fixes
This branch carries the Next.js app-router compilation fixes (walls 1–5 of the bring-up). The render goes from an immediate SIGSEGV crash to booting and running deep into the request pipeline (past
IncrementalCacheconstruction when the remaining wall is worked around).Key fixes (non-exhaustive)
fix(runtime): resolve (don't construct) a class-object on a 0-arg value-call(9970fbbe7) + the construct-path companion — wall 2/4 (value is not a function/undefined.get).fix(fs): readFileSync throws Node-shaped ENOENT on failed read (not empty string)(af8c832b0) — wall 3.fix(hir): forward captured enclosing locals to ctor for directly-constructed anonymous class expressions(6c41417ff) — wall 5.fix(transform): scan class field inits + extends in compute_max_local_id(cbfd6d097) — LocalId collision, parallel to compilePackages: hono app.request() resolves to undefined (reading 'status') #5143.fix(hir): scope-aware native-module shadowing(58d8c77b2);fix(runtime): js_object_get_index_polymorphic primitive-receiver SIGSEGV guard(0ba3b22fc);fix(nextjs): walls 1-4(29425f555); plus a series of HIR scope/shadowing/hoisting and runtime descriptor fixes.Remaining blocker (filed separately)
The 6th wall is #5437 — a deferred/adopted
requirebinding captured by value (as an unresolved thunk) by a class constructor (IncrementalCache→new uw.SharedCacheControls). Root cause + candidate fixes + a deterministic full-bundle repro are in that issue. It is not addressed here.Notes for merge
mainby a few commits; rebase or 3-way merge at your discretion (linear history / squash-or-rebase enforced).Summary by CodeRabbit
Release Notes
.next/server/**deferred module initialization via path-based runtime registration, including on-demand JSON disk loading.super()/ inherited constructor dispatch for dynamic parents.thisbinding and improved filesystem read error reporting.