Skip to content

feat(arrays): array_slice() on an associative receiver - #1042

Merged
nahime0 merged 2 commits into
mainfrom
fix/683-assoc-slice-chunk
Sep 18, 2026
Merged

nahime0 merged 2 commits into
mainfrom
fix/683-assoc-slice-chunk

Conversation

@Guikingone

Copy link
Copy Markdown
Collaborator

Part of #683 — the array_slice() half. array_chunk() is filed separately; see Scope below.

Every associative receiver was refused at compile time — string-keyed and integer-keyed alike, with and without preserve_keys — behind two different messages:

array_slice(["x" => 1, "y" => 2, "z" => 3], 1, 2, true)
  unsupported EIR backend feature: array_slice preserve_keys for PHP type AssocArray { key: Str, value: Int }

array_slice(["x" => 1, "y" => 2, "z" => 3], 1, 2)
  unsupported EIR backend feature: array_slice for PHP type AssocArray { key: Str, value: Int }

The indexed receiver was fully supported in both modes, so the gap was the receiver being a hash, not the element type.

The helper

$offset and $length count positions in insertion order, not keys, which a hash cannot answer without a walk. __rt_hash_slice therefore walks the source through __rt_hash_iter_next and counts, exactly as __rt_hash_flip and __rt_hash_clone_shallow do. The window normalization is the existing emit_slice_bounds prologue, unchanged: it reads the source length from the first header word, and a hash stores its entry count there just as an indexed array stores its length.

One helper covers both preserve_keys modes, because they differ by a single per-entry decision and nothing else. preserve_keys is not "keep all keys" versus "drop all keys" — php-src only ever renumbers integer keys, and a string key survives either way:

call result
array_slice(["x"=>1,"y"=>2,"z"=>3], 1, 2) ["y"=>2, "z"=>3]
array_slice(["x"=>1,"y"=>2,"z"=>3], 1, 2, true) ["y"=>2, "z"=>3]
array_slice([5=>1, 9=>2, 12=>3], 1, 2) [0=>2, 1=>3]
array_slice([5=>1, 9=>2, 12=>3], 1, 2, true) [9=>2, 12=>3]

A mixed-key source shows the rule directly: [5=>"a", "k"=>"b", 9=>"c"] slices to [0=>"a", "k"=>"b", 1=>"c"] — only the integer entries move.

The result's key type is therefore the source's key type in both modes, which is already what the checker records for an associative source ("narrowing a hash preserves its keys"). No checker change was needed.

Ownership mirrors __rt_hash_clone_shallow, because the window holds the same values the source does: string keys and refcounted values are retained, string values are re-persisted through __rt_str_persist, scalars are copied inline, and a renumbered integer key needs no ownership at all.

Verified

Twenty-four shapes against host PHP 8.5.10, all byte-identical: string keys, integer keys, mixed keys, both modes; an omitted length, a negative offset, a negative length, an offset past the end, an offset before the start, a zero length, an over-long length, an empty source; string, float, bool and nested-array values; the source left untouched; and the indexed receiver still taking its own path in both modes.

The routing was confirmed load-bearing by disabling it: both tests fail with the original unsupported EIR backend feature: array_slice for PHP type AssocArray.

--gc-stats reports allocs=5600 frees=5600 over 200 iterations of five slice shapes, and a leak summary: clean heap-debug test pins the retains and releases the copy performs.

cargo test --test codegen_tests -- codegen::arrays passes in full (568), as do the 1681 lib tests and 1534 error tests.

iOS

test_hash_slice_is_emitted_for_every_supported_target emits the runtime for all five supported targets — macos-aarch64, ios-arm64, ios-sim-arm64, linux-aarch64, linux-x86_64 — and asserts each body reaches __rt_hash_iter_next, __rt_hash_new and __rt_hash_insert_owned. Those three are what make it a hash slice rather than a clone: the iterator is what gives positions their meaning, and the allocator is what makes the result a table of its own. The executable shards cover three of the five; the two iOS targets are covered by emission tests like this, which is what the policy asks for.

Docs and example

docs/php/arrays.md states the real rule on the array_slice() row, with both integer-key outcomes spelled out. docs/internals/the-runtime.md gains the __rt_hash_slice row next to the splice helpers. examples/assoc-arrays/main.php slices a roster by position and shows a mixed-key array where only the integer entries renumber; the whole example is byte-identical to host PHP 8.5.10.

Scope

array_chunk() on an associative receiver is the other half of #683 and is not in this PR. The two do not share an implementation: array_chunk's preserve_keys = false drops string keys too and restarts numbering inside each chunk, so the per-entry rule is different, and the result is a nested container rather than a window. Splitting it keeps each review to one helper rather than making this one carry a second ~350-line dual-architecture emitter it shares nothing with.

🤖 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:m Medium-sized pull request. type:feature Introduces new user-visible behavior or capabilities. labels Sep 16, 2026
@greptile-apps

greptile-apps Bot commented Sep 16, 2026

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

The PR appears safe to merge; no actionable correctness, security, ownership, target-support, or repository-rule issue remains.

Summary

Adds PHP-compatible array_slice() support for associative arrays.

  • Routes associative receivers through a new insertion-order hash-slicing runtime helper.
  • Preserves string keys in both modes while renumbering only integer keys when preserve_keys is false.
  • Implements matching AArch64 and x86_64 paths with explicit value/key ownership handling.
  • Adds cross-target emission, behavior, edge-case, and heap-clean regression coverage.
  • Updates the array/runtime documentation and associative-array example.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A[PHP array_slice call] --> B{Receiver storage}
    B -->|Indexed array| C[Existing indexed slice path]
    B -->|Associative hash| D[Normalize positional bounds]
    D --> E[Walk insertion order]
    E --> F{Key and preserve_keys}
    F -->|String key| G[Retain original key]
    F -->|Integer and true| H[Keep original integer key]
    F -->|Integer and false| I[Assign next sequential key]
    G --> J[Copy or retain value]
    H --> J
    I --> J
    J --> K[Insert owned entry into new hash]
    K --> L[Return independent slice]
Loading

Reviews (4) · Last reviewed commit: "Merge origin/main into fix/683-assoc-sli..."

@Guikingone Guikingone self-assigned this Sep 16, 2026
@Guikingone
Guikingone requested a review from nahime0 September 16, 2026 13:07
@Guikingone
Guikingone force-pushed the fix/683-assoc-slice-chunk branch from 5bdc728 to 3aa9127 Compare September 16, 2026 21:02
Every associative receiver was refused at compile time -- string-keyed and
integer-keyed alike, with and without `preserve_keys` -- behind two different
messages:

    array_slice(["x" => 1, "y" => 2, "z" => 3], 1, 2, true)
    unsupported EIR backend feature: array_slice preserve_keys for PHP type
    AssocArray { key: Str, value: Int }

    array_slice(["x" => 1, "y" => 2, "z" => 3], 1, 2)
    unsupported EIR backend feature: array_slice for PHP type
    AssocArray { key: Str, value: Int }

The indexed receiver was fully supported in BOTH modes, so the gap was the
receiver being a hash, not the element type.

## The helper

`$offset` and `$length` count POSITIONS in insertion order, not keys, which a hash
cannot answer without a walk. `__rt_hash_slice` therefore walks the source through
`__rt_hash_iter_next` and counts, exactly as `__rt_hash_flip` and
`__rt_hash_clone_shallow` do. The window NORMALIZATION is the existing
`emit_slice_bounds` prologue, unchanged: it reads the source length from the first
header word, and a hash stores its entry count there just as an indexed array
stores its length.

One helper covers both `preserve_keys` modes, because they differ by a single
per-entry decision and nothing else. `preserve_keys` is NOT "keep all keys" versus
"drop all keys" -- php-src only ever renumbers INTEGER keys, and a string key
survives either way:

    array_slice(["x"=>1,"y"=>2,"z"=>3], 1, 2)        ["y"=>2, "z"=>3]
    array_slice(["x"=>1,"y"=>2,"z"=>3], 1, 2, true)  ["y"=>2, "z"=>3]
    array_slice([5=>1, 9=>2, 12=>3], 1, 2)           [0=>2, 1=>3]
    array_slice([5=>1, 9=>2, 12=>3], 1, 2, true)     [9=>2, 12=>3]

A mixed-key source shows the rule directly: `[5=>"a", "k"=>"b", 9=>"c"]` slices to
`[0=>"a", "k"=>"b", 1=>"c"]` -- only the integer entries move.

The result's key type is therefore the SOURCE's key type in both modes, which is
already what the checker records for an associative source ("narrowing a hash
preserves its keys"). No checker change was needed.

OWNERSHIP mirrors `__rt_hash_clone_shallow`, because the window holds the same
values the source does: string keys and refcounted values are retained, string
values are re-persisted through `__rt_str_persist`, scalars are copied inline, and
a renumbered integer key needs no ownership at all.

## Verified

Twenty-four shapes against host PHP 8.5.10, all byte-identical: string keys,
integer keys, mixed keys, both modes; an omitted length, a negative offset, a
negative length, an offset past the end, an offset before the start, a zero length,
an over-long length, an empty source; string, float, bool and nested-array values;
the source left untouched; and the indexed receiver still taking its own path in
both modes.

The routing was confirmed load-bearing by disabling it: both tests fail with the
original `unsupported EIR backend feature: array_slice for PHP type AssocArray` .

`--gc-stats` reports `allocs=5600 frees=5600` over 200 iterations of five slice
shapes, and a `leak summary: clean` heap-debug test pins the retains and releases
the copy performs.

`cargo test --test codegen_tests -- codegen::arrays` passes in full (568), as do
the 1681 lib tests and 1534 error tests.

## iOS

`test_hash_slice_is_emitted_for_every_supported_target` emits the runtime for all
five supported targets -- `macos-aarch64`, `ios-arm64`, `ios-sim-arm64`,
`linux-aarch64`, `linux-x86_64` -- and asserts each body reaches
`__rt_hash_iter_next`, `__rt_hash_new` and `__rt_hash_insert_owned`. Those three
are what make it a hash slice rather than a clone: the iterator is what gives
positions their meaning, and the allocator is what makes the result a table of its
own. The executable shards cover three of the five; the two iOS targets are covered
by emission tests like this, which is what the policy asks for.

## Docs and example

`docs/php/arrays.md` states the real rule on the `array_slice()` row, with both
integer-key outcomes spelled out. `docs/internals/the-runtime.md` gains the
`__rt_hash_slice` row next to the splice helpers. `examples/assoc-arrays/main.php`
slices a roster by position and shows a mixed-key array where only the integer
entries renumber; the whole example is byte-identical to host PHP 8.5.10.

## Scope

`array_chunk()` on an associative receiver is the other half of #683 and is NOT in
this commit. The two do not share an implementation: `array_chunk`'s
`preserve_keys = false` drops string keys too and restarts numbering inside each
chunk, so the per-entry rule is different, and the result is a nested container
rather than a window. Filed separately so neither review has to carry the other.

Part of #683

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
@nahime0

nahime0 commented Sep 18, 2026

Copy link
Copy Markdown
Member

Grok review (READ-ONLY)

Verdict: sound with nits — merge yes, with follow-ups.

Closes the AOT gap for array_slice() on an AssocArray receiver. No blockers.

What this PR changes

Routing in lower_array_slice peels AssocArray before the indexed / Mixed paths and calls __rt_hash_slice with the existing bounds tuple plus a preserve_keys flag. New AArch64 + x86_64 helper: emit_slice_bounds__rt_hash_new → insertion-order walk via __rt_hash_iter_next → per-entry key rewrite → clone-shallow ownership → __rt_hash_insert_owned.

preserve_keys matches php-src: only integer keys are renumbered when the flag is false; string keys always survive. Result key type stays the source’s. Indexed fixtures still hit the old helpers.

What looks fine

  • Insertion-order window; shared bounds prologue (count at [hash+0]).
  • Ownership / COW aligned with __rt_hash_clone_shallow.
  • ABI / callee-saved / five-target emission coverage.
  • Docs + example state the real PHP int-vs-string rule.
  • array_chunk correctly left out of this PR.

Follow-ups opened

Issue Topic
#1091 array_chunk() on AssocArray (remaining half of #683)
#1092 first-class/synthetic array_slice on AssocArray still rejected (array<mixed> result)
#1093 nit: dedupe lower_array_slice rustdoc; optionally save AArch64 preserve_keys before emit_slice_bounds

Keep associative array_slice() from #1042 and take main's
array_push/unset-by-ref updates in the shared example and docs.

Co-authored-by: Vincenzo Petrucci <nahime0@users.noreply.github.com>
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:m Medium-sized pull request. type:feature Introduces new user-visible behavior or capabilities.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants