fix(runtime): #5552 demote unique strings in remaining array insert paths (unshift / fill / with / from_jsvalue)

[proggeramlug]

Closes #5552. Follow-up to #5533 and #5548.

The bug

A uniquely-owned (refcount==1) heap string written into an array element aliases that slot, so a later in-place s += x on the source local (js_string_append's refcount==1 fast path) rewrites the stored element and corrupts it. #5548 demoted the string at the array store paths it enumerated (push / set / from_values / inline element store / splice-insert); this PR covers the sibling insert/replace paths it left uncovered.

let s = "prefix";
s += "_init";       // append on shared -> fresh heap string, refcount==1
const a = ["x"];
a.unshift(s);       // (or a.fill(s) / a.with(0, s)) -> aliases the slot
s += "_more";       // refcount==1 in-place append -> must NOT corrupt the stored element
console.log("a0=" + a[0] + " s=" + s);  // expect a0=prefix_init s=prefix_init_more

The fix

Add the tag-checked js_string_addref_if_heap_string demote (no-op for SSO / non-string, idempotent) before the element write at each remaining path:

• js_array_unshift_f64 — covers js_array_unshift_jsvalue transitively — and the per-item loop in js_array_unshift_variadic.
• js_array_fill / js_array_fill_range (demote once before the fill loop — the source otherwise aliases the source local and every filled slot) and js_array_fill_generic (its object-receiver loop writes value into each index directly; the array receiver delegates to the two above, so the extra demote is idempotent).
• js_array_with — the replacement value stored into the new array's slot. The cloned elements come from the source array and are already shared.
• js_array_from_jsvalue — mixed-type literal construction, the JSValue sibling of the already-covered js_array_from_values.

Internal reshuffles (sort, splice tail shift, slice copy, copyWithin) only move values already stored in an array — already shared — so no demote is needed (same reasoning as #5548).

Tests

crates/perry/tests/string_append_heap_alias.rs gains compile-run regressions for unshift / fill / with, each verified to fail without the demote and pass with it.

js_array_from_jsvalue is not emitted by codegen from any TypeScript source (no compiled-TS test can reach it), so it gets a runtime unit test in crates/perry-runtime/src/array/tests.rs, also verified to fail without the demote.

All 12 integration tests + the runtime unit test pass with the fix.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Resolved string aliasing/corruption issues in array operations when the same heap string is later mutated in-place, affecting fill(), with(), and unshift().
  • Improved mixed-type array construction to prevent stored string values from being affected by subsequent string growth.

• Tests

  • Added unit and end-to-end regression tests covering unshift, fill, and immutable replacement (with()) to ensure array-stored string values remain stable after in-place += appends.

[coderabbitai]

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: 10e8f1a2-6c25-43c2-882d-fd80e62d20a2

📥 Commits

Reviewing files that changed from the base of the PR and between 23d0b8d and 9600499.

📒 Files selected for processing (1)

• crates/perry-runtime/src/array/push_pop.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• crates/perry-runtime/src/array/push_pop.rs

---

📝 Walkthrough

Walkthrough

Seven array mutation functions across four runtime files now call js_string_addref_if_heap_string before writing values into slots to demote uniquely-owned heap strings to shared form. New unit and end-to-end regression tests confirm the aliasing fix for each path.

Changes

Heap-string demotion in array insert/replace paths

Layer / File(s)	Summary
Demotion calls in fill, with, from_jsvalue, and unshift crates/perry-runtime/src/array/concat_reverse.rs, crates/perry-runtime/src/array/immutable.rs, crates/perry-runtime/src/array/jsvalue_api.rs, crates/perry-runtime/src/array/push_pop.rs	js_string_addref_if_heap_string(value) is inserted before element writes in all seven uncovered functions: js_array_fill, js_array_fill_range, js_array_fill_generic, js_array_with, js_array_from_jsvalue, js_array_unshift_f64, and js_array_unshift_variadic. In the three fill variants, the call is placed once before the fill loop to avoid redundant per-slot demotions.
Unit test: from_jsvalue demotion crates/perry-runtime/src/array/tests.rs	New #[test] manually sets refcount == 1 on a heap string with spare capacity, stores it via js_array_from_jsvalue, then runs js_string_append on the source and asserts the stored array element is not mutated by the in-place append.
E2E regression tests for unshift, fill, and with crates/perry/tests/string_append_heap_alias.rs	Three new compiled-TypeScript regression tests: each stores a snapshot of a unique non-SSO string via a.unshift(s), a.fill(s), or a.with(0, s), mutates the source via +=, and asserts the stored element(s) retain their pre-mutation value. Includes an introductory comment and a clarification note that js_array_from_jsvalue is covered by a unit test instead of a compiled-TS test because it is not emitted by current TypeScript codegen.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• PerryTS/perry#5533: Introduced js_string_addref_if_heap_string and applied the demotion to object field store paths — the direct prerequisite this PR extends to array insert/replace paths.
• PerryTS/perry#5548: Extended the same demotion to js_array_push_f64, js_array_set_f64, js_array_from_values, array literal init, splice insert, and related codegen choke points — the immediate predecessor that left the seven paths addressed here uncovered.

Poem

🐇 A string alone, refcount of one,
Slipped into arrays — corruption begun!
But fill, with, unshift, now demote with care,
And from_jsvalue joins the fix everywhere.
Safe sharing at last — no more aliasing blues!
The heap sleeps soundly in Perry's warm burrows. 🥕

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: fixing heap-string aliasing by demoting unique strings in remaining array insert paths (unshift/fill/with/from_jsvalue).
Description check	✅ Passed	The description comprehensively covers the bug, fix, testing approach, and reasoning. All required template sections are present with detailed explanations and examples.
Linked Issues check	✅ Passed	All coding requirements from issue #5552 are met: demote calls added to unshift (f64 and variadic), fill (three functions with demote before loop), with, and from_jsvalue paths. Tests added for unshift/fill/with and a runtime unit test for from_jsvalue.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the heap-string aliasing issue in array operations. No extraneous modifications to internal reshuffles or unrelated code paths.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch worktree-fix+5552-heap-string-alias

---

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