Skip to content

feat(arrays): array_slice() on an indexed string array - #1035

Open
Guikingone wants to merge 2 commits into
mainfrom
fix/675-array-slice-string-slots
Open

Guikingone wants to merge 2 commits into
mainfrom
fix/675-array-slice-string-slots

Conversation

@Guikingone

Copy link
Copy Markdown
Collaborator

Part of #675 — the array_slice member. The issue's headline repro:

print_r(array_slice(["a", "b", "c"], 1, 2));
unsupported EIR backend feature: array_slice indexed-array element PHP type Str

An indexed array<string> stores 16-byte {pointer, length} slots, and both existing helpers copy 8 bytes per element — so a string receiver was refused at compile time rather than miscompiled. __rt_array_slice_str copies the pair.

Following array_splice_str

The issue names __rt_array_splice_str as the model, and this follows it in both respects.

The destination is allocated with 16-byte slots, and the copy duplicates through __rt_array_push_str (which persists via __rt_str_persist) rather than aliasing. A string array owns its bytes exclusively and array_slice() leaves its argument untouched, so both arrays end up owning independent copies and freeing either one is safe.

The window arithmetic is not re-derived: it goes through the shared emit_slice_bounds prologue every other array_slice variant uses, so negative offsets, negative lengths, clamping and an omitted $length behave identically by construction rather than by coincidence.

One detail worth flagging for review: the loop index is spilled rather than parked in a register. __rt_array_push_str persists the payload and may grow the destination, so it is a full call and every caller-saved register is fair game across it.

Measured

Eighteen rows byte-identical to the host PHP 8.5.10 — the whole offset/length matrix (mid, from, all, neg-off, neg-off-len, neg-len, neg-both, zero-len, past-end, too-long, too-far-back, empty-src, null-len), plus the four that are really about ownership:

write into the result source unchanged
slice returned from a function outlives the local it came from
slice of a slice independent again
100/200/300-byte strings real heap buffers, not anything inline

200 slices in a loop report allocs=1400 frees=1400 under --gc-stats — no leak, no double free. That is asserted as its own test, because getting the duplication wrong in either direction is silent at the value level: the strings still print.

Verified against the defect by putting the layout gate back: the fixture then fails with the exact message above.

Suites: codegen::arrays 568, codegen::strings 352, codegen::runtime_gc 298, error_tests 1534, --bin elephc 1915 — clean.

Still refused, deliberately not in this PR

array_chunk, array_pad, array_reverse, array_merge, array_unique, array_diff and shuffle still report their own variant of the message, so #675 stays open.

The first four are the same copy shape as this one and should follow quickly. array_unique and array_diff need string comparison over 16-byte slots, and shuffle needs in-place 16-byte swaps — different pieces of work with their own ownership questions. Landing them one at a time keeps each helper's contract reviewable on its own rather than burying eight assembly emitters in one diff.

🤖 Generated with Claude Code

https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr

@github-actions github-actions Bot added area:builtins Touches PHP builtin declarations or emitters. area:codegen Touches target-aware assembly or backend lowering. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. size:s Small pull request. type:feature Introduces new user-visible behavior or capabilities. labels Sep 15, 2026
@greptile-apps

greptile-apps Bot commented Sep 15, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; the implementation, ownership behavior, documentation, example, and supported-target coverage are aligned.

Summary

This PR enables array_slice() for indexed string arrays by selecting a dedicated runtime helper that allocates 16-byte string slots and persists each copied string so the result owns independent storage.

  • Adds AArch64 and Linux x86_64 implementations of __rt_array_slice_str.
  • Routes indexed string arrays to the new helper while retaining shared slice-bound normalization.
  • Adds PHP behavior, ownership/GC, and supported-target emission regressions.
  • Adds the required example and updates user-facing and internal runtime documentation.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A["array_slice(array&lt;string&gt;, offset, length)"] --> B["Shared slice-bound normalization"]
    B --> C["__rt_array_new with 16-byte slots"]
    C --> D["Read each {pointer, length} pair"]
    D --> E["__rt_array_push_str"]
    E --> F["Persist independent string bytes"]
    F --> G["Return independently owned slice"]
Loading

Reviews (5) · Last reviewed commit: "docs(arrays): the example, the runtime p..."

Comment thread tests/codegen/arrays/indexed/slice_bounds.rs
Comment thread src/codegen_support/runtime/arrays/array_slice_str.rs
Comment thread src/codegen_support/runtime/arrays/array_slice_str.rs
@Guikingone Guikingone self-assigned this Sep 15, 2026
@Guikingone
Guikingone requested a review from nahime0 September 15, 2026 21:06
@Guikingone
Guikingone force-pushed the fix/675-array-slice-string-slots branch 2 times, most recently from 8fe4e15 to 2978d4f Compare September 18, 2026 13:09
Part of #675, the `array_slice` member.

    print_r(array_slice(["a", "b", "c"], 1, 2));

    unsupported EIR backend feature: array_slice indexed-array element PHP type Str

An indexed `array<string>` stores 16-byte `{pointer, length}` slots, and both
existing helpers copy 8 bytes per element -- so a string receiver was refused at
compile time rather than miscompiled. `__rt_array_slice_str` copies the pair.

The issue names `__rt_array_splice_str` as the model, and this follows it in both
respects. The destination is allocated with 16-byte slots, and the copy
DUPLICATES through `__rt_array_push_str` (which persists via `__rt_str_persist`)
rather than aliasing: a string array owns its bytes exclusively, `array_slice()`
leaves its argument untouched, so both arrays end up owning independent copies
and freeing either one is safe.

The window arithmetic is not re-derived. It goes through the shared
`emit_slice_bounds` prologue every other `array_slice` variant uses, so negative
offsets, negative lengths, clamping and an omitted `$length` behave identically
by construction.

The loop index is SPILLED rather than parked in a register: `__rt_array_push_str`
persists the payload and may grow the destination, so it is a full call and every
caller-saved register is fair game across it.

Eighteen rows byte-identical to the host PHP 8.5.10 -- the whole offset/length
matrix, plus the four that are really about ownership: writing into the result
leaves the source intact, a slice outlives the local it came from, a slice of a
slice is independent again, and 100/200/300-byte strings force real heap buffers.

200 slices in a loop report `allocs=1400 frees=1400` under `--gc-stats`: no leak,
no double free. That is asserted as a test, because getting the duplication wrong
in either direction is silent at the value level.

Suites: `codegen::arrays` 568, `codegen::strings` 352, `codegen::runtime_gc` 298,
`error_tests` 1534, `--bin elephc` 1915 -- clean.

`array_chunk`, `array_pad`, `array_reverse`, `array_merge`, `array_unique`,
`array_diff` and `shuffle` still report their own variant of the message. The
first four are the same copy shape as this one; `array_unique`/`array_diff` need
string COMPARISON over 16-byte slots and `shuffle` needs in-place 16-byte swaps,
which are different pieces of work. Landing them one at a time keeps each
helper's ownership contract reviewable on its own.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Review follow-up: three repository requirements the first commit did not meet.

`examples/arrays/main.php` already demonstrated `array_splice()` on a string
array, next to a comment about elements "wider than a scalar slot". The
`array_slice()` counterpart now sits beside it, and shows the property that
separates a correct 16-byte copy from a plausible-looking one -- writing into the
result leaves the source alone:

    Sliced names: GRACE, Linus (source still Ada, Grace, Linus, Barbara)

The whole example is byte-identical to the host PHP 8.5.10, not just the new row.

`docs/internals/the-runtime.md` gains the `__rt_array_slice_str` row beside
`__rt_array_splice_str`, stating the slot width, that the window arithmetic is
the shared `slice_bounds` prologue, and why the copy DUPLICATES rather than
aliases.

`test_array_slice_str_is_emitted_for_every_supported_target` emits the runtime
for all five targets -- `macos-aarch64`, `ios-arm64`, `ios-sim-arm64`,
`linux-aarch64`, `linux-x86_64` -- and asserts the helper is present, asks
`__rt_array_new` for 16-byte slots, and copies through `__rt_array_push_str`.

The slot width is asserted rather than assumed because it is the one number that
makes this helper different from its 8-byte siblings: get it wrong and the copy
still runs, reading half of each pair. The executable shards cover three of those
targets; the two iOS ones are covered only by emission tests like this, which is
what the policy asks for.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
@Guikingone
Guikingone force-pushed the fix/675-array-slice-string-slots branch from 2978d4f to 8ef5a2c Compare September 19, 2026 07:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:builtins Touches PHP builtin declarations or emitters. area:codegen Touches target-aware assembly or backend lowering. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. size:s Small pull request. type:feature Introduces new user-visible behavior or capabilities.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant