fix(codegen): own method beats a Date/Array builtin on unproven receivers (#11493) - #11529
Merged
Merged
Conversation
…vers (#11493) `const d: any = new Date(0); d.getTime = () => 42; d.getTime()` printed 0. #10943 added the own-override test for receivers whose kind is proven, but #10476's receiver-kind guard, which serves UNPROVEN receivers, still went straight to the builtin once the runtime check said "this is a Date" (or a plain array). A matching kind says nothing about own properties. - builtin_kind_guard: a heap receiver that passes the Date / plain-array check (including the Date arm of the toLocaleString guard) now takes the #10943 own-override test before the builtin, and falls through to the universal dispatcher when it may own the name. Numbers and Symbols are primitives and skip it. The test may allocate, so the receiver and every argument are rooted across it and re-read in each arm. - own_override_guard: the proven-receiver name list now comes from the kind guard's own Date table, so a proven Date's getUTCHours, setTime, toUTCString, ... get the diamond too, and it covers the array names the kind guard handles (toReversed, toSorted, toSpliced, reduceRight, copyWithin). - folded_builtin_override: add the zero-argument Date folds that were missing from the table (getTimezoneOffset, toJSON, toDateString, toTimeString, toLocaleDateString, toLocaleTimeString, toLocaleString). DateToUTCString stays out: HIR folds toUTCString and toGMTString into it, so the node does not know which name to test. test_gap_11493_own_method_beats_date_builtin.ts: 71 of 89 lines differ from node on main, 0 with this change (TZ=UTC, America/New_York, Asia/Kolkata).
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (6)
✨ 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 |
proggeramlug
pushed a commit
that referenced
this pull request
Sep 27, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WV7PJ81Uf8F8YQMvhKKSyf
proggeramlug
force-pushed
the
claude/brave-lovelace-kizfge
branch
from
September 27, 2026 13:08
fd2789e to
f72ebea
Compare
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
const d: any = new Date(0); d.getTime = () => 42; d.getTime()printed0; Node prints42. #10943 added the own-override test for receivers whose kind the compiler proves. It did not cover the #10476 receiver-kind guard, which handles unproven receivers. That guard checks at runtime that the value is a Date (or a plain array) and then called the builtin directly, but a matching kind says nothing about own properties. This PR adds the #10943 test to that path. It also closes the gaps I found next to it on the proven path.Changes
lower_call/property_get/builtin_kind_guard.rs(the bug in the issue)emit_own_override_branchbefore the builtin. So does the Date arm of thetoLocaleString()guard. A receiver that may own the name takes the existing universal-dispatch arm.time(the Date's time value) is a Number, so it is reused.lower_call/property_get/own_override_guard.rsshadowable_builtin_namenow comes from the kind guard's owndate_builtintable (is_direct_date_builtin_name) instead of a hand-kept list. The old list stopped short, so a proven Date'sgetUTCHours,setTime,toUTCString, … reached the direct builtin with no diamond.toReversed,toSorted,toSpliced,reduceRight,copyWithin). Without them, the same call answered differently depending on what the compiler proved.expr/folded_builtin_override.rsm.get = () => x; m.get()runs the native method (wrong value, plain JS, on main) #10943 table:getTimezoneOffset,toJSON,toDateString,toTimeString,toLocaleDateString,toLocaleTimeString,toLocaleString.DateToLocaleStringnode ((12345).toLocaleString()) keeps the plain fold.DateToUTCStringis deliberately left out, with a comment and a test saying why. HIR folds bothtoUTCStringandtoGMTStringinto that one node, so the node doesn't record which name to test or dispatch.test-files/test_gap_11493_own_method_beats_date_builtin.ts(89 rows) covers:this/arguments, an own accessor (run once), a non-callable own value (TypeError), delete-restores-builtin, a borrowed builtin, and an own method installed mid-loop;builtin_kind_guard_tests.rsand two table tests infolded_builtin_override.rs. The IR tests assert the test is present on the Date, array and locale arms and absent for Numbers, and useassert_rooted_acrossto check that the receiver reaches both the predicate and the dispatcher through its root.No runtime change and no new side table. Version not bumped. The changelog fragment follows in a second commit, keyed to this PR number.
Related issue
Fixes #11493
Test plan
Local, Linux x86-64,
--profile perry-dev, LLVM 22.1.8.main=63e89977built the same way.main; 0 differ with this change, underTZ=UTC,America/New_YorkandAsia/Kolkata.cargo test -p perry-codegen(RUST_TEST_THREADS=1): all 41 test binaries pass, including 1764 lib tests and the 9 new ones.test-files/name matching date/own/override/kind/expando/locale/toSorted/reduce/flat/dayjs/…): 120 byte-identical to Node. The other 13 behave the same onmain: identical output, a missing npm package (date-fns, dayjs) or UI library at link time,Date.now()output, or a feature Node 22 lacks. 0 regressions.gc_root_dominance_check.py --moving-onlyon the IR of the new test plus 4 Date/own-override tests: 0 violations on both builds, with root stores up from 595 to 675.--stale-registersdrops from 286 to 233 on the same sources.--unrooted-allocas: 0 on both.scripts/run_lint_gates.sh(SKIP_COMPILE_GATES=1): 89 of 92 pass. The 3 failures are environmental or already red onmain: nocargo xwinhere; the benchmark-artifact gate sees zero RSS samples in this sandbox and no Bun; public-benchmark freshness.cargo test --workspace, the auto-optimize path. CI covers these.Cost
Instruction counts from
valgrind --tool=callgrind, two-N differential, two Date calls per iteration, unproven receiver:Mapexpandomain, so this makes the unproven path consistent rather than adding a new cost. Most of it is the predicate's key allocation plushasOwn, which perf(runtime): memoize %Function.prototype% per realm in the dispatcher's own-override check (#10497) #11491 is shrinking for both paths.Not covered
toUTCString/toGMTStringon a Date that HIR proves (the shared fold above).findLast/findLastIndex, which the kind guard never handled either.Generated by Claude Code