fix(hir): an Array-named method on an unproven receiver keeps its own method (#11187) - #11199
Conversation
… method (#11187) A call result, awaited value, conditional or property receiver was classified by method name alone in try_array_only_methods: sort, flat and toSpliced folded to the dense Array ops unconditionally, and the callback iterators folded for every receiver shape except a few recognised ones. mongodb's collection.find({}).sort({ a: 1 }) therefore became Array.prototype.sort and threw on the object argument. Those arms now require a proven Array receiver (inferred Array/Tuple type or an Array-producer root); bare typed identifiers keep their existing classification. Declining falls through to shape-aware dynamic dispatch, which still reaches the Array helper for a real array.
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughArray-only method lowering now checks whether a receiver is proven to be an Array before applying selected array-method folds. Unproven, non-class receivers fall through to generic dynamic dispatch. Added unit and TypeScript tests cover unproven receivers and proven-array controls. ChangesArray method lowering
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Bug fix · Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to The identified class-receiver defect predates this change, and no remaining issue attributable to this PR is established. Merge after normal checks. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
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 |
|
Ready to merge once CI is clean. Fixes #11187, a mongodb blocker. perry-hir only: 13 Array-named methods on non-local receivers now require a proven array before taking the Array fast path, so |
1 participant
Fixes #11187
Root cause
try_array_only_methods(crates/perry-hir/src/lower/expr_call/array_only_methods.rs) is the fold pass for Array-named methods on any receiver that is not a bare local. It decided whether a receiver was an array from the method name, not from the receiver itself:map,find,reduce, ...) needed a proven Array root (call_roots_at_array).sort,flatandtoSplicedfell torecv_is_class = falseand folded unconditionally.await x,cond ? a : b, and so on), the_ => falsecatch-all let every callback name fold too.So mongodb's
collection.find({}).sort({ a: 1 })becameExpr::ArraySort.FindCursor.prototype.sortnever ran, and the runtime threw "The comparison function must be either a function or undefined". Binding the cursor to a local first worked because bare identifiers go through the type-driven gate inlocal_array_methods.rs, which already sends unknown receivers to dynamic dispatch (#5139, #10476).Fix
Added
receiver_is_proven_array, which the arms that have no proof of their own now require:map,filter,forEach,find,findIndex,findLast,findLastIndex,some,every,reduce,sort,flat,toSpliced, and thethisArgArrayLikeMethodroute that follows.None/any/unknown) local counts as unproven.T[], tuple,Array<T>,ReadonlyArray<T>) or bychain_roots_at_array.The remaining arms already check their own proof:
slice,join,indexOfandincludes(HIR shape);pushandwith(type);entries,keys,values,reduceRight,toReversedandtoSorted(recv_is_proven_array).recv_is_classis not changed, so no receiver that currently goes to dynamic dispatch starts folding. The only effect is that unproven receivers stop folding. Declining is always sound: the generic tail'sjs_native_call_methodselects by the runtime shape, so a real Array still reaches the dense helper. The gap test coversany-typed call results that are real arrays.Checked the other array-name gates:
local_array_methods.rsalready requires type proof for bare locals (compilePackages: react-dom/server renderToStaticMarkup returns empty string #5139/User methods named like Date/Number/Array built-ins (getTime,toFixed,toISOString,toSorted, …) are compiled as the built-in regardless of the receiver — the user method never runs #10476).imported_array_methods.rsgates onimported_binding_is_array.inline_array_methods.rshandles literals only.Tests
test-files/test_gap_11187_call_result_array_named_methods.tscovers:anyholder's method.this.any.awaitreceivers.anycall results that are real arrays, and proven arrays (literal,number[]-returning function,split,Object.keys,Array.from,slice/filter/mapchains).find().sort,limit().sort,fn().sort,cond.sort,cond.mapand(await …).sortdiverge; theawaitcase ends in an unhandled rejection). Passes with the fix, byte-identical to Node 26.5.1 (/opt/node-v26.5.1-linux-x64). Both arms were perry-dev builds,PERRY_NO_AUTO_OPTIMIZE=1.array_only_methods_tests.rs:any_call_result_receiver_does_not_fold_to_array_methods: noArraySort/ArrayFlat/ArrayToSpliced/ArrayMapforanycall and conditional receivers.proven_array_call_results_keep_array_folds: the folds are kept fornumber[]returns,splitand literal chains.cargo test --profile perry-dev -p perry-hir -p perry-codegen: 92 test binaries, 0 failures.console.timetiming noise and tests that fail to compile in both arms outside the harness: package-backed tests and tests that import_helpers/. I re-ran the helper-importing ones in place (namespace_member_not_array_method,namespace_member_sort_dual,imported_member_array_method_names,gc_namespace_and_computed_dispatch_rooting), and each gives the same result in both arms.cargo fmt --checkandscripts/check_file_size.shpass.SKIP_COMPILE_GATES=1 scripts/run_lint_gates.sh: 88 of 90 script gates pass, compile tier not run. The 2 failures are the known-red "Public benchmark evidence freshness" and "Type-check Windows runtime and stdlib" (cargo xwin, which is not available on the Linux host).Perf
sort/map/filter/reduce/flat/toSpliced/some/forEachonnumber[]-returning calls, a literal and asplitchain) produces LLVM IR identical in both arms, apart from the embedded entry-path string.perf stat -e instructions:u, two iteration counts plus a bare-loop control:anycall result that is an array at runtime) now go through dynamic dispatch instead of the dense op. That is intended: the old fast path was only fast because it assumed the receiver was an array, which is the bug.mongodb 7.5.0 (bson 7.3.3, auto-optimize on, own mongod)
insertManywith E11000 duplicate_id, which is bson ObjectId: every generated id is identical (counter stuck at 1, PROCESS_UNIQUE zero) — mongodb insertMany E11000 #11157 (fixed by fix(hir): a per-evaluation class declaration's own statics resolve to its evaluation, not the template (#11157) #11188)..sort({a:1})all succeed.find().sort()now returns the cursor itself, as in Node. The next failure istoArray():TypeError: value is not a function, with or without.sort(). That is a separate, pre-existing defect that reproduces package-free on main: an inherited static (MongoDBResponse.make) called through a subclass (CursorResponse.make) reads its module-scope bindings wrongly. Filed as mongodb find().toArray(): an inherited static (MongoDBResponse.make) called through a subclass reads its module-scope bindings wrongly (value is not a function) #11200 with a reduced package-free repro.Not run
PERRY_NO_AUTO_OPTIMIZE=1.cargo test --workspace.find, and this change does not touch that path.Summary by CodeRabbit
.sort()and.map()on values not known to be arrays now use the receiver’s own method instead of being incorrectly treated as Array operations, preventing runtime errors in chained calls.