fix(runtime): dispatch inherited Array statics on a subclass constructor (#7541) - #7605
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe runtime now dispatches inherited ChangesArray subclass inherited statics
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: 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 |
6c9bfd1 to
6766285
Compare
3bbb402 to
0fc7d7e
Compare
6766285 to
40f1d47
Compare
0fc7d7e to
008e65d
Compare
40f1d47 to
780f5aa
Compare
Fixes #7541.
Stacked on #7603 (
fix/7574-array-subclass-raw-paths), and not byconvenience — see Why this is stacked below. Review the top commit only.
The bug is not in spread
The issue guessed that
array_from_spread_value's subclass arm(
is_array_subclass_instance/array_subclass_has_iterator_override) wasanswering wrong. It is not — a directly constructed instance spreads
correctly today, and did before this PR:
MyArr.fromresolves to nothing.Array.from/Array.of/Array.isArrayare folded in the HIR on the literal identifierArray(
perry-hir/src/lower/expr_call/array_only_methods.rs:293), so a subclassreceiver matches no fold and the call falls through to
js_class_static_method_call, whose documented miss-fallback returns thereceiver.
MyArr.from([1,2,3])therefore evaluates to the class ref — whichis genuinely not iterable, so the spread throws exactly where the issue saw it.
This is the CLAUDE.md "native base-class subclassing… keying any of that on a
literal
extendsname loses it" weak area, on the STATIC side.Fix
js_class_static_method_callalready has arms for inherited builtin statics ona
class X extends Promise(X.all/X.resolve) and on aclass X extends Buffer(nm_static_buffer_proto_chain). Add the Array onebeside them, gated on
is_array_subclass_class_id(class_id)— the same boundedclass-chain walk the instance-side dispatch already uses.
Both spec statics are already implemented constructor-aware:
array::array_from_full(c, items, mapfn, thisArg)andarray::array_of_full(c, vals)runConstruct(C, …)wheneverIsConstructor(this), andis_constructor_valuerecognizes an INT32 class ref.So passing the subclass receiver through as
thisbuilds a real subclassinstance —
MyArr.from(x)behaves asArray.from.call(MyArr, x)does innode, rather than degrading to a plain
Array.isArrayneeds no receiver androutes to
js_array_is_array.Why this is stacked on #7603 (not merely convenient)
array_from_full's element install isCreateDataPropertyOrThrow, whoseimplementation branches on
jsv_is_array(result)— andjs_array_is_arrayanswers true for an Array-subclass instance. So it calls
js_array_set_f64_extendon what is physically anObjectHeader. Onmainthat is precisely the #7574 corruption this PR would newly start triggering:
without #7603 the fix half-works and then writes into the object header —
With #7603's funnel underneath, the same code is correct. Shipping this against
mainalone would trade one wrong answer for a memory-safety hazard, so itmust land after #7603.
Validation (local; CI has a deep backlog, so local is what this rests on)
test-files/test_gap_7541_array_subclass_inherited_statics.ts—byte-identical to
node --experimental-strip-types(v26.5.1), exit 0, 19lines. Covers the issue's exact repro plus
fromwith a mapFn, from aSet,from an array-like,
of,isArray, an indirect subclass(
class Indirect extends MyArr), and the whole iteration surface on astatic-produced instance (
for…of, spread,Array.from, destructuring,map, indexing,length). Controls: the baseArray.from/Array.ofintrinsics and an ordinary user-class inherited static are unchanged.
crates/perry-runtime/reverted in full to this PR'sbase (i.e. fix(runtime): an Array subclass in a base-typed binding was read as a raw header (#7574) #7603 present, this change absent) and the test file untouched,
the same file exits 1 with
TypeError: value is not iterable— thereported symptom, verbatim. Restored, it exits 0, byte-identical.
test_gap_7574_array_subclass_declared_base_type.tsstill byte-identical onthis branch.
cargo test -p perry-runtime: 1853 passed, 0 failed.python3 scripts/raw_handle_debt.py: 998 (baseline 998).python3 scripts/addr_class_inventory.py,python3 scripts/class_id_collisions.py,./scripts/check_file_size.sh,cargo fmt --all -- --check: all clean.Scope — deliberately NOT fixed here
typeof MyArr.fromstill reportsundefined;only the CALL form is dispatched (the GET path is a different resolver, and
the same is true today for the
Promise/Buffersubclass statics thatalready work as calls). Called out inline in the test file.
sub instanceof MyArris false for a static-produced instance — thepre-existing class-registry parent-edge gap, the Array sibling of
instanceofa Map/Set SUBCLASS is false (m instanceof MyMap); only the native base edge survives #7575.ArraySpeciesCreateonsub.map(f)still yields a plainArray(node:MyArr). Pre-existing, identical on the unannotated path, noted in fix(runtime): an Array subclass in a base-typed binding was read as a raw header (#7574) #7603.No version bump (maintainer bumps at merge).
Summary by CodeRabbit
Bug Fixes
Array.from,Array.of, andArray.isArraybehavior forArraysubclasses.Array.fromandArray.ofnow correctly create instances of the subclass, including indirect subclasses.Tests
Chores
0.5.1344.