Skip to content

fix(lru-cache): dispatch cache.size property read to js_lru_cache_size - #7156

Merged
proggeramlug merged 1 commit into
PerryTS:mainfrom
jdalton:fix/handle-size-property-getter
Aug 1, 2026
Merged

fix(lru-cache): dispatch cache.size property read to js_lru_cache_size#7156
proggeramlug merged 1 commit into
PerryTS:mainfrom
jdalton:fix/handle-size-property-getter

Conversation

@jdalton

@jdalton jdalton commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Reading the number of entries in an lru-cache the normal way, as the property cache.size, returns undefined instead 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-cache is one of them. When the compiler sees a bare member read on one of those objects, such as cache.size with 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 plain PropertyGet, which just looks the name up on the object. That decision is made in crates/perry-hir/src/lower/expr_member.rs, and it emits the getter form only when is_native_dispatch_member(module, class, property) returns true. Everything else falls through to the plain PropertyGet default.

The distinction matters because both lowerings are correct for different members. A getter such as size holds a value that only the native side knows. A method such as get needs to stay a plain PropertyGet when read without parentheses, because const fn = cache.get should hand back the function itself, not invoke it.

Root cause

is_native_dispatch_member, in crates/perry-hir/src/lower/expr_member/native_dispatch.rs, had no arm for "lru-cache" at all. A bare cache.size therefore reached the catch-all _ => false and lowered to a plain PropertyGet.

That PropertyGet reached the runtime's handle-property lookup, and that lookup has no size handler for the compact lru-cache handle, so it returned undefined. Nothing further down the chain was broken: the js_lru_cache_size extern exists and the lru-cache size dispatch row is correct, declared as has_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 in local_natives.rs and lower_call rewrites it to NativeMethodCall { module: "lru-cache", method: "size" }, which code generation already routes to js_lru_cache_size through the native dispatch table.

The change

The "lru-cache" => prop == "size" arm is added to is_native_dispatch_member, mirroring the handle-backed getter arms that already exist for blob (size, type, and others), readline (line and terminal), __disposable__ (disposed), and perry/ui State (value).

size is the only lru-cache getter the runtime and native table expose, so the arm is deliberately narrow. The other members (get, set, has, delete, clear, and peek) stay as method-value reads that lower to a plain PropertyGet, 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 existing lru-cache size dispatch row handles it.

Verification runs

A new regression unit test, lru_cache_dispatches_only_the_size_getter in native_dispatch.rs, asserts that size dispatches and that the methods do not.

Running --print-hir over new LRUCache({max:10}); …; cache.size now shows NativeMethodCall { module: "lru-cache", method: "size", args: [] } where it previously showed a plain PropertyGet.

Compiling and running new LRUCache(...); cache.set(...); cache.size prints 2, where it previously printed undefined. The full test-files/test_parity_lru_cache.ts fixture now prints correct size: values of 3, 3, 2, and 0.

cargo test -p perry-hir native_dispatch is green. cargo test -p perry-codegen shows no new failures; its three manifest_consistency failures relating to iovalkey, dotenv, nanoid and others already exist on main and are unrelated to this change.

How this relates to #7136

The faithful lru-cache rewrite in #7136 is not on main yet, 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.size reports the current runtime's entry count. Once it lands, cache.size will report the faithful entry count. The dispatch fix is correct in both cases.

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
jdalton force-pushed the fix/handle-size-property-getter branch from 5f69ada to 9e79133 Compare August 1, 2026 01:31
@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The native member dispatcher now routes lru-cache size property reads to js_lru_cache_size. Method-valued reads remain on normal property handling. Regression coverage verifies both behaviors.

Changes

LRU cache dispatch

Layer / File(s) Summary
Size getter behavior
crates/perry-hir/src/lower/expr_member/native_dispatch.rs, changelog.d/7156-lru-cache-size-property-getter.md
The dispatcher recognizes bare lru-cache.size reads and preserves normal handling for get, set, has, and clear. Regression coverage verifies the dispatch behavior. The changelog records the fix.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: proggeramlug

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: dispatching lru-cache size property reads to the native getter.
Description check ✅ Passed The description clearly covers the bug, root cause, implementation, related issue, and verification, but omits the template headings and checklist.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@proggeramlug
proggeramlug merged commit 072e7ac into PerryTS:main Aug 1, 2026
25 of 37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants