Merge train 218: shadowed inherited fields, new(X) shadowing, class-ctor arguments, variable-box release, native-base subclass prototypes and field init, Tier A binding removal (v0.5.1596) - #10652
Merged
Conversation
A regex replace with a function replacement built a fresh JS array per match to carry the callback's arguments: `js_array_alloc(0)`, then a push per argument that reallocated as it grew, each push opening a handle scope and a caught frame. `call` then read the whole array straight back out into native slots and dropped it. No user code could observe it -- except through a proxy replacer, whose `apply` trap does receive the arguments as an array. The slots `call` builds are already GC roots: it binds each one to the shadow stack before invoking the replacer. Binding them first and writing the arguments into them afterwards removes the array entirely. A value is rooted from the moment it is written, so producing the next argument may allocate and collect, which is what copying a match's capture strings does. The buffer is sized once from the program's capture count and reused across matches. A proxy replacer keeps the array path. Instructions per workload, control subtracted, same commit, release build, over 1.1-1.5M character subjects with ~200,000 matches each: replace with callback, ASCII 70,896,578,215 -> 51,624,603,466 -27.2% replace with callback, Unicode 79,956,564,097 -> 61,666,635,190 -22.9% replace with a string template 28,421,246,293 -> 28,429,203,739 +0.0% That is 30.3x -> 22.1x Node 26.5.1 on the ASCII row and 21.1x -> 16.3x on the Unicode one. The template row is the control: it does not take this path and does not move. A profile attributes the saving. Before, the callback path spent 40% of its instructions in the collector and 11.7% in JS array operations against a template path that spent ~0% in array operations; the regex engine did the same work in both (8.2B vs 7.3B instructions), which is what says the difference is host machinery rather than matching. `tests.rs` gains a regression test with a measured reason to exist: with the reset of the reused buffer removed, the whole 3,984-test lib suite still passed, and `(a)|(b)` over "aba" -- where each match leaves a capture unset that the previous match set -- returns `<a,undefined><a,b><a,b>` instead of `<a,undefined><undefined,b><a,undefined>`. The test asserts it reached the direct path, so it cannot quietly pass against the exec-object fallback.
…slot
An overridden field (`class Sub extends Base { tag = ... }` where `Base`
also declares `tag`) is not deduplicated in the packed inline-slot layout:
the object holds one slot per declaration, ancestor first. The
compile-time-typed read path already resolves to the most-derived slot
("TS shadowing"); every dynamic by-name lookup returned the first
(ancestor's, never-written) slot instead -- observable from an inherited
accessor's `this.field`, a computed `obj[key]` read, and Reflect/has-own
checks.
…d function ctor lower_new_impl_inner called lower_builtin_new for any class_name absent from ctx.classes before checking import_function_prefixes. Imported classes already land in ctx.classes and skip the builtin block; an imported plain function constructor (Headers, EventEmitter-shaped, ...) never does, so any unconditional builtin arm (not gated by required_sources) fired regardless of whether the callee was a bare identifier or wrapped in (X as any) -- peel_new_callee strips that cast before lower_new branches on callee shape.
…(X as any)() Gap test: Headers/EventEmitter/Stream x function/class declaration x named/default import x plain-new/cast-new, plus a local-alias control that already worked. Property-based discriminators, not instanceof -- #10477 (imported non-class instanceof) is not yet fixed on this base and would conflate the two bugs. Unit tests: direct Expr::New harness asserting js_new_function_construct fires (not the builtin arm) once a name resolves to an imported function constructor, the builtin still fires when unshadowed, a V8-fallback import of the same name still falls to the builtin, and an imported CLASS of the same name already shadowed the builtin before this fix (regression guard).
…#10362) The registry holding an explicit [[Prototype]] for a non-meta-capable owner was gated only by OBJECT_PROTOTYPES_NONEMPTY, a process-global latch. One re-prototyped object anywhere armed it for the rest of the run, after which every traced owner-capable cell paid a lock plus a SipHash probe to ask a question that is false for almost all of them. A latch is a cliff: it turns the fast path off for every cell at once, invisibly to any benchmark that does not contain the trigger. Bit 6 of _reserved is OBJ_FLAG_NULL_PROTO, which has exactly one setter (returning *mut ObjectHeader) and seven readers, every one provably unreachable with a non-GC_TYPE_OBJECT cell: three by an explicit obj_type check, three by a converter that returns None first, one by a preceding conjunct in the same && chain. The registry excludes GC_TYPE_OBJECT by construction, so the bit is free across the registry's whole population, not only for arrays -- which is why both existing witnesses exercise it, one of them a lazy array that an array-scoped bit would have missed. GC_RESIDUAL_PROTO_OWNER is set at the single funnel, under the registry lock and before the insert: the proof is published before the fact it guards. It is never cleared, and that is sound. Entries outlive owners only when the owner is dead; the prune touches only dead owners; both rekey paths keep the entry while _reserved rides the move (#10381's contract, enforced by assert_relocation_copied_the_header). One writer under one lock writes both, so the dangerous direction -- entry present, bit absent -- has no producer. A GC_TYPE_OBJECT owner that reaches the registry anyway keeps the latch-only gate, since bit 6 means something else there. The latch stays as the first test -- one byte load, false for any process that never re-prototyped a non-object -- and the bit is the second, which is what stops an ARMED process paying per traced cell. Sabotage: with the setter made a process-wide no-op, both existing witnesses in gc/tests/residual_prototype_relocation.rs fail at their real verdicts, the registry entry no longer following the lazy header nor the array owner. The bit is load-bearing, not decorative. Measured on main 9df5075, exact instruction counts: the fixture that arms the latch -0.408%, and three that do not are flat (+0.015%, -0.060%, -0.021%). Attributed: -94.3M RandomState::hash_one, -58.2M SipHash write, -36.9M run_copied_minor_attempt, -30.0M transfer_residual_prototype. pointer_slots_read is identical between arms: the collector does bit-identical work.
…ect the call site HIR stops padding a new-site's argument list to the declared arity for a constructor that reads `arguments` (`monomorph/defaults.rs`) -- an appended `undefined` was indistinguishable from one the caller wrote. Runtime: the four dynamic-construct paths (super-apply caps arm, flat-ctor replay, and both class-object/registered-class replay paths) now share `constructor_user_arg_slots`, which packs the synthesized `arguments` slot from every call arg instead of binding it like a user `...rest` (only the args past the declared count) -- construction through a value, an imported class, or a CommonJS class all saw an empty `arguments` before this. Codegen: constructor ABI (`CtorAbi`: param count, has-rest, has-synthetic- arguments) is read from the constructor's fixed/rest/arguments layout instead of inspecting only its last declared parameter, which missed every capturing constructor -- i.e. every CommonJS class, since Perry adds capture params mechanically. The ABI threads through constructor-contract resolution (so a no-own-ctor forwarder inherits its ancestor's full ABI), imported-class metadata, and cross-module `new`-site arg marshaling, which can now pack up to two trailing arrays (a user rest, then `arguments`) instead of assuming at most one.
A let/var a closure captures and something reassigns lives in a malloc-side box cell, and every registered cell is a strong GC root (scan_box_roots_mut). Only the async-to-generator transform's terminal Stmt::ReleaseBoxes ever released one (#7933/#8208/#8303); an ordinary function, method, arrow, generator, or an async function with no await leaked one registered root per boxed binding per call, plus everything that binding last pointed at. Codegen registers every entry slot holding a cell this frame minted (stmt/boxed_frame_release.rs, new) and the existing return-site rewrite that already injects js_shadow_frame_pop now also emits js_box_scope_release before every ret; a declaration inside a loop releases the previous iteration's cell before minting the next. Runtime (box/scope_release.rs, new): a cell no closure captured publishes immediately; a captured cell is marked frame-released in its capture-edge record and published only when its last capture edge dies via the existing dead-owner pruning -- the same escape contract #8303 built for async activations. Two holders the runtime cannot count keep their cells instead of double-releasing: a sloppy-mode mapped arguments object, and a plain-async step closure's own activation cells; a step closure's capture of an enclosing frame's cell is now counted, since the activation token never covered it. Fixes a latent GC hole shared with #8303 in the same commit: a full trace that stops rooting a released cell must still keep it in BOX_YOUNG_ROOTS while young, because a minor walks only that log -- dropping it left the next minor with no root for a payload a live closure still reads.
…in prototype
class Sub extends EventEmitter {} left Sub.prototype's [[Prototype]] on
Object.prototype instead of EventEmitter.prototype. class_decl_prototype_value
resolves a registered parent class id by recursing into itself, which bails
for a RESERVED native-builtin parent id (builtin_parent_reserved_class_id in
perry-codegen wires this edge for a native base with no declared-class
registration), silently falling through to the Object.prototype default.
Resolve EventEmitter/EventEmitterAsyncResource's real, closure-identity-keyed
prototype object through the same js_function_prototype_value_for_read path
the existing runtime-function-valued-parent branch already uses, so
Object.getPrototypeOf(Sub.prototype) === EventEmitter.prototype holds by
identity. Fixes #10599.
…s-parent id builtin_parent_reserved_class_id (perry-codegen) already gained an "EventEmitter" => 0xFFFF0076 entry (#10592), but not its AsyncResource variant: class Sub extends EventEmitterAsyncResource {} left get_parent_class_id(Sub) unresolved entirely (no parent-edge call is ever emitted), so the perry-runtime getPrototypeOf-identity fallback for reserved native-builtin parents (#10599) never runs for it -- the same gap #10592 closed for plain EventEmitter, one id over.
Covers: direct subclass with field+ctor, fieldless no-ctor subclass, two-level (indirect) subclass, unnamed and named-via-indirection class expressions, EventEmitterAsyncResource, in/for-in walking the same chain, own-enumeration non-regression, dispatch still works, an instanceof-still-holds control (guards #10592), and a class-extends-Array control (dedicated ArrayHeader path, unaffected by this fix). Verified: fails on the runtime fix alone (state.rs reverted, standalone) with every getPrototypeOf/instanceof/`in` assertion false where Node says true; passes with both the runtime fix and both codegen table entries. Two pre-existing, unrelated gaps hit while writing this were deliberately left uncovered (documented inline, not fixed here): EventEmitterAsyncResource.prototype's own [[Prototype]] does not chain to EventEmitter.prototype (a native-to-native link, not a user `extends` subclass), and Object.keys(new Sub()) leaks EventEmitter's prototype methods as literal own enumerable instance properties instead of Node's real _events/_eventsCount/_maxListeners own fields (CLAUDE.md "Native base-class subclassing -- a native base's surface is installed at super() time"). Both reproduce identically with the fix reverted, so neither is caused by it.
Class members nested in a function or CommonJS module body captured enclosing var/let bindings by value snapshot instead of by reference: a method saw the value the var had at class-declaration time, and writes from constructors/static methods to a captured var were silently lost. The desugar_shared_mutable_captures pass (#5951's box-sharing machinery) decided whether a captured id was safe to box by counting Let declarations per LocalId and requiring exactly one; a var is declared twice in HIR (a body-entry predefine slot, then the declaration statement itself), so every var capture was rejected as ambiguous and fell back to the stale value-snapshot path. Two functions that each declared a same-named var plus a same-named class could also collide. Replace the declaration counter with a DeclCensus/CensusWalker that walks a region in execution order and asks whether an id denotes ONE binding (a single declaration, or several redeclarations in the same closure scope under the same name where the first dominates) rather than requiring literally one declaration. Redeclarations of an already-captured id are demoted so the box, not a fresh local, is written. Captured cells now propagate to nested classes explicitly. Fixes #10485 Fixes #10489
… typed Map/Set receivers - #10443: a class whose direct parent is a built-in Error type (or any non-user base) never ran its own field initializers when it had its own super()-calling constructor; the Error arm of this_super_call.rs was the one arm that skipped applying them. - #10446: .add/.set/.get/... on a statically-typed Set/Map receiver lowered straight to js_set_*/js_map_* with no tag check, so an undefined/null/primitive receiver dereferenced its unboxed payload and segfaulted instead of throwing a catchable TypeError.
Removes the bare-name node-fetch alias binding and the vestigial in-tree accounting for the tursodb/iroh native bindings, whose actual implementations already moved to @perryts/tursodb and @perryts/iroh in v0.5.557. See changelog fragment for details.
Fetch's manifest entries stay (internal dispatch tag for the built-in Web Fetch API), so mark it in the test-only INTERNAL_MODULE_KEYS allowlist now that it is no longer a NATIVE_MODULES import specifier.
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (129)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This was referenced Sep 18, 2026
fix(codegen): run field initializers at native-base super() and guard typed Map/Set receivers
#10617
Closed
Closed
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.
This train lands 10 PRs as v0.5.1596. Each source commit is verified to preserve its patch-id and authorship.
#10595) — resolves a shadowed inherited field to its own value.#10589) — stopsnew (X)picking a builtin over a same-named local.argumentsin class constructors reflect the call site #10612 (#10484) — makesargumentswork in class constructors.#10464) — releases a frame's variable-box cells.#10599) — links a native-base subclass prototype to the real builtin prototype.#10485,#10489) — captures class-member enclosing vars by reference.#10443,#10446) — runs field initializers at native-basesuper().This train's gap phase was re-run, because the first pass was vacuous
The first run reported
unexplained_regressions={}having executed zero tests. All ten areas exited rc=2: this train validated in a newly-built second environment whose worktree had nonode_modules, and the harness refused to run rather than misreportERR_MODULE_NOT_FOUNDas a Perry regression — the right call, and the reason the emptiness was recoverable rather than a false green that shipped.The driver's liveness assertion did not catch it. It compared what a filter selected against what
test_gap_*matching predicts and assertedran <= gate_scope, which zero satisfies: the upper bound was guarded and the lower bound was not. That is the "gate runs, subject never did" shape, and it is now closed — the check requiresrc != 2,ran > 0andran <= gate_scope, and counts DISTINCT test names rather than progress lines, because the harness can emit progress for more than one pass in a single invocation.Both already-merged trains were checked rather than assumed: 216 and 217 each had zero vacuous areas, every area matching its gate scope exactly.
Validation
Validated head
90c8943bb5. Five-package release build pinned and hash-verified.test_gap_*count —emitter3/3,instanceof13/13,arguments7/7,prototype,timer3/3,regex11/11,replace9/9,field25/25,capture13/13,fetch9/9. No unexplained regressions.lintwas 4 of 83 on the first pass, not the usual 1. Three were real or environmental and are resolved: two unnecessaryunsafeblocks in fix(hir,codegen,runtime): makeargumentsin class constructors reflect the call site #10612's test code failed-D warnings --all-targets(fixed here, and the runtime suite re-run at the fixed head), andRegenerate API docs/API docs driftfail only in a worktree whosetarget/is not in-tree — a local artifact of this train's validation environment, not a repo-wide red. What remains is the known public-benchmark freshness step.Two owner-approved compute trades
Both were measured by their authors in instructions and put to the owner with those numbers before merging.
arguments(12.390B → 14.863B, 2M iters); +2.9% always-paid on dynamic constructionargumentsat all — that is the bug — and its output diverged from Node by 4000; the fix is byte-identical to Nodevarcaptured from a function-nested class)Off the affected paths both measure as noise (−0.04% closure-heavy, +0.05% compile time). Tracked as #10594, #10602, #10616.
Pin provenance
The gap phase ran against artifacts pinned at
ae084402f2; the head then moved by exactly two lines, both inside#[cfg(test)] mod constructor_arg_slot_tests(verified by brace walk to span 1434→EOF), which the release build excludes. A rebuild at the new head hashes differently forperry/libperry_runtime.a/libperry_stdlib.awhilelibperry_ext_net.aandlibperry_ui_macos.amatch — the three that moved are exactly the crates built from perry-runtime source, and the only source delta is test-gated, so this is Rust build non-reproducibility atcodegen-units=16rather than a content change.Issues closed by this train
A merge train closes its source PRs rather than merging them, so the
Fixes #Nkeywords in those PR bodies never evaluate. They are carried here, on the PR that actually merges, so they fire:Fixes #10595
Fixes #10589
Fixes #10484
Fixes #10464
Fixes #10599
Fixes #10485
Fixes #10489
Fixes #10443
Fixes #10446
Summary by CodeRabbit
Bug Fixes
argumentsbehavior, inherited field shadowing, captured class variables, native subclass prototype identity, and field initialization for built-in error subclasses.TypeErrors instead of crashing.Performance
String.prototype.replacecallback performance.Breaking Changes
fetch,tursodb, andirohnative bindings.Chores