Skip to content

fix(hir): dispatch Array-mutator methods on interface-typed receivers to their own method#5492

Merged
proggeramlug merged 1 commit into
PerryTS:mainfrom
machineloop:fix/interface-arraylike-dispatch
Jun 21, 2026
Merged

fix(hir): dispatch Array-mutator methods on interface-typed receivers to their own method#5492
proggeramlug merged 1 commit into
PerryTS:mainfrom
machineloop:fix/interface-arraylike-dispatch

Conversation

@machineloop

@machineloop machineloop commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Summary

A method whose name collides with an Array.prototype mutator (push/pop/shift/…),
called on a receiver whose static type is a non-class named type (a user interface or a
function/factory return type), was folded to the array fast-path intrinsic instead of the
object's own method — so the call silently no-opped. This extends the #5139 fix (which only
covered any-typed receivers) to interface-typed and factory-return-typed receivers.

Changes

  • crates/perry-hir/src/lower/context.rs — add LoweringContext::is_interface_type(name)
    (a name registered in self.interfaces).
  • crates/perry-hir/src/lower/expr_call/local_array_methods.rs and …/array_only_methods.rs
    — the is_user_class_* Type::Named arm now also accepts ctx.is_interface_type(name), so
    an interface receiver skips the array-only-method fold and dispatches to its own method.
    (Kept the check precise rather than a blanket Type::Named(_) => true, so TypedArrays —
    Int8Array etc. — still legitimately fold for fill/map; real arrays are never Type::Named.)
  • crates/perry/tests/interface_typed_arraylike_method_dispatch.rs — regression tests.

Related issue

Refs #5139 (extends that fix to interface- / factory-return-typed receivers). No dedicated
issue — happy to file one for tracking if preferred.

Test plan

cargo test -p perry --test interface_typed_arraylike_method_dispatch   # 3 passed
cargo fmt -p perry-hir -p perry --check                                # clean
cargo build --release --workspace --exclude perry-ui-{ios,tvos,watchos,gtk4,android,windows}

TDD red→green: before the fix the interface cases printed len=0 (the push was dropped);
after, len=2. The real-array control (number[] push/pop) still prints sum=3 len=2 popped=3.

  • cargo build --release -p perry clean (the crates this change touches). The full
    --workspace release build fails only on windows-future — a Windows-only transitive
    dep that can't compile on this macOS host (env limitation, unrelated to this change).
  • cargo test --workspace … passes — my new tests pass; the suite's only failures
    (integer_locals_provenance, functional_batch2_regressions, and a
    blocklist_addsubnet_prefix TempDir::join compile break) reproduce identically on
    clean main without this change
    , so they are pre-existing and unrelated.
  • (if user-facing) Added a #[test] in the affected crate (crates/perry/tests/)
  • (if CLI / stdlib / runtime API changed) Updated docs/src/ — N/A (internal HIR dispatch)
  • (if touching a platform UI backend) — N/A

Screenshots / output

interface Sink { items: string[]; push(x: string): void; }
const s: Sink = { items: [], push(x) { this.items.push(x); } };
s.push("a"); s.push("b");
console.log("len=" + s.items.length);
// before: len=0   after: len=2

Checklist

  • I have NOT bumped the workspace version or edited CLAUDE.md / CHANGELOG.md
  • My commits follow the loose fix: prefix convention
  • I've read CONTRIBUTING.md and agree to the Code of Conduct

Summary by CodeRabbit

  • Bug Fixes

    • Fixed incorrect handling of method calls on interface-typed receiver objects. Previously, these were being incorrectly optimized as array operations instead of dispatching to the intended methods.
  • Tests

    • Added regression tests for method dispatch on interface-typed receiver objects.

… to their own method

A method call whose name collides with an Array.prototype mutator
(push/pop/shift/unshift/splice/sort/reverse/concat) on a receiver whose static
type is a (non-class) named type — an interface or a function/factory return
type — was eagerly lowered to the array fast path (Expr::ArrayPush / the
array.push_single native arm). That reads the plain object header as an
ArrayHeader, so an object that merely owns a same-named closure property had its
push silently dropped: the call returned a bogus numeric length and the user
method never ran.

Same class of bug as PerryTS#5139 (which fixed any-typed receivers): HIR lowering's
array-only-method fold treated Type::Named as a user receiver only when
lookup_class(name) matched a class, so an interface (not a class) fell through.

Add LoweringContext::is_interface_type and recognize interface-typed receivers
as user objects in both fold paths (local_array_methods and array_only_methods),
so the method dispatches through the runtime js_native_call_method, which selects
by the receiver's runtime shape (real array -> dense helper; object with an own
callable -> that method).
@coderabbitai

coderabbitai Bot commented Jun 21, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 39595b7f-0b93-4a98-9301-3a9852e94ac1

📥 Commits

Reviewing files that changed from the base of the PR and between d30bfa5 and e6a1905.

📒 Files selected for processing (4)
  • crates/perry-hir/src/lower/context.rs
  • crates/perry-hir/src/lower/expr_call/array_only_methods.rs
  • crates/perry-hir/src/lower/expr_call/local_array_methods.rs
  • crates/perry/tests/interface_typed_arraylike_method_dispatch.rs

📝 Walkthrough

Walkthrough

Adds is_interface_type to LoweringContext and uses it in two array-method lowering guards (try_array_only_methods, try_local_array_methods) so that interface-typed named receivers are treated as user instances and skip the array fast-path. Three regression tests verify interface, factory-returned interface, and real array dispatch behavior.

Changes

Interface receiver guard for array fast-path lowering

Layer / File(s) Summary
is_interface_type predicate
crates/perry-hir/src/lower/context.rs
Adds pub(crate) fn is_interface_type(&self, name: &str) -> bool to LoweringContext, checking self.interfaces to determine whether a name is a declared interface.
Array fast-path guard updates
crates/perry-hir/src/lower/expr_call/array_only_methods.rs, crates/perry-hir/src/lower/expr_call/local_array_methods.rs
Extends the Type::Named receiver checks in try_array_only_methods (is_user_class_receiver) and try_local_array_methods (is_user_class_instance) to also bail when ctx.is_interface_type(name) is true, preventing array built-in fast-paths from being applied to interface-typed objects.
Regression tests
crates/perry/tests/interface_typed_arraylike_method_dispatch.rs
Adds compile_and_run helper and three end-to-end tests: an interface-typed receiver with its own push, a factory-function-returned interface receiver, and a control test confirming real array push/pop semantics are unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • PerryTS/perry#5194: Modifies the same array-mutator lowering files to skip the array fast-path based on Any/Unknown/None receiver static types, directly adjacent to the interface-type guard added here.
  • PerryTS/perry#5386: Updates the same try_array_only_methods/try_local_array_methods dispatch guards in the same directory to prevent incorrect array folding based on receiver type classification.
  • PerryTS/perry#5454: Changes try_array_only_methods folding decisions for push/indexOf/includes based on receiver static form, the same mechanism extended here for interface types.

Poem

🐇 A rabbit once typed with an interface in hand,
And push tried to fold where it shouldn't have land—
is_interface_type said "nope, bail out of here!"
The fast-path stepped back, and dispatch rang clear.
Three tests confirmed arrays still hop as they should,
The bug is now fixed in the lowering wood! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: fixing array-mutator method dispatch on interface-typed receivers, which aligns with the PR's core objective.
Description check ✅ Passed The PR description comprehensively covers the summary, changes, related issue, test plan with commands, and all checklist items are addressed.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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

@proggeramlug proggeramlug merged commit b73bc49 into PerryTS:main Jun 21, 2026
15 checks passed
@proggeramlug

Copy link
Copy Markdown
Contributor

Thanks @machineloop great PR

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