fix(lru-cache): dispatch cache.size property read to js_lru_cache_size - #7156
Merged
proggeramlug merged 1 commit intoAug 1, 2026
Merged
Conversation
A handle-backed native instance's zero-arg data getters lower to a 0-arg NativeMethodCall only when `is_native_dispatch_member` recognizes the `(module, class, property)` triple. `lru-cache` had no arm, so a bare `cache.size` fell through to the inverted default (a plain PropertyGet) and the runtime's handle-property lookup — which has no `size` handler for the compact lru-cache handle — returned `undefined`. The method-call form `cache.size()` was unaffected (it routes through the call-expression path to the existing js_lru_cache_size dispatch row). Add the `"lru-cache" => prop == "size"` arm so the property read dispatches to js_lru_cache_size, mirroring the `blob` (size/type/…) and `__disposable__` (disposed) getter arms. Add a regression unit test.
jdalton
force-pushed
the
fix/handle-size-property-getter
branch
from
August 1, 2026 01:31
5f69ada to
9e79133
Compare
📝 WalkthroughWalkthroughThe native member dispatcher now routes ChangesLRU cache dispatch
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 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 |
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.
Reading the number of entries in an
lru-cachethe normal way, as the propertycache.size, returnsundefinedinstead of a number. Any program that checks its cache size, logs it, or branches on it gets the wrong answer silently, with no error raised. Calling it as a method,cache.size(), has always worked, so the bug looks arbitrary from the outside: the same value is reachable one way and not the other.This adds the one missing entry that makes the property form dispatch to the same native function the method form already reaches. No code generation changes are needed, and no other cache member changes behavior.
How a native property read is compiled
Some Perry objects are backed by a compact native handle rather than by an ordinary JavaScript object, and
lru-cacheis one of them. When the compiler sees a bare member read on one of those objects, such ascache.sizewith no call parentheses, it has to decide between two lowerings.It can emit a zero-argument
NativeMethodCall, which is the getter form and invokes the native function behind the property. Or it can emit a plainPropertyGet, which just looks the name up on the object. That decision is made incrates/perry-hir/src/lower/expr_member.rs, and it emits the getter form only whenis_native_dispatch_member(module, class, property)returnstrue. Everything else falls through to the plainPropertyGetdefault.The distinction matters because both lowerings are correct for different members. A getter such as
sizeholds a value that only the native side knows. A method such asgetneeds to stay a plainPropertyGetwhen read without parentheses, becauseconst fn = cache.getshould hand back the function itself, not invoke it.Root cause
is_native_dispatch_member, incrates/perry-hir/src/lower/expr_member/native_dispatch.rs, had no arm for"lru-cache"at all. A barecache.sizetherefore reached the catch-all_ => falseand lowered to a plainPropertyGet.That
PropertyGetreached the runtime's handle-property lookup, and that lookup has nosizehandler for the compactlru-cachehandle, so it returnedundefined. Nothing further down the chain was broken: thejs_lru_cache_sizeextern exists and thelru-cachesizedispatch row is correct, declared ashas_receiver: true, args: &[], ret: NR_F64. The value simply was never asked for.cache.size()was unaffected because a call expression takes a different route entirely. The call-expression path inlocal_natives.rsandlower_callrewrites it toNativeMethodCall { module: "lru-cache", method: "size" }, which code generation already routes tojs_lru_cache_sizethrough the native dispatch table.The change
The
"lru-cache" => prop == "size"arm is added tois_native_dispatch_member, mirroring the handle-backed getter arms that already exist forblob(size,type, and others),readline(lineandterminal),__disposable__(disposed), andperry/uiState(value).sizeis the onlylru-cachegetter the runtime and native table expose, so the arm is deliberately narrow. The other members (get,set,has,delete,clear, andpeek) stay as method-value reads that lower to a plainPropertyGet, and their call forms keep dispatching through the call-expression path as before.No code generation change is required. Once the getter lowers to a
NativeMethodCall, the existinglru-cachesizedispatch row handles it.Verification runs
A new regression unit test,
lru_cache_dispatches_only_the_size_getterinnative_dispatch.rs, asserts thatsizedispatches and that the methods do not.Running
--print-hirovernew LRUCache({max:10}); …; cache.sizenow showsNativeMethodCall { module: "lru-cache", method: "size", args: [] }where it previously showed a plainPropertyGet.Compiling and running
new LRUCache(...); cache.set(...); cache.sizeprints2, where it previously printedundefined. The fulltest-files/test_parity_lru_cache.tsfixture now prints correctsize:values of3,3,2, and0.cargo test -p perry-hir native_dispatchis green.cargo test -p perry-codegenshows no new failures; its threemanifest_consistencyfailures relating toiovalkey,dotenv,nanoidand others already exist onmainand are unrelated to this change.How this relates to #7136
The faithful
lru-cacherewrite in #7136 is not onmainyet, and the two changes do not conflict. This one is on the code-generation pipeline side and is independent: it corrects how the property getter is dispatched, not how the cache stores its values.Until #7136 lands,
cache.sizereports the current runtime's entry count. Once it lands,cache.sizewill report the faithful entry count. The dispatch fix is correct in both cases.