chore: merge train 223 (v0.5.1602) - #10737
Merged
Merged
Conversation
…t heritage shape
class X extends AsyncLocalStorage threw "Class constructor
AsyncLocalStorage cannot be invoked without 'new'" at super() for every
heritage shape except a bare import { AsyncLocalStorage } from
"node:async_hooks" binding -- the same defect #10621 fixed for
AsyncResource (#10453). A local alias, a namespace member, a default
import, and a CJS destructured require() all reach the identical bound
native export value the bare import does, but only that shape is
recognized statically at HIR-lowering time
(crates/perry-hir/src/lower_decl/class_decl.rs), which routes to
perry-stdlib's js_async_local_storage_subclass_init via a
codegen-declared extern symbol. Every other shape fell through
js_fetch_or_value_super to a plain CALL of the bound export.
Unlike AsyncResource, whose implementation lives entirely in
perry-runtime, AsyncLocalStorage's subclass-init helper lives in
perry-stdlib (it needs the stdlib Handle registry), and perry-runtime
cannot depend on perry-stdlib. Route through a registration hook
perry-stdlib installs at startup (JS_NATIVE_ASYNC_LOCAL_STORAGE_SUBCLASS_INIT),
matching the existing JS_NATIVE_ASYNC_HOOKS_CONSTRUCT /
JS_NATIVE_EVENTS_CONSTRUCT pattern already used for this exact kind of
cross-crate reach.
Adds test_gap_10625_asynclocalstorage_heritage.ts covering the canonical
import (control), local alias, namespace member, default import, and
two CJS require() shapes, asserting a real run()/getStore() round-trip
through the subclass -- not just that construction doesn't throw.
Every static-key class-field read gated its fast path on `@PERRY_CLASS_FIELD_INLINE_GUARD_DISABLED`. That is an `external global`, so on arm64 reading it costs `adrp` + a GOT `ldr` + a dependent `ldrb` through it + a compare — four instructions and TWO dependent loads, in the gate block, before the guard has looked at the receiver at all. It cannot be hoisted: the runtime flips it mid-execution when a descriptor or accessor lands on a class prototype, so the load is `volatile` by necessity. The authority moves onto a value the guard already had to load. Each class gains `@perry_class_guard_shape_*`, seeded at module init with the same ShapeId as `@perry_class_shape_id_*` and registered with the runtime; `disable_class_field_inline_guard` now poisons every registered slot with `u32::MAX`. ShapeIds are allocated from `[0x8000_0000, 0xC000_0000)` and never reused, so a poisoned expectation can never match a live object — every guard misses and routes to the IC, which is exactly what the latch bought. The expectation is read volatile per access for the same freshness reason the latch was, and is still cheaper: one module-local `adrp`+`ldr`, no GOT hop. It is a SEPARATE global from the ShapeId on purpose. `js_object_alloc_class_inline_keys_stamped` stamps every new instance with the value read out of the ShapeId global (`lower_call/new_alloc.rs`), so poisoning that one would brand live objects with a bogus ShapeId instead of closing a fast path. Subclass arms move to the poisonable global too, or a subclass receiver would keep hitting the fast path after a flip. Imported-class stubs carry the expectation through `js_register_imported_class_shape_slot`, and a rewrite that lands after a disable re-poisons rather than resurrects. Measured, arm64, `-Os` + `llc -O2 -mcpu=apple-m1`: - one `o.a` on a typed receiver, executed fast path: 28 -> 23 instructions (-18%), one fewer dependent load; - 16 reads on one receiver, EXECUTED instructions per call (`/usr/bin/time -l`, best-of-5, differential over iteration count): 595.2 -> 563.0, -5.4%. The per-read marginal is -2 rather than -5 because LLVM already hoisted the latch's base register across accesses within a function. Unlike a clone-gated optimisation this fires wherever the guard does: the latch is gone from `$generic`, `$spec_b` AND the copy the inliner leaves in the caller, which is the code that actually executes. `js_class_field_get_ic`'s truthful ShapeId operand is now loaded in the cold miss arm instead of the function entry, since the fast path no longer reads it. The two updated ratchet tests pin both halves of the swap — three loads in the guard AND no latch — because "three loads" alone would also pass a lowering that kept the latch and added the expectation.
`perry-codegen` does not depend on `perry-runtime` — its deps are perry-hir, perry-dispatch and perry-api-manifest. Yet the compiler bakes the collector's header layout into emitted code twice over: the inline `new` path stores a packed `GcHeader` word as a COMPILE-TIME constant (#8122, pre-composed per class into `@perry_class_header_image_*`), and every class-field / element-shape / method-probe guard masks that word against a literal. So both sides carry their own `GC_TYPE_OBJECT`, `GC_FLAG_FORWARDED`, `OBJ_FLAG_HAS_DESCRIPTORS`, `GC_OBJ_TYPED_LAYOUT_INTACT` — 36 restatements across 10 files — held together by a code comment. Nothing enforced it. What looks like enforcement is const GC_FLAG_FORWARDED_I8: &str = "-128"; debug_assert_eq!(GC_FLAG_FORWARDED_I8, "-128"); which compares codegen's constant to a string literal: a tautology that never references the runtime, and is compiled out of `release` and `perry-dev` besides. Every codegen test naming these bits asserts codegen's own constant reaches the IR, so they pin codegen to itself and would all stay green. A flag renumbered in perry-runtime therefore compiled clean, passed every suite, and shipped a compiler whose inline allocator baked one bit layout while the collector read another — objects born with flags the GC misreads, which CLAUDE.md describes as surfacing cycles later as `TypeError: value is not a function`. `scripts/check_gc_header_constants.py` re-derives every restatement from the runtime constant it quotes, including the composites (`READ_FAST_PATH_BLOCKED` = ARRAY_DESCRIPTORS|HAS_DESCRIPTORS, the fused 32-bit masks `ELEM_HEADER_MASK` / `GC_OBJECT_METHOD_GUARD_MASK_I32`). A registered constant that stops existing FAILS, so a fix must delete its own entry; a new header-shaped `const` in a watched file must be registered or exempted with a reason, so the next one cannot arrive silently. `--self-test` proves it can fail; `--list` prints the whole duplicated surface, including the 6 constants deliberately out of scope. Build-free, so it joins `lint`, a required context. Writing the registry found 5 restatements a manual grep missed (they are declared inside function bodies, not at module scope): the array-literal allocator's three, and two copies of the method-probe fused mask. Two existing gates moved with it, both of which correctly caught this branch: `gc_store_site_inventory` wanted GC_STORE_AUDIT markers on the new raw slot writes (POINTER_FREE — a `u32` in the program's data segment, never a heap edge), and `shape_descriptor_census` pinned the precheck's old `expected_class_identity(..., expected_shape_id)` spelling. The census is updated to the new spelling AND strengthened: it now also requires the expectation to be read VOLATILE from the poisonable global, because a lowering that hoisted that load would reopen a fast path the runtime has closed and would still satisfy a shape-only assertion. Verified to fail when the `volatile` is dropped. This does not make the GC header bits movable — it makes moving them a red build instead of a silent miscompile.
`restore_class_guard_shapes_for_test` is reached only from `test_reset_class_field_inline_guard`, which is `#[cfg(test)]`, so in an ordinary build it is dead code and `-D warnings` rejects it. Gate it the same way its only caller is gated. Caught by the `warnings` job, which the local run that cleared this branch had skipped: it was invoked with SKIP_COMPILE_GATES=1, and that tier IS the `warnings`/`check` jobs.
…declaration merges
`axios` throws `TypeError: Class extends value is not a constructor` at
module-init time via its `https-proxy-agent` -> `agent-base` dependency
chain. `agent-base`'s `src/index.ts` merges `namespace createAgent { export
class Agent extends EventEmitter { ... } }` onto a same-named `function
createAgent()` and exports the result with TS's `export =` form.
`perry.compilePackages` prefers compiling a package's raw TypeScript source
over its published JS emit, and picks that file. Perry's HIR lowers the
namespace's exported `Agent` class as a static-field-set against a synthetic
class entity that is not the same runtime value `export =` ends up
exporting, so `require("agent-base").Agent` reads back `undefined` and the
downstream `class HttpsProxyAgent extends agent_base_1.Agent` throws.
`is_hybrid_cjs_emit_input` (resolve.rs) already falls back to a package's
compiled JS emit for one other TS-source shape Perry can't correctly lower
(#6586's ESM+CJS-epilogue hybrid). Extend it with a second, narrowly-scoped
trigger: a top-level `namespace`/`module` block (excluding ambient `declare
namespace`, which is type-only) combined with a top-level `export =`
statement. Node can't run this non-erasable TS syntax directly either
(`--experimental-strip-types` rejects `namespace`/`export =`), so a package
built this way is never executed from its raw `.ts` source in practice —
falling back to the compiled emit matches what Node actually runs, instead
of attempting to implement namespace/function declaration-merging semantics
in HIR.
Fixes #10662
This was referenced Sep 19, 2026
|
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 (37)
✨ 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 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
2 participants
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.
Merge train 223 — three PRs validated together as one tree, released as v0.5.1602.
Trains land as their own PR, so the source PRs are closed, not merged, and their close-keywords never fire. Issues resolved are listed at the bottom.
Contents
fix(runtime): routeAsyncLocalStoragesuper()through any bound-export heritage shapeperf(codegen,runtime): retire the per-access class-field latch, and gate the compiler's copy of the GC header layoutfix(compile): fall back to compiled JS emit for TS namespace/export=declaration mergesNotes on assembly
#10634 and #10649 are a stack, not independent PRs. Both append a heritage-recognition arm to the same region of
crates/perry-runtime/src/object/global_this/fetch_globals.rs, so only one can ride a given train — #10634 cherry-picked clean, then #10649 conflicted on top of it. #10649 follows in a later train, rebased onto this one's result rather than onto #10634's branch head, since this lands as a squash and rebasing onto the pre-squash lineage would guarantee a second rebase.That file has now produced the same conflict three times, and it carries a specific hazard worth recording: when both sides append a block ending in a coincidentally identical
}, git folds that brace into the common region. Accepting both sides then leaves the earlier arm unclosed and a stray}below — with no conflict markers remaining and a plausible-looking diff. Counting braces is a weak check;cargo fmtparses, so it is the real gate. It is green here.#10646 carries 6 lines that main has since deleted. The assembly proof reports them as absent from the train, which is correct:
50951a7188removed that code after the PR branched, so the train holds main's newer state and the PR is the stale side. Classified as superseded rather than dropped; 0 lines were actually dropped from any of the three PRs.Validation
Assembled on
7c5d04d0ea; source heads asserted unchanged since assembly; every PR proven fully represented by patch-id and by subject+author rather than by a pathspec diff.Green:
fmt,file_size,raw_handle(self-test, ceilings, vs-main),gc_runtime_root_holders,check_test_registration,addr_class_inventory,cargo check --workspace --all-targetsunder-D warnings, the release build of all five pinned artifacts, every unit suite, the derived integration suites, and a 13-area gap sweep with each area asserted to have run a non-zero number of tests.This train's gap sweep runs with
PERRY_RUN_TIMEOUT=30rather than the harness default of 10s. The default is close enough to a real compile-and-run that a loaded machine turns a pass into a timeout, and the harness classifies a timeout identically to a failure — so the default makes slowness and breakage indistinguishable. It does not substitute for running sweeps sequentially, which is also done.Issues resolved
Closes #10625
Closes #10662