Skip to content

fix(hir,runtime): native chained-static-class decorators + reflect-metadata require binding (NestJS bootstrap) - #5721

Merged
proggeramlug merged 2 commits into
PerryTS:mainfrom
proggeramlug:fix/nestjs-walls-2-3
Jun 27, 2026
Merged

fix(hir,runtime): native chained-static-class decorators + reflect-metadata require binding (NestJS bootstrap)#5721
proggeramlug merged 2 commits into
PerryTS:mainfrom
proggeramlug:fix/nestjs-walls-2-3

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

Two independent codegen/runtime bugs that block a NestJS app from booting natively. Both fire at module-evaluation time (before any user code), discovered while bringing up the official nestjs/typescript-starter under perry.

Wall 2 — chained static-class self-alias → TypeError: Cannot read properties of undefined (reading 'value')

@nestjs/common's Logger is emitted by tsc as a decorator-captured class: let Logger = Logger_1 = class Logger {…}. The self-alias reference inside a member body lowered to a capture, forcing the class onto the per-evaluation ClassExprFresh path — whose fresh class object never resolved static methods/accessors/descriptors from the class registry. So Object.getOwnPropertyDescriptor(Logger, 'error') returned undefined, and NestJS's decorator doing Reflect.defineMetadata(..., descriptor.value) threw.

Fix: HIR resolves a class self-alias to ClassRef (off the fresh path); runtime resolves static methods/accessors/own-descriptors via the class registry for ClassExprFresh heap class objects (guarded for the tagged class-object form).

Wall 3 — require("reflect-metadata")ReferenceError: _req_1 is not defined

@nestjs/common/index.js does require("reflect-metadata"); the CJS wrap hoists it to import _req_1 … + a return _req_1 shim, but module_decl special-cased reflect-metadata with an early return that never bound the default-import local.

Fix: bind reflect-metadata import specifiers to {} (the require() result is always discarded — the real effect is the global Reflect polyfill perry already provides). Bare import "reflect-metadata" is unaffected.

Verification

  • Upstream nestjs/typescript-starter: both module-eval crashes gone; the app now advances to the express HTTP-adapter load.
  • cargo test -p perry-hir (388) and -p perry-runtime (1082) green; no regressions.

6 files, +228 lines. Follow-up to #5710 (independent files).

https://claude.ai/code/session_017S2d3tRok3oMbi6AwfJyUU

Summary by CodeRabbit

  • Bug Fixes
    • Fixed class self-references in chained-assignment class patterns so identifiers resolve to the correct class reference during lowering.
    • Improved CommonJS require("reflect-metadata") module behavior when re-exported, ensuring all specifiers get initialized locals to avoid undefined references.
    • Corrected property descriptor and field/property lookup for fresh per-evaluation class constructors, restoring expected static method and static accessor results.

…tadata require binding (NestJS bootstrap)

Two independent bugs that block a NestJS app from booting natively (both
hit at module-evaluation time, before any user code runs).

1. Wall 2 — chained static-class self-alias (`reading 'value'` crash).
   tsc emits decorator-captured classes as `let Logger = Logger_1 = class Logger {…}`.
   The self-alias reference inside a member body lowered to a capture, forcing
   the class onto the per-evaluation `ClassExprFresh` path, whose fresh class
   object never resolved STATIC methods/accessors/descriptors from the class
   registry. So `Object.getOwnPropertyDescriptor(Logger, 'error')` returned
   undefined and NestJS's decorator (`Reflect.defineMetadata(..., descriptor.value)`)
   threw "Cannot read properties of undefined (reading 'value')".
   Fix: HIR resolves a class self-alias to `ClassRef` (off the fresh path);
   runtime resolves static methods/accessors/own-descriptors via the registry
   for `ClassExprFresh` heap class objects (guarded for the tagged class-object form).

2. Wall 3 — `require("reflect-metadata")` (`ReferenceError: _req_1 is not defined`).
   `@nestjs/common/index.js` does `require("reflect-metadata")`; the CJS wrap
   hoists it to `import _req_1 ...` + a `return _req_1` shim, but module_decl
   special-cased reflect-metadata with an early return that never bound the
   default-import local. Fix: bind reflect-metadata import specifiers to `{}`
   (the require result is always discarded; perry already provides the global
   `Reflect` polyfill). Bare `import "reflect-metadata"` is unaffected.

Verified on the upstream NestJS typescript-starter: both module-eval crashes
are gone (app advances to the express HTTP-adapter load). cargo test -p
perry-hir + perry-runtime green.

Claude-Session: https://claude.ai/code/session_017S2d3tRok3oMbi6AwfJyUU
@coderabbitai

coderabbitai Bot commented Jun 26, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 62213280-e73d-4b36-b574-69796b27a1d8

📥 Commits

Reviewing files that changed from the base of the PR and between ab26853 and 2911a15.

📒 Files selected for processing (4)
  • crates/perry-hir/src/lower/lower_expr/arm_ident.rs
  • crates/perry-hir/src/lower/stmt.rs
  • crates/perry-runtime/src/object/descriptors.rs
  • crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
🚧 Files skipped from review as they are similar to previous changes (4)
  • crates/perry-hir/src/lower/stmt.rs
  • crates/perry-hir/src/lower/lower_expr/arm_ident.rs
  • crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
  • crates/perry-runtime/src/object/descriptors.rs

📝 Walkthrough

Walkthrough

The PR updates HIR lowering for chained class aliases and reflect-metadata imports, and extends runtime object lookup and descriptor handling for fresh class-expression values.

Changes

HIR lowering and runtime class property handling

Layer / File(s) Summary
Record chained aliases
crates/perry-hir/src/lower/stmt.rs, crates/perry-hir/src/lower_decl/body_stmt.rs
let and var lowering now record chained assignment targets for nested class expressions before later lowering steps run.
Resolve alias identifiers
crates/perry-hir/src/lower/lower_expr/arm_ident.rs
Identifier lowering returns Expr::ClassRef when a recorded alias resolves to the class currently being lowered.
Synthesize reflect-metadata locals
crates/perry-hir/src/lower/module_decl.rs
The reflect-metadata export branch defines missing mutable Any locals for import specifiers and initializes them with empty objects.
Fresh class field lookup
crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
js_object_get_field_by_name adds a raw-pointer class-expression branch that checks own fields first and then resolves static methods or accessors from the class registry.
Fresh class property descriptors
crates/perry-runtime/src/object/descriptors.rs
js_object_get_own_property_descriptor returns a bound static-method descriptor for fresh class objects when the queried key is not already an own property.

Sequence Diagram(s)

sequenceDiagram
  participant lower_body_stmt
  participant record_chained_class_self_aliases
  participant LoweringContext
  participant lower_ident_expr
  lower_body_stmt->>record_chained_class_self_aliases: scan chained assignment declarator
  record_chained_class_self_aliases->>LoweringContext: store class_expr_aliases entries
  lower_ident_expr->>LoweringContext: read current_class and aliases
  LoweringContext-->>lower_ident_expr: return ClassRef for alias target
Loading
sequenceDiagram
  participant js_object_get_field_by_name
  participant get_field_by_name_object_tail
  participant class_registry
  participant js_class_method_bind
  participant class_static_accessor_getter_value
  js_object_get_field_by_name->>get_field_by_name_object_tail: own-field lookup
  get_field_by_name_object_tail-->>js_object_get_field_by_name: own value or undefined
  js_object_get_field_by_name->>class_registry: resolve static member for fresh class pointer
  class_registry-->>js_object_get_field_by_name: method or accessor metadata
  js_object_get_field_by_name->>js_class_method_bind: bind static method
  js_object_get_field_by_name->>class_static_accessor_getter_value: read static accessor getter
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

Hop hop, I traced the class-name thread,
And fresh class members now walk ahead.
Reflect-metadata hums along,
While bound methods sing a tidy song.
🐇✨

Possibly related PRs

  • PerryTS/perry#5325 — Also changes crates/perry-hir/src/lower_decl/body_stmt.rs with class-related lowering behavior that affects name resolution during module/class lowering.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the main change: fixing NestJS bootstrap issues in HIR and runtime.
Description check ✅ Passed The description covers the summary, concrete fixes, related context, and verification, but it is not formatted to the repository template.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 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-hir/src/lower/lower_expr/arm_ident.rs`:
- Around line 58-61: Compare the class alias against the resolved current class
name in arm_ident lowering. In `lower_expr::arm_ident`, the `ctx.current_class`
check should use `ctx.resolve_class_name(...)` before comparing to the recorded
alias target, since `class_expr_aliases` may store the source name while the
current class is collision-renamed. Update the `if let Some(cur)` /
`ctx.class_expr_aliases.get(&name)` branch so the equality check uses the
resolved class name, preventing the alias from falling through to local or
capture lowering.

In `@crates/perry-hir/src/lower/stmt.rs`:
- Around line 120-121: The chained-assignment handling in stmt lowering is too
broad: it currently records every target in a `let X = Y = class ...` chain as a
class self-alias, which makes `lower_ident_expr` rewrite unrelated `Y`
references inside the class to `ClassRef`. Update the logic in `chained_targets`
handling within `lower_stmt`/`ctx.class_expr_aliases` so only proven
compiler-generated self aliases are registered, and leave ordinary chained
targets as normal captures or mutable bindings. Use the existing
`class_expr_aliases` and `lower_ident_expr` flow to keep the fix localized.

In `@crates/perry-runtime/src/object/descriptors.rs`:
- Around line 140-143: Avoid converting Symbol keys to strings in the
class-object metadata path: in descriptors.rs, the `is_class_object_value`
branch should not call `metadata_key_to_string(key_value)` before the
symbol-specific own-property logic later in `get_own_property_descriptor` runs.
Update the `class_registry`/static method descriptor handling so it only
proceeds for non-Symbol keys, and let Symbol keys fall through to the existing
symbol descriptor path instead of being coerced into string-named descriptors.
- Around line 146-159: The fresh-class handling in the descriptor lookup only
checks class_has_own_static_method, so static accessor properties are missed and
Object.getOwnPropertyDescriptor returns undefined for them. Update the
fresh-class branch in descriptors.rs around the class_id/method_name logic to
also detect own static accessors on the class object, using the same
descriptor-building path as the class-ref branch (or equivalent accessor
descriptor construction) so static get/set members are mirrored for fresh class
objects.

In `@crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs`:
- Around line 46-50: The handle check in get_field_by_name currently lets values
in the 0x10000–0x0FFFFF range reach is_class_object_ptr, where they may be
treated as heap pointers too early. Update the pointer-probe guard in
get_field_by_name to use the required small-handle threshold (value < 0x100000)
before calling crate::object::class_registry::is_class_object_ptr, so native
handles are excluded from the class-object dereference path.
- Around line 51-54: The fallback in get_field_by_name.rs should not use
get_field_by_name_object_tail() as the ownership test, because it can resolve
prototype/class values and miss own properties whose value is undefined. Update
the lookup logic in get_field_by_name to perform an own-property check on obj
first, then only fall back to the class registry when no own property exists;
keep using get_field_by_name_object_tail only for non-own resolution cases where
that behavior is intended.
🪄 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: e2e473fe-3ff8-4dbb-9eb4-3cf8875ffe3d

📥 Commits

Reviewing files that changed from the base of the PR and between 692b04e and ab26853.

📒 Files selected for processing (6)
  • crates/perry-hir/src/lower/lower_expr/arm_ident.rs
  • crates/perry-hir/src/lower/module_decl.rs
  • crates/perry-hir/src/lower/stmt.rs
  • crates/perry-hir/src/lower_decl/body_stmt.rs
  • crates/perry-runtime/src/object/descriptors.rs
  • crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs

Comment thread crates/perry-hir/src/lower/lower_expr/arm_ident.rs Outdated
Comment thread crates/perry-hir/src/lower/stmt.rs Outdated
Comment on lines +120 to +121
for t in chained_targets {
ctx.class_expr_aliases.entry(t).or_insert(class_name.clone());

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Avoid treating arbitrary chained assignment targets as immutable class self-aliases.

Line 120 records every Y in let X = Y = class C { ... }, and lower_ident_expr later rewrites matching Y references inside the class to ClassRef. That breaks valid code where Y is a mutable outer binding that should be observed by methods after reassignment. Constrain this to proven compiler-generated self aliases, or keep generic chained targets as normal captures.

Possible narrow fix
+    let is_probable_tsc_self_alias = |alias: &str| {
+        alias
+            .strip_prefix(&class_name)
+            .and_then(|suffix| suffix.strip_prefix('_'))
+            .is_some_and(|suffix| !suffix.is_empty() && suffix.bytes().all(|b| b.is_ascii_digit()))
+    };
     for t in chained_targets {
-        ctx.class_expr_aliases.entry(t).or_insert(class_name.clone());
+        if is_probable_tsc_self_alias(&t) {
+            ctx.class_expr_aliases.entry(t).or_insert(class_name.clone());
+        }
     }
📝 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
for t in chained_targets {
ctx.class_expr_aliases.entry(t).or_insert(class_name.clone());
let is_probable_tsc_self_alias = |alias: &str| {
alias
.strip_prefix(&class_name)
.and_then(|suffix| suffix.strip_prefix('_'))
.is_some_and(|suffix| !suffix.is_empty() && suffix.bytes().all(|b| b.is_ascii_digit()))
};
for t in chained_targets {
if is_probable_tsc_self_alias(&t) {
ctx.class_expr_aliases.entry(t).or_insert(class_name.clone());
}
}
🤖 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/stmt.rs` around lines 120 - 121, The
chained-assignment handling in stmt lowering is too broad: it currently records
every target in a `let X = Y = class ...` chain as a class self-alias, which
makes `lower_ident_expr` rewrite unrelated `Y` references inside the class to
`ClassRef`. Update the logic in `chained_targets` handling within
`lower_stmt`/`ctx.class_expr_aliases` so only proven compiler-generated self
aliases are registered, and leave ordinary chained targets as normal captures or
mutable bindings. Use the existing `class_expr_aliases` and `lower_ident_expr`
flow to keep the fix localized.

Comment thread crates/perry-runtime/src/object/descriptors.rs Outdated
Comment on lines +146 to +159
if class_id != 0
&& !super::class_registry::class_is_key_deleted(class_id, &method_name)
&& super::class_registry::class_has_own_static_method(
class_id,
&method_name,
)
{
let leaked: &'static [u8] = method_name.as_bytes().to_vec().leak();
let value = super::js_class_method_bind(
obj_value,
leaked.as_ptr(),
leaked.len(),
);
return build_data_descriptor(value, true, false, true);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Mirror static accessor descriptors for fresh class objects.

The fresh-class branch only reports static methods. Object.getOwnPropertyDescriptor(C, "x") for static get x() will still fall through to undefined, while the class-ref path below already returns accessor descriptors.

Suggested fix
                     if class_id != 0
                         && !super::class_registry::class_is_key_deleted(class_id, &method_name)
-                        && super::class_registry::class_has_own_static_method(
-                            class_id,
-                            &method_name,
-                        )
                     {
+                        if let Some((g, s)) =
+                            super::class_registry::class_own_static_accessor_ptrs(
+                                class_id,
+                                &method_name,
+                            )
+                        {
+                            return build_accessor_descriptor(
+                                super::class_registry::class_accessor_function_value(
+                                    g,
+                                    false,
+                                    &method_name,
+                                ),
+                                super::class_registry::class_accessor_function_value(
+                                    s,
+                                    true,
+                                    &method_name,
+                                ),
+                                false,
+                                true,
+                            );
+                        }
+                        if !super::class_registry::class_has_own_static_method(
+                            class_id,
+                            &method_name,
+                        ) {
+                            return f64::from_bits(crate::value::TAG_UNDEFINED);
+                        }
                         let leaked: &'static [u8] = method_name.as_bytes().to_vec().leak();
                         let value = super::js_class_method_bind(
                             obj_value,
📝 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
if class_id != 0
&& !super::class_registry::class_is_key_deleted(class_id, &method_name)
&& super::class_registry::class_has_own_static_method(
class_id,
&method_name,
)
{
let leaked: &'static [u8] = method_name.as_bytes().to_vec().leak();
let value = super::js_class_method_bind(
obj_value,
leaked.as_ptr(),
leaked.len(),
);
return build_data_descriptor(value, true, false, true);
if class_id != 0
&& !super::class_registry::class_is_key_deleted(class_id, &method_name)
{
if let Some((g, s)) =
super::class_registry::class_own_static_accessor_ptrs(
class_id,
&method_name,
)
{
return build_accessor_descriptor(
super::class_registry::class_accessor_function_value(
g,
false,
&method_name,
),
super::class_registry::class_accessor_function_value(
s,
true,
&method_name,
),
false,
true,
);
}
if !super::class_registry::class_has_own_static_method(
class_id,
&method_name,
) {
return f64::from_bits(crate::value::TAG_UNDEFINED);
}
let leaked: &'static [u8] = method_name.as_bytes().to_vec().leak();
let value = super::js_class_method_bind(
obj_value,
leaked.as_ptr(),
leaked.len(),
);
return build_data_descriptor(value, true, false, true);
🤖 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/descriptors.rs` around lines 146 - 159, The
fresh-class handling in the descriptor lookup only checks
class_has_own_static_method, so static accessor properties are missed and
Object.getOwnPropertyDescriptor returns undefined for them. Update the
fresh-class branch in descriptors.rs around the class_id/method_name logic to
also detect own static accessors on the class object, using the same
descriptor-building path as the class-ref branch (or equivalent accessor
descriptor construction) so static get/set members are mirrored for fresh class
objects.

Comment thread crates/perry-runtime/src/object/field_get_set/get_field_by_name.rs
Comment on lines +51 to +54
let own = get_field_by_name_object_tail(obj, key);
if !own.is_undefined() {
return own;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Use an own-property check before falling back to the class registry.

get_field_by_name_object_tail is not own-only; it can walk prototype/class fallbacks. That can make a fresh class constructor return an instance/prototype method before the static lookup runs, and it also fails to preserve an own property whose value is undefined.

Suggested fix
-        let own = get_field_by_name_object_tail(obj, key);
-        if !own.is_undefined() {
-            return own;
+        if super::super::own_key_present(obj, key) {
+            return get_field_by_name_object_tail(obj, key);
         }
📝 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 own = get_field_by_name_object_tail(obj, key);
if !own.is_undefined() {
return own;
}
if super::super::own_key_present(obj, key) {
return get_field_by_name_object_tail(obj, key);
}
🤖 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/get_field_by_name.rs` around
lines 51 - 54, The fallback in get_field_by_name.rs should not use
get_field_by_name_object_tail() as the ownership test, because it can resolve
prototype/class values and miss own properties whose value is undefined. Update
the lookup logic in get_field_by_name to perform an own-property check on obj
first, then only fall back to the class registry when no own property exists;
keep using get_field_by_name_object_tail only for non-own resolution cases where
that behavior is intended.

- get_field_by_name: exclude the whole small-handle band (is_above_handle_band,
  >=0x100000) from the class-object pointer probe — a native handle in
  [0x10000,0x100000) could reach is_class_object_ptr and SIGSEGV (CodeRabbit
  Critical).
- arm_ident: compare the RESOLVED class-expr alias target against current_class
  so a collision-renamed class expression still matches (CodeRabbit).
- descriptors: skip Symbol keys in the fresh-class string-key static-method
  branch so they reach the symbol descriptor path (CodeRabbit).
- cargo fmt.

Deferred (follow-up): chained-assignment self-alias narrowing, static-accessor
descriptors for fresh class objects, own-only field check before the class
registry fallback — subtler changes left for a focused pass.
@proggeramlug
proggeramlug merged commit 7d3aac2 into PerryTS:main Jun 27, 2026
15 checks passed
@proggeramlug

Copy link
Copy Markdown
Contributor Author

Superseded by #5732, which includes these Walls 2–3 fixes (with the Wall 3 fix refined to the cjs_wrap path) plus Walls 4–16 for full native NestJS bring-up. Closing in favor of #5732.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Correction: this PR is already merged — disregard my prior 'closing in favor of #5732' note. #5732 was assembled on the pre-merge base and overlaps these Walls 2–3 (with the Wall 3 fix in cjs_wrap/wrap.rs rather than the module_decl.rs approach merged here). On rebase onto current main, the already-merged Walls 2–3 drop out, leaving #5732 = Walls 4–16.

proggeramlug added a commit that referenced this pull request Jun 28, 2026
…, collections, HTTP, DI, iterare, regex/GC) (#5739)

* feat(nestjs): native NestJS support — Walls 4–18 (express, generators, collections, HTTP, DI, iterare, regex/GC) + review hardening

Rebased onto current main. Walls 2–3 (chained-static-class decorators +
reflect-metadata) are already merged via #5721, so those hunks are dropped here
(main keeps its module_decl.rs reflect-metadata variant + arm_ident/stmt/
descriptors/get_field_by_name Wall-2/3 portions). This PR is the net Walls 4–18
contribution plus CodeRabbit hardening and two POST-body fixes.

Walls (net):
- Express node-compat: Object.setPrototypeOf(handle, proto) member reads on
  native res/req handles (Wall 10, handle_proto_inherited_field + handle
  dispatch fallback); express/router/serve glue in ext-http-server
  (dispatch_ext.rs, handle_dispatch, request, lib, http2_server).
- Generators: nested function* that capture outer locals or are
  forward-referenced by an earlier sibling now lower as capture-aware generator
  closures instead of hoisted top-level Functions (gen_capture_scan.rs,
  body_stmt FnDecl arm, block.rs / expr_function.rs forward-ref scoping,
  nested_fn_decl, context/lowering_context field).
- Collections: class X extends Map|Set — backing-store .size read +
  method/iterator dispatch (map_set_subclass.rs, get_field_by_name .size
  branch, array iter_object/iterator, collection_methods, symbol iterator).
- HTTP / DI / iterare / regex / GC / symbols / weakref / numbers / descriptors-
  adjacent runtime + codegen fixes (call_spread, compare, this_super_call,
  console_promise, streams_events, strings_part2, native_module, descriptors
  define_properties, has_property, ic_miss, dynamic_props, regex, symbol/get,
  builtins/numbers, gc/layout, array_only_methods, native_module HIR).

CodeRabbit hardening (12 applied):
- Map/Set .size probe guarded by is_above_handle_band (consistent with the
  class-object probe) so a native handle id in [0x10000,0x100000) never reaches
  own_key_present's ObjectHeader deref.
- plus the fixes already folded into the reference change-set (resolved-alias
  comparison, symbol-key skips, own-key precedence, etc.).
Deferred (2): claimed/unclaimed FFI contract on setPrototypeOf'd member-name
collision with a native member returning undefined — only fires for a name
collision NestJS/Express never produce.

POST-body fixes:
- B1: req.readable on the native request handle.
- B2: isNaN small-string-optimization path.

Verified on this rebased base (PERRY_NO_AUTO_OPTIMIZE=1, worktree staticlibs):
- starter GET / → Hello World!
- realapp GET / → Realapp root OK; GET /items, /items/2, /items/99, POST /items
  {"name":"Gamma"} → all correct.
- cargo test -p perry-hir -p perry-runtime --release -- --test-threads=1:
  1089 passed, 0 failed.

https://claude.ai/code/session_017S2d3tRok3oMbi6AwfJyUU

* fix(nestjs): address CodeRabbit review — collection-subclass receiver/enumeration, inherited size, setPrototypeOf `in`, early-return captures, RegExp GC slots

CodeRabbit findings on #5739:

2 (expr_function.rs): refresh class captures before EVERY reachable return in a
   function-expression body, not just a trailing one, so an early `return <class>`
   after captured locals are assigned no longer returns a stale snapshot. Added a
   recursive walk that descends statement children (if/loops/try/switch/labeled)
   but not into nested closures.

3 (gc/layout.rs + regex.rs): scan only the RegExp GC-visible slots — pattern_ptr/
   flags_ptr (2-slot payload range) + last_index (prefix slot) — via a new
   `regex_gc_slot_ptrs` helper using addr_of offsets; off-heap regex_ptr/fancy_ptr,
   bools, magic and padding are no longer inspected during evacuation.

4 (get_field_by_name.rs): respect an inherited `size` override on a Map/Set
   subclass (`class M extends Map { get size(){return 42} }`) — check the class
   vtable before returning the backing size.

5 (has_property.rs): after `Object.setPrototypeOf(instance, proto)` records a
   replacement prototype, skip the class-vtable fallback in `'key' in instance`
   so replaced/deleted members are not resurrected.

6 (enumeration/descriptors/has_own/common_methods): filter the hidden
   `__perry_*` backing key from Object.keys / for…in / getOwnPropertyNames /
   JSON.stringify / propertyIsEnumerable on Map/Set subclass instances.

7 (collection_methods.rs + map.rs + set.rs): preserve the subclass instance as
   the observable receiver — set/add return `this` for chaining, forEach passes
   the instance as the 3rd callback arg (new `js_{map,set}_foreach_with_collection`),
   and non-collection methods fall through to normal dispatch instead of being
   swallowed by the backing redirect.

1 (console_promise.rs): restrict the collection-subclass method suppression to
   Map/Set; WeakMap/WeakSet subclasses (no backing) fall through to normal
   dispatch. Full WeakMap/WeakSet backing is deferred (heavy, NestJS unaffected).

Verified: targeted repros match Node; both NestJS apps serve (starter Hello
World!; realapp all routes incl. POST /items); perry-hir + perry-runtime tests
0 failures (--test-threads=1).

https://claude.ai/code/session_017S2d3tRok3oMbi6AwfJyUU

* style: cargo fmt --all (line-wrapping only; fixes lint check)

Claude-Session: https://claude.ai/code/session_017S2d3tRok3oMbi6AwfJyUU

* fix(nestjs): CodeRabbit round-3 — exact internal-key match (un-hide user __perry_* fields), forEach no-callback validation, hasOwn/hasOwnProperty backing-key exclusion, Set forEach live size

Round-3 follow-ups on the collection-subclass work in #5739:

1 (REGRESSION) enumeration.rs / map_set_subclass.rs: the hidden runtime-key
  filter matched the broad `__perry_*` PREFIX, so a legitimate user property
  like `this.__perry_user = 1` vanished from Object.keys / getOwnPropertyNames /
  JSON.stringify / propertyIsEnumerable. Replaced the prefix test with an EXACT
  match against the one genuinely-internal key (`__perry_collection_backing__`,
  now `pub(crate) BACKING_KEY`). `is_internal_runtime_key{,_bytes}` are an
  allowlist of exactly that key.

2 collection_methods.rs: `forEach` was guarded by `&& !args.is_empty()`, so
  `new SubMap().forEach()` / `new SubSet().forEach()` silently returned
  undefined instead of throwing. Drop the guard; pass
  `args.first().copied().unwrap_or(undefined)` so the impl's
  `js_validate_array_callback` throws `TypeError` like Node.

3 has_own.rs (js_object_has_own) + common_methods.rs (hasOwnProperty path):
  the backing-key exclusion previously only covered enumeration /
  propertyIsEnumerable, so `Object.hasOwn(inst,"__perry_collection_backing__")`
  and `inst.hasOwnProperty(...)` still returned true. Added the same exact-key
  exclusion to both own-property paths.

4 set.rs (js_set_foreach_impl): snapshotted `size` once, so entries added by the
  callback mid-iteration were never visited. Re-read `(*set).size` each step in a
  `loop` (mirrors js_map_foreach_impl) for Node's live-visit semantics.

Verified on a feature-unified staticlib rebuild: targeted repros all match Node
(user __perry_* visible; SubMap/SubSet forEach() throw TypeError; backing key
hidden from keys/hasOwn/hasOwnProperty/propertyIsEnumerable; Set forEach visits
mid-iteration adds). Both NestJS apps serve (starter "Hello World!"; realapp GET
/, GET /items, POST /items, GET /items/:id). perry-hir + perry-runtime tests 0
failures (--test-threads=1); cargo fmt --check clean.

https://claude.ai/code/session_017S2d3tRok3oMbi6AwfJyUU

---------

Co-authored-by: Ralph Küpper <ralph2@skelpo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant