fix: #809 — object literal spread+methods+computed-keys; Object.create wires prototype#815
Open
proggeramlug wants to merge 2 commits into
Open
fix: #809 — object literal spread+methods+computed-keys; Object.create wires prototype#815proggeramlug wants to merge 2 commits into
proggeramlug wants to merge 2 commits into
Conversation
…e wires prototype
Four independent bugs in the chain, all needed for the standalone repro to
match node (`Proto = { [TypeId]:TypeId, [Symbol.iterator]() {}, pipe() {},
...Inspectable.BaseProto, toJSON() {} }; Object.create(Proto).toJSON()`):
1. expr_object.rs `has_spread` path lowered to `Expr::ObjectSpread` whose
`parts` only carries static-string KeyValue + spreads, silently dropping
`Prop::Method` and computed `KeyValue` keys. Rewritten as a source-ordered
IIFE so static props, computed keys, this-binding methods, and spreads
interleave correctly (later same-key wins). Shared key/method-resolution
helpers extracted (`resolve_keyvalue_key`, `lower_method_prop`) and used
by both spread and non-spread paths — behavior-identical for non-spread.
2. `static_receiver_class` now returns `Some("Object")` for object-literal /
`Object.create(...)` receivers and locals tracked in a new
`plain_object_locals` set, so the ambiguous-Date-method gate skips
`DateToJSON`/`DateToString`/`DateValueOf`/... for plain objects.
Pre-fix `inst.toJSON()` where `inst = Object.create(Proto)` lowered to
`DateToJSON(inst)`, reading the pointer bits as a timestamp → epoch.
3. `js_object_create` ignored its prototype argument and returned a bare
empty object. Now allocates a synthetic class_id, maps it to `proto` in
`CLASS_PROTOTYPE_OBJECTS` (reusing the #711 machinery), and stamps the
object. `Object.create(null)` / non-object / builtin-backed sources
preserve the original prototype-less behavior.
4. Prototype-walk gaps exposed by (3):
- Codegen PIC fast path spuriously "hit" when `obj->keys_array == null`
(zero-init cache matched), bypassing the miss handler. Added a
non-null keys_array requirement to the hit predicate.
- `js_object_get_field_by_name`'s `keys_array == null` arm now consults
the prototype chain before returning undefined; the existing inline
#711 walk also routes through the new shared `resolve_proto_chain_field`
helper (DRY).
- `js_native_call_method`'s prototype-object walk was gated inside
`if let Some(ref reg) = *CLASS_VTABLE_REGISTRY` — skipped entirely for
programs with no user classes. Added an independent
`resolve_proto_chain_field` resolution after the registry walk, with
`IMPLICIT_THIS` set so methods see the right receiver.
Plus a new runtime helper `js_object_set_method_by_name` (string-key analog
of `js_object_set_symbol_method`) used by the IIFE lowering for static
this-binding methods placed after a spread.
Validation: repro byte-identical to `node --experimental-strip-types`; gap
suite 34/36 (2 = pre-existing console.dir/group + lookbehind regex gaps);
`cargo test` for perry-hir / perry-codegen / perry-runtime all green;
12-file curated smoke over spread/object/prototype test-files matches node;
real `new Date(0).toJSON()`/`toISOString()` still work, `Object.create(null)`
still works, spread override ordering preserved.
Known remaining pre-existing gaps not in #809's DoD: symbol-keyed property
inheritance through an `Object.create` prototype chain still returns
undefined (symbol side-table is keyed by receiver pointer); and
`Object.getPrototypeOf(Object.create(p))` returns the class-ref shim, not
`p`. Tracked separately.
Closes #809. Refs #321, #711.
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.
Closes #809. Refs #321, #711.
The Effect end-to-end repro (#321) advanced past
ParseResult.tsand threwTypeError: value is not a functioninHashRing.ts__init. The 20-line standalone repro from the issue (Proto = { [TypeId]:TypeId, [Symbol.iterator]() {}, pipe() {}, ...Inspectable.BaseProto, toJSON() {} }+Object.create(Proto).toJSON()) printedkeys: 2(node: 4 — the issue's "5" predates checking against node, which excludesSymbol.iteratorfromObject.keys),inst.toJSON()→\"1970-01-01T00:00:00.000Z\",inst[TypeId]→undefined.Four independent bugs in the chain, all needed to pass
has_spreadlowering dropped entries.crates/perry-hir/src/lower/expr_object.rs's spread path lowered toExpr::ObjectSpread { parts }whosepartsonly carries static-string KeyValue + spreads;Prop::Methodfell into_ => {}and computedKeyValuekeys into_ => continue— so[Symbol.iterator](),pipe(), the[TypeId]computed string key, and the trailingtoJSON()were all silently discarded.DateToJSONlowering.inst.toJSON()whereinst's static class is unknown rewrote toDateToJSON(inst); the runtime read the pointer's NaN-box bits as a timestamp → epoch.js_object_createignored its argument. Allocated a bare empty object, droppingprotoentirely.Object.create(P).xsaw nothing.obj.propspuriously "hit" whenobj->keys_array == null(anObject.createresult) because the zero-init per-site cache matched, returningslot[0]without invoking the miss handler.js_object_get_field_by_nameearly-returned undefined on nullkeys_arraybefore itsclass_idprototype-walk.js_native_call_method's prototype-object walk was gated insideif let Some(ref reg) = *CLASS_VTABLE_REGISTRY— skipped entirely for programs with no user classes.Fix
has_spreadpath as a source-ordered IIFE((__o) => { …ordered ops…; return __o })({})so static props, computed keys, this-binding methods, and spreads interleave correctly (a later prop/spread overrides an earlier same key — the non-spread fast path can't be used because it applies all static props before any post-init, which would let a trailing...srcclobber the literal's owntoJSON()). Spreads emitjs_object_assign_one; static this-methods a newjs_object_set_method_by_nameruntime helper (string-key analog ofjs_object_set_symbol_method); computed this-methods reusejs_object_set_symbol_method; everything elseIndexSet. Extractedresolve_keyvalue_key/lower_method_propshared helpers used by both spread and non-spread paths (non-spread behavior identical).static_receiver_classnow returnsSome(\"Object\")for object-literal /Object.create(...)receivers and locals in a newLoweringContext.plain_object_localsset (populated inlower_var_decl_with_destructuring); the ambiguous-Date-method gate treats it likeSome(\"URL\")and skips the Date arms.js_object_createnow allocates a synthetic class_id, maps it toprotoinCLASS_PROTOTYPE_OBJECTS(reusing the perry-runtime: Effect framework throwsTypeError: pipe is not a functionduringSchema.tsmodule init (Effect end-to-end blocker, post-#685) #711 machinery), and stamps the object.Object.create(null)/ non-object / builtin-backed sources fall back to the original prototype-less object.keys_array != 0so keyless receivers route to the slow path.js_object_get_field_by_name's null-keys_arrayarm consults the prototype chain via a new sharedresolve_proto_chain_fieldhelper (which also de-duplicates the existing inline perry-runtime: Effect framework throwsTypeError: pipe is not a functionduringSchema.tsmodule init (Effect end-to-end blocker, post-#685) #711 walk).js_native_call_methodruns an independentresolve_proto_chain_fieldresolution after the registry walk, withIMPLICIT_THISset so methods see the right receiver.Validation
node --experimental-strip-types:keys: 4,inst.toJSON: {\"_id\":\"HashRing\",\"x\":1},inst[TypeId]: ~test/HashRing.cargo test --releasefor perry-hir / perry-codegen / perry-runtime all green (0 failed).test_issue_711_function_prototype,test_spread,test_edge_rest_spread_defaults,test_get_prototype_of_instance) byte-identical to node.new Date(0).toJSON()/toISOString()still work, spread override ordering preserved ({a:1,...{a:2}}→{a:2},{...{a:2},a:1}→{a:1}),Object.create(null), empty-object missing-prop,Object.create(proto).method()withthis-binding all match node.Known remaining pre-existing gaps (not in #809's DoD, not regressions)
Object.createprototype chain (o[sym]wheresymis on the proto) still returns undefined — the symbol side-table is keyed by receiver pointer and the prototype walk only resolves string keys. Not exercised by perry-codegen: object literal with computed-key props + computed-key methods + cross-module spread drops keys & mis-resolves methods — EffectHashRing.tsblocker (post-#740) #809 (Object.keysexcludes symbols;inst[TypeId]is a string key) andObject.createreturned nothing before this commit, so it never worked there.Object.getPrototypeOf(Object.create(p)) === pstill returns the class-ref shim, notp.Both tracked separately.