fix: static this is the constructor; class name/toString/inspect report source identity (from #9465) - #9472
Merged
Merged
Conversation
added 3 commits
September 1, 2026 23:17
…ce (#9404) Inside a `static` body, `this` is the class CONSTRUCTOR — an INT32 class ref — not a heap instance. Two independent defects both made instance members resolve on it, and each is reachable on its own. 1. `receiver_class_name(Expr::This)` and `static_type_of(Expr::This)` answered `Named(class_stack.last())` in a static body exactly as in an instance body, so every consumer was entitled to prove instance facts about the constructor object: field slots, shape ids, direct method dispatch. Both now return `None` under `FnCtx::in_static_member`, and `CodegenTypeFacts::this_type` carries the same gate so the generic HIR inference cannot re-derive the instance type for an expression that merely CONTAINS `this`. This closes the alias residual #9386 left open (`static viaLocal(){ const t = this; … }`: `undefined|` -> `object|9`). "The constructor object of C" would not be a better answer: static members are inherited, so a static body's `this` is whatever subclass the call came through. `None` is the only sound answer. 2. Declared instance methods are mirrored onto the reflective `C.prototype` object as own data fields, and the CONSTRUCTOR-side read in `js_object_get_field_by_name` walked that object via `resolve_proto_chain_field`. So `class P { m(){} }` gave `typeof P.m === "function"` and `P.m === P.prototype.m` with no `this` involved at all. `js_object_has_property` already had the gate (`"m" in P` was correctly false); the receiver-less `resolve_proto_chain_field` — whose one caller is that static-side read — now refuses a name that `class_instance_has_member` reports as a prototype method/getter/setter of the chain. Keyed on that predicate, NOT on "skip the decl-prototype entirely". The blanket form was tried and measured: it also removes `C.constructor`, which the decl-prototype carries as a data field, and that answer is load-bearing because perry hands a PROPERTY DECORATOR the class itself where the spec hands `Class.prototype` — so NestJS-style `Reflect.defineMetadata(k, v, target.constructor)` relies on `C.constructor === C`. Node says `Function`; perry has two divergences that cancel, and the blanket form took `test_decorators_nest_common_canary` and `test_decorators_legacy_property_metadata` from pass to parity_fail. The decorator-target defect is the one worth fixing and is not this issue. The `class_prototype_object` step is never skipped: for a subclass of a class-EXPRESSION value it holds the parent CLASS OBJECT (#1788/#6552), genuinely on the constructor's static chain. Known residual, deliberately not asserted in the fixture: the CALL form `this.m()` in a static body still resolves the instance vtable method. The value read is fixed, and both `P.m()` and read-then-call throw correctly; the residual is keyed on the runtime class id (an inherited `static go(){ return this.m() }` called on a subclass runs the SUBCLASS's override), so it is the runtime call tower, not a codegen direct call. Repro in the fixture's comment. Refs #9404, #9369, #9386. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp
) Three leaks of the compiler's internal class identity into user-visible strings, plus the missing half of `Function.prototype.toString`. `.name` reported the disambiguation key. Two `class Made {}` in sibling scopes are distinct classes, so the second registers under a uniquified key (`Made$0`) to keep the name-keyed dedup from aliasing the bodies; that key reached `js_register_class_name`. And a class expression constructed IN PLACE lost its name entirely: `new (class Q {})()` gave `__anon_class_6`, `new (class extends Error {})("m")` gave `__anon_class_8` where node gives `""`. Both are fixed by populating the existing `Module::class_display_names` override that `codegen/string_pool.rs` already prefers over the registration key — the sibling `lower_expr/arm_class.rs` had recorded exactly that since #5592. `console.log(C)` / `util.inspect(C)` printed the raw class id (`6`): a class ref shares the INT32 encoding with a tagged small integer, and the console formatter's `is_int32()` arm printed the payload. It now renders node's `[class Klass]` / `[class Sub extends Named]` / `[class (anonymous)]`, gated on the class-id registry — the same probe `js_jsvalue_to_string` already used, so a plain small integer whose value collides with a live class id still prints as a number (asserted). `String(C)` / `C.toString()` returned `function C() { [native code] }`. Perry already retained function source (`closure_source_text`, #4101); the same span-slice-at-lowering mechanism, keyed by ClassId, was simply never applied to classes — the one callable kind that is not a ClosureHeader and so cannot recover source from the closure registry. `Module::class_source_text` is sliced against `ast::Class::span`, emitted as `js_register_class_source`, and read by all three class-ref toString sites. A class with no registered source keeps the `[native code]` form, which Test262's `assertToStringOrNativeFunction` accepts. Monomorphized specializations inherit the origin's source, as #7632 makes them inherit its name. Not addressed: `String(C.prototype.m)` for a class METHOD still returns `function () { [native code] }`. Class methods compile to `perry_method_*` symbols rather than closures with a registered source, so that needs the method-side equivalent of the closure source registry. Object-literal methods already work and are kept in the fixture as the control. Refs #9413, #5592, #4101, #7632. Claude-Session: https://claude.ai/code/session_014knX724SYDogwzsXybCGxp
# Conflicts: # crates/perry-runtime/src/builtins/formatting.rs
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (62)
📝 WalkthroughWalkthroughThe change corrects static ChangesClass behavior and identity
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant StaticMethod
participant TypeAnalysis
participant RuntimeLookup
StaticMethod->>TypeAnalysis: resolve static this
TypeAnalysis-->>StaticMethod: avoid instance type
StaticMethod->>RuntimeLookup: read constructor property
RuntimeLookup-->>StaticMethod: resolve static member only
sequenceDiagram
participant ClassLowering
participant HIRModule
participant Codegen
participant Runtime
ClassLowering->>HIRModule: capture class name and source
HIRModule->>Codegen: emit class metadata
Codegen->>Runtime: register class source
Runtime-->>Codegen: format class text and inspect label
Suggested reviewers: ✨ 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 2, 2026
proggeramlug
added a commit
that referenced
this pull request
Sep 2, 2026
…C.constructor === Function (#9467) (#9496) * fix(decorators): instance-member decorators receive Class.prototype; C.constructor === Function (#9467) Two divergences that cancelled, found while fixing #9404 (#9465/#9472): 1. A legacy decorator on an INSTANCE member (property, method, method parameter) received the class itself as `target`; tsc's `__decorate([...], C.prototype, key, desc)` hands it `Class.prototype`. Static members correctly keep the constructor. 2. `C.constructor === C`; node says `Function`. The decl-prototype carries `constructor` as an ordinary data field and the constructor-side chain walk (`resolve_proto_chain_field`) returned it before the class-ref arm's existing `constructor -> Function` tail fallback was reached. NestJS-style `Reflect.defineMetadata(k, v, target.constructor)` only landed on `C` because both were wrong. Both halves fixed together: - perry-hir `lower/decorators.rs`: `member_decorator_target` hands instance members `PropertyGet(ClassRef, "prototype")` (the reflective decl-proto object), statics `ClassRef`; `design:type` / `design:paramtypes` ride the same target. The metadata store's prototype->class fold keeps the historical `getMetadata(..., Class, prop)` reads resolving. - perry-runtime `prototype_objects.rs`: the constructor-side walk also skips the `constructor` key, so `C.constructor` falls through to `Function`; `C.prototype.constructor` and instance reads are untouched. Fixture `test_decorators_target_prototype_9467` (expected output from tsc --experimentalDecorators --emitDecoratorMetadata + reflect-metadata under node) pins: target identity per member kind, `C.constructor === Function`, `p.constructor === C`, and `Reflect.getMetadata` round-trips through both `target` and `target.constructor`, including inheritance. Claude-Session: https://claude.ai/code/session_01MmDfS97fv8TRgyDnj6bgsL * changelog: #9496 decorator target / C.constructor fragment * 9467: `"constructor" in C` agrees with the read; fixture pins user-class metadata types only --------- Co-authored-by: Ralph Küpper <ralph@skelpo.com>
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.
Lands #9465, which cherry-picks with conflicts but merges cleanly; this carries a merge of current
main. Author's commits preserved.One conflict, resolved toward main's canonical form. Both this PR (#9413) and #9461 (#9415) fixed the same int32-tag/class-ref confusion in
formatting.rs's inspect ladder. Main's version routes throughvalue_repr::int32_or_class_repr— the declared single decision point for that tag, with the identical registry-probe behaviour this PR implemented inline — plus an array-hole arm this PR predates. I verified the helper's body subsumes the inline version before choosing it.Verified behaviourally:
static who() { return this.name }answers the class name on a plain class AND on one with a computed member — the #9404 shape that took downcc --help— andname/toString()/util.inspectall report source identity, byte-identical to node.Validation:
perry-runtime+perry-codegengreen (39 suites) underRUST_TEST_THREADS=1; release build clean, no warnings; file-size, raw-handle, addr-class, census, root-holder, thread-local and fmt gates all pass.Summary by CodeRabbit
New Features
String(Class),Class.toString(), and related output now show the class’s source text when available.[class ...]formatting.Bug Fixes
thisbehavior so instance members are not exposed as static members.Tests