fix(hir+runtime): #838 — JS-classic prototype-method dispatch (Class.prototype.m = fn) - #877
Merged
Merged
Conversation
…prototype.m = fn) Surfaced by the #805 npm sweep on dayjs / chalk: `(new Dayjs()).format` returned undefined vs. Node's function. dayjs (and the rest of the pre-ES6 npm tail) attaches instance methods via `Dayjs.prototype.format = function(){…}` — and the minified bundle uses the aliased shape `var m = M.prototype; m.parse = …`. Both lowered through the generic PropertySet path onto a prototype-proxy that nothing in dispatch consulted, so the assignment was a silent no-op. HIR-lowering now recognises both shapes (with TS-syntactic-noise unwrapping for `(X.prototype as any).m = fn`) and routes them to a new `Expr::RegisterPrototypeMethod` node that codegen emits as a `js_register_prototype_method(class_id, name, value)` call. Runtime stores those into `CLASS_PROTOTYPE_METHODS` keyed by class_id; the dispatch hot paths (`js_object_get_field_by_name`, `js_native_call_method`) consult `lookup_prototype_method` after the vtable / proto-object walks miss, binding `this` to the receiver via the existing `IMPLICIT_THIS` thread-local. Parent-class chain is walked (depth-capped at 32) so base-class methods reach subclass instances. Validation: new test-files/test_issue_838_prototype_methods.ts covers direct, aliased, and multi-method `this`-aware composition shapes — byte-for-byte parity with `node --experimental-strip-types`. Chalk's callable-with-properties (Proxy-wrapped) is a separate shape and stays gated on Proxy support in the compiled-package codegen. Refs #793 #805 #115. Closes #838.
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
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.
Summary
Closes #838. Surfaced by the #805 npm sweep on dayjs and chalk:
(new Dayjs()).formatcame backundefinedeven though the compiledpackage linked. dayjs (and the pre-ES6 npm tail more broadly) attaches
instance methods via
Dayjs.prototype.format = function(){…}— and theminified dayjs bundle uses the aliased shape
var m = M.prototype; m.parse = ….Both lowered through perry's generic
PropertySetpath onto aprototype-proxy that nothing in dispatch consulted, so the assignment
was a silent no-op from the user's perspective.
Fix
HIR-lowering (
crates/perry-hir/src/lower/expr_assign.rs) recognisestwo assignment shapes — with TS-syntactic-noise unwrapping
(
TsAs/TsNonNull/TsSatisfies/TsTypeAssertion/TsConstAssertion/Paren)so
(Dayjs.prototype as any).format = …is handled identically to thebare form:
<ClassName>.prototype.<method> = <fn>—member.objis a
MemberExprof the form<Ident>.prototypewhere the identresolves to a known class.
<local>.<method> = <fn>where the local was previouslyinitialised from
<ClassName>.prototype. A newctx.prototype_aliases: HashMap<LocalId, String>is populated bylower_var_declwhen it seeslet p = ClassName.prototype;(orvar/const).Both routes emit a new
Expr::RegisterPrototypeMethod { class_name, method_name, value }HIR node. Codegen lowers it tojs_register_prototype_method(class_id, name_ptr, name_len, value).Runtime (
crates/perry-runtime/src/object.rs) adds:CLASS_PROTOTYPE_METHODS: RwLock<Option<HashMap<u32, HashMap<String, u64>>>>side-table keyed by class_id.
lookup_prototype_method(class_id, name)consulted byboth dispatch hot paths after the regular vtable / proto-object
walks miss:
js_object_get_field_by_name→ returns the closure value forinst.methodreads.js_native_call_method→ bindsthis = receivervia the existingIMPLICIT_THISthread-local and routes throughjs_native_call_valueforinst.method(args).on a base class are reachable via subclass instances (mirrors
Object.getPrototypeOfsemantics).Generic
obj.prop = fnon non-prototype objects is unchanged — onlyassignments the HIR identifies as prototype-method attachments emit
the new FFI, and only those participate in the new dispatch arm.
Why this layer
A pure-runtime fix (turn the existing PropertySet-on-prototype-proxy
path into a real per-class side-table) would also work but would
require runtime introspection of the prototype-proxy shape on every
property assignment, plus a class-id back-pointer threaded through.
Doing recognition at HIR keeps the runtime hot paths free of
speculative lookups.
Test plan
cargo build --release -p perry-runtime -p perry-stdlib -p perrycleantest-files/test_issue_838_prototype_methods.tscovers direct, aliased, and multi-method
this-aware composition shapesnode --experimental-strip-typeson the new testlint,cargo-test,parity,compile-smoke,api-docs-drift,security-auditgreenOut of scope (follow-up)
Chalk's exported callable-with-properties (
chalkis callable AND has.green/.red/ …) is a separate shape — a Proxy-wrapped callable,not a
Class.prototype.X = …pattern — and stays gated on Proxy /callable-object support in the compiled-package codegen.
Refs #793 (Node.js + TypeScript compatibility roadmap), #805 (npm sweep harness), #115 (port-npm-to-perry skill).