Skip to content

feat(arrays): ksort() and krsort() take PHP's sort flags - #1059

Open
Guikingone wants to merge 4 commits into
mainfrom
fix/699-ksort-krsort-sort-flags
Open

Guikingone wants to merge 4 commits into
mainfrom
fix/699-ksort-krsort-sort-flags

Conversation

@Guikingone

Copy link
Copy Markdown
Collaborator

Closes #699.

$a = ["img10" => 1, "img2" => 1];
ksort($a, SORT_NATURAL);
// EIR backend error: ksort expected 1 args, got 2

Both key sorts were unary and always compared keys with SORT_REGULAR. The six SORT_*
constants PHP defines for them did not exist either, so the argument had no spelling to pass.

The flag decides the answer, not the path

$sizes = [10 => "l", 9 => "m", 100 => "xl"];
ksort($sizes, SORT_NUMERIC);   // 9, 10, 100
ksort($sizes, SORT_STRING);    // 10, 100, 9

An integer key has no bytes of its own, so every byte-comparing mode has to spell it out as
decimal text first — zend_print_long_to_buf in php-src, a 24-byte buffer in the comparator's
own frame here. That is the whole reason 100 lands between 10 and 9.

How the mode is chosen

PHP resolves the comparator from $flags & ~SORT_FLAG_CASE and silently ignores everything it
does not recognize: SORT_ASC, SORT_DESC, a bare SORT_FLAG_CASE and 999 all mean
SORT_REGULAR. That resolution happens once per call, in __rt_hash_key_sort_enter, which
folds a small selector into the mode word the merge sort already carries. The comparison step
then reads a number between 0 and 6 instead of reasoning about the raw flag word n log n
times.

Selector 0 does not reach the new comparator at all: a flagless key sort still runs
__rt_key_compare_regular, untouched. An emitter test asserts that comparator is not
reimplemented in the new file.

What each mode reuses

SORT_STRING and its case-folded twin are __rt_strcmp and __rt_strcasecmp, the comparators
strcmp() and strcasecmp() already use, so ksort() cannot drift away from them.

SORT_NUMERIC reads a string key through __rt_str_to_number, which is PHP's numeric grammar
rather than libc's — no hexadecimal, no INF/NAN, and an exponent only when a digit follows
it, so "0x10" is 0.0 and "1e" is 1.0. Two integer keys compare exactly rather than
through a double, because 9223372036854775806 and 9223372036854775807 round to the same
f64 and PHP orders them.

SORT_NATURAL is the one thing that had to be written: __rt_strnatcmp, a port of php-src's
strnatcmp_ex with its quirks intact. Leading zeros are skipped once before the loop rather
than per run, whitespace is skipped on each side independently, a run starting with 0 on
either side compares left-aligned ("a0.5" before "a0.10") and any other run compares by
length first. Two operands that differ only in skipped bytes compare equal, which is why
strnatcmp("a 7", "a7") is 0. It is a leaf with no calls: it runs once per comparison inside
an O(n log n) sort.

php-src walks one byte past the operand in its whitespace skip and relies on the NUL its strings
always carry. An elephc string is a pointer/length pair with no terminator, so the skip
synthesizes that 0 at the boundary instead of reading whatever follows in memory.

Two boundaries, stated rather than papered over

SORT_LOCALE_STRING clips both operands at the first NUL and compares bytes. That is
strcoll in the C locale, and the C locale is the only one an elephc program can be in: there
is no PHP-visible setlocale(). Calling libc would mean two heap allocations per comparison to
manufacture the NUL-terminated operands strcoll wants, for the same answer. The one place it
differs from SORT_STRING is a key with an embedded NUL, and a test pins that pair.

SORT_NATURAL | SORT_FLAG_CASE folds ASCII az and nothing else. php-src folds with
libc toupper() there, which maps Latin-1 under Darwin's C locale and not under glibc's — PHP
itself answers differently on macOS and Linux for a byte above 127. Elephc gives glibc's answer
on every target. ASCII keys are unaffected, and a test pins the boundary by name.

Verified

Against host PHP 8.5.10, byte-identical: 210 sorts over 15 key sets and every mode including
999, 3, 4 and a bare SORT_FLAG_CASE, in both backends — compiled and eval(). The
sets cover PHP whitespace, 0x10/INF/NAN/abc, embedded NULs, exponents and dangling
exponents, leading zeros, fractional runs, negative keys and both int64 extremes, keys past
int64, single and empty arrays, bytes above 127, and integer keys mixed with string keys.

Through every receiver, since the sort relinks in place: a local, an object property, a nested
array cell, a by-reference parameter, named arguments in both orders, and a packed array that
krsort() promotes — that promotion used to run only when the call had exactly one argument.
The flag expression is evaluated once, in source order.

The x86_64 helpers were executed, not just assembled: __rt_strnatcmp and
__rt_key_compare_flagged were extracted from the emitted runtime, linked against a C driver
and run under Rosetta over the same PHP-derived cases (25 and 21 respectively, all passing).
All three target runtimes assemble clean.

--heap-debug reports leak summary: clean over 200 iterations of four modes: the comparator's
decimal buffer is in its own frame and nothing is allocated per comparison.

The behavioural tests were confirmed load-bearing by forcing the resolver back to
SORT_REGULAR: 7 of the 12 fail, and the 5 that pass are the ones that do not depend on the
comparison (evaluation order, promotion, heap hygiene, and the two that assert SORT_REGULAR
behaviour on purpose).

Out of scope, deliberately

sort(), rsort(), asort() and arsort() still take no flags; the comparison machinery is
reusable when they do. This matches the plan's own non-goals.

A non-numeric string in $flags is coerced to 0 rather than raising PHP's TypeError. That
is what elephc already does for every integer builtin parameter — str_repeat("z", "x") answers
"" where PHP raises — not something this change introduces.

The generated signature renders the default as int $flags = 0 rather than SORT_REGULAR: a
symbolic default cannot reach an eval registry binding, and both key sorts have one. The plan
anticipated this; the hand-written docs/php/arrays.md spells SORT_REGULAR.

Docs

docs/php/arrays.md gains a key-sort-flags section with the mode table and both boundaries;
docs/internals/the-runtime.md gains the three new runtime rows; examples/assoc-arrays
demonstrates numeric versus string versus natural key ordering and is byte-identical to PHP. The
generated builtin pages, registries and the compatibility page were regenerated with the
documented pipeline. .plans/ksort-krsort-php-sort-flags.md is marked implemented, with the
three places the implementation diverged from it.

Note for reviewers, unrelated to this change

Two pre-existing behaviours turned up while verifying and were left alone:

  • krsort() on a packed local, then passing that local to an array-hinted function, reads
    garbage:

    function show(array $a): void { foreach ($a as $k => $v) { echo $k, "=", $v, ","; } }
    $a = [10, 20, 30];
    krsort($a);
    show($a);   // PHP: 2=30,1=20,0=10   elephc: 0=10,1=0,2=1

    Reproduced on pristine main, so it is not from this change: the packed→hash promotion
    rewrites the local's storage without the checker seeing it, and the callee is specialized for
    a packed array. Worth its own issue.

  • SORT_REGULAR over integer keys mixed with non-numeric string keys can order differently from
    PHP. docs/php/arrays.md already documents this: PHP's own key comparison is not transitive
    there, so each side resolves the cycle through its own sort algorithm.

🤖 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:magician Touches eval, include execution, or elephc-magician. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. scope:multi-area Touches more compiler areas than the automatic area-label cap. size:l Large pull request. target:linux-x86_64 Contains behavior specific to the Linux x86_64 target. 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 outstanding correctness or repository-rule failures remain.

Summary

This PR adds PHP sort-flag support to ksort() and krsort() across compiled and eval execution.

  • Adds the standard SORT_* constants and optional $flags contracts.
  • Implements numeric, string, locale, natural, and case-folded key comparison modes.
  • Preserves stable sorting, packed-array promotion, named-argument evaluation order, and target-specific runtime behavior.
  • Adds extensive behavioral, emitter, parity, documentation, and example coverage.
Diagram
%%{init: {'theme': 'neutral'}}%%
flowchart LR
    A["ksort()/krsort() call"] --> B["Shared argument planning"]
    B --> C["Evaluate receiver and optional flags"]
    C --> D["Resolve flags once"]
    D -->|SORT_REGULAR| E["Existing regular key comparator"]
    D -->|Other supported mode| F["Flag-selected key comparator"]
    E --> G["Stable hash merge sort"]
    F --> G
    G --> H["Relink entries in place"]
Loading

Reviews (6) · Last reviewed commit: "feat(arrays): register SORT_ASC and SORT..."

Comment thread src/ir_lower/expr/array_builtin_args.rs Outdated
Comment thread src/ir_lower/expr/array_builtin_args.rs Outdated
Guikingone added a commit that referenced this pull request Sep 16, 2026
Review follow-up on #1059. AGENTS.md is explicit that `src/types/call_args/`
owns named matching, duplicate detection and spread expansion, and that EIR
lowering consumes `CallArgPlan` rather than rebuilding them. `plan_key_sort_args`
was rebuilding them.

It now asks `plan_call_args` and only re-reads the answer in SOURCE order, which
is the one thing the packed-to-hash promotion needs that a parameter-indexed plan
does not already say: `krsort(flags: f(), array: $a)` writes the flag first, so
the receiver cannot be promoted before `f()` has run.

Two shapes the plan reports and this declines, both to the shared path that owns
them: a spread, which has to be evaluated before anything can be said about which
element lands on the receiver slot, and a plan whose receiver slot is filled by a
spread element.

The planner answers in two shapes and both are handled: with no named argument it
returns a passthrough whose written order IS parameter order, and with one it
returns per-slot bindings carrying `source_index`. Reading only the second is what
made the first attempt silently stop promoting -- caught by the existing
`krsort` packed-array tests.

Verified byte-identical to PHP 8.5.10 across named arguments in both orders,
packed promotion with and without a flag, and the single evaluation of a
side-effecting flag expression. 47 key-sort tests pass.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Comment thread src/codegen_support/runtime/strings/strnatcmp.rs
@Guikingone Guikingone self-assigned this Sep 16, 2026
Guikingone added a commit that referenced this pull request Sep 16, 2026
Review follow-up on #1059. AGENTS.md is explicit that `src/types/call_args/`
owns named matching, duplicate detection and spread expansion, and that EIR
lowering consumes `CallArgPlan` rather than rebuilding them. `plan_key_sort_args`
was rebuilding them.

It now asks `plan_call_args` and only re-reads the answer in SOURCE order, which
is the one thing the packed-to-hash promotion needs that a parameter-indexed plan
does not already say: `krsort(flags: f(), array: $a)` writes the flag first, so
the receiver cannot be promoted before `f()` has run.

Two shapes the plan reports and this declines, both to the shared path that owns
them: a spread, which has to be evaluated before anything can be said about which
element lands on the receiver slot, and a plan whose receiver slot is filled by a
spread element.

The planner answers in two shapes and both are handled: with no named argument it
returns a passthrough whose written order IS parameter order, and with one it
returns per-slot bindings carrying `source_index`. Reading only the second is what
made the first attempt silently stop promoting -- caught by the existing
`krsort` packed-array tests.

Verified byte-identical to PHP 8.5.10 across named arguments in both orders,
packed promotion with and without a flag, and the single evaluation of a
side-effecting flag expression. 47 key-sort tests pass.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
@Guikingone
Guikingone force-pushed the fix/699-ksort-krsort-sort-flags branch from 22f51ee to 85089ea Compare September 16, 2026 20:52
Guikingone added a commit that referenced this pull request Sep 18, 2026
Review follow-up on #1059. AGENTS.md is explicit that `src/types/call_args/`
owns named matching, duplicate detection and spread expansion, and that EIR
lowering consumes `CallArgPlan` rather than rebuilding them. `plan_key_sort_args`
was rebuilding them.

It now asks `plan_call_args` and only re-reads the answer in SOURCE order, which
is the one thing the packed-to-hash promotion needs that a parameter-indexed plan
does not already say: `krsort(flags: f(), array: $a)` writes the flag first, so
the receiver cannot be promoted before `f()` has run.

Two shapes the plan reports and this declines, both to the shared path that owns
them: a spread, which has to be evaluated before anything can be said about which
element lands on the receiver slot, and a plan whose receiver slot is filled by a
spread element.

The planner answers in two shapes and both are handled: with no named argument it
returns a passthrough whose written order IS parameter order, and with one it
returns per-slot bindings carrying `source_index`. Reading only the second is what
made the first attempt silently stop promoting -- caught by the existing
`krsort` packed-array tests.

Verified byte-identical to PHP 8.5.10 across named arguments in both orders,
packed promotion with and without a flag, and the single evaluation of a
side-effecting flag expression. 47 key-sort tests pass.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
@Guikingone
Guikingone force-pushed the fix/699-ksort-krsort-sort-flags branch from 85089ea to 1e95e2d Compare September 18, 2026 13:10
    $a = ["img10" => 1, "img2" => 1];
    ksort($a, SORT_NATURAL);
    EIR backend error: ksort expected 1 args, got 2

Both key sorts were unary and always compared keys with `SORT_REGULAR`. The six
`SORT_*` constants PHP defines for them did not exist either, so the argument had
no spelling to pass.

    $sizes = [10 => "l", 9 => "m", 100 => "xl"];
    ksort($sizes, SORT_NUMERIC);   // 9, 10, 100
    ksort($sizes, SORT_STRING);    // 10, 100, 9

An integer key has no bytes of its own, so every byte-comparing mode has to spell
it out as decimal text first -- `zend_print_long_to_buf` in php-src, a 24-byte
buffer in the comparator's own frame here. That is the whole reason `100` lands
between `10` and `9`.

PHP resolves the comparator from `$flags & ~SORT_FLAG_CASE` and silently ignores
everything it does not recognize: `SORT_ASC`, `SORT_DESC`, a bare `SORT_FLAG_CASE`
and `999` all mean `SORT_REGULAR`. That resolution happens ONCE per call, in
`__rt_hash_key_sort_enter`, which folds a small selector into the mode word the
merge sort already carries. The comparison step then reads a number between 0 and
6 instead of reasoning about the raw flag word n log n times.

Selector `0` does not reach the new comparator at all: a flagless key sort still
runs `__rt_key_compare_regular`, untouched.

`SORT_STRING` and its case-folded twin are `__rt_strcmp` and `__rt_strcasecmp`,
the comparators `strcmp()` and `strcasecmp()` already use, so `ksort()` cannot
drift away from them. `SORT_NUMERIC` reads a string key through
`__rt_str_to_number`, which is PHP's numeric grammar rather than libc's -- no
hexadecimal, no `INF`/`NAN`, and an exponent only when a digit follows it, so
`"0x10"` is `0.0` and `"1e"` is `1.0`. Two integer keys compare exactly instead
of through a double, because `9223372036854775806` and `9223372036854775807`
round to the same `f64` and PHP orders them.

`SORT_NATURAL` is the one thing that had to be written: `__rt_strnatcmp`, a port
of php-src's `strnatcmp_ex` with its quirks intact. Leading zeros are skipped once
before the loop rather than per run, whitespace is skipped on each side
independently, a run starting with `0` on either side compares left-aligned
(`"a0.5"` before `"a0.10"`) and any other run compares by length first. Two
operands that differ only in skipped bytes compare EQUAL, which is why
`strnatcmp("a 7", "a7")` is `0`. It is a leaf with no calls: it runs once per
comparison inside an O(n log n) sort.

php-src walks one byte past the operand in its whitespace skip and relies on the
NUL its strings always carry. An elephc string is a pointer/length pair with no
terminator, so the skip synthesizes that `0` at the boundary instead of reading
whatever follows in memory.

`SORT_LOCALE_STRING` clips both operands at the first NUL and compares bytes. That
IS `strcoll` in the C locale, and the C locale is the only one an elephc program
can be in: there is no PHP-visible `setlocale()`. Calling libc would mean two heap
allocations per comparison to manufacture the NUL-terminated operands `strcoll`
wants, for the same answer.

`SORT_NATURAL | SORT_FLAG_CASE` folds ASCII `a`..`z` and nothing else. php-src
folds with libc `toupper()` there, which maps Latin-1 under Darwin's C locale and
not under glibc's -- PHP itself answers differently on macOS and Linux for a byte
above 127. Elephc gives glibc's answer on every target. ASCII keys are unaffected,
and a test pins the boundary by name.

Against host PHP 8.5.10, byte-identical: 210 sorts over 15 key sets and every mode
including `999`, `3`, `4` and a bare `SORT_FLAG_CASE`, in BOTH backends -- compiled
and `eval()`. The sets cover PHP whitespace, `0x10`/`INF`/`NAN`/`abc`, embedded
NULs, exponents and dangling exponents, leading zeros, fractional runs, negative
keys and both int64 extremes, keys past int64, single and empty arrays, bytes above
127, and integer keys mixed with string keys.

Through every receiver, since the sort relinks in place: a local, an object
property, a nested array cell, a by-reference parameter, named arguments in both
orders, and a packed array that `krsort()` promotes -- that promotion used to run
only when the call had exactly one argument. The flag expression is evaluated once,
in source order.

The x86_64 helpers were executed, not just assembled: `__rt_strnatcmp` and
`__rt_key_compare_flagged` were extracted from the emitted runtime, linked against
a C driver and run under Rosetta over the same PHP-derived cases. All three target
runtimes assemble clean.

`--heap-debug` reports `leak summary: clean` over 200 iterations of four modes: the
comparator's decimal buffer is in its own frame and nothing is allocated per
comparison.

The behavioural tests were confirmed load-bearing by forcing the resolver back to
`SORT_REGULAR`: 7 of the 12 fail, and the 5 that pass are the ones that do not
depend on the comparison (evaluation order, promotion, heap hygiene, and the two
that assert `SORT_REGULAR` behaviour on purpose).

`sort()`, `rsort()`, `asort()` and `arsort()` still take no flags; the comparison
machinery is reusable when they do. A non-numeric string in `$flags` is coerced to
`0` rather than raising PHP's `TypeError`, which is what elephc already does for
every integer builtin parameter (`str_repeat("z", "x")` answers `""`), not
something this change introduces.

Closes #699

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
Review follow-up on #1059. AGENTS.md is explicit that `src/types/call_args/`
owns named matching, duplicate detection and spread expansion, and that EIR
lowering consumes `CallArgPlan` rather than rebuilding them. `plan_key_sort_args`
was rebuilding them.

It now asks `plan_call_args` and only re-reads the answer in SOURCE order, which
is the one thing the packed-to-hash promotion needs that a parameter-indexed plan
does not already say: `krsort(flags: f(), array: $a)` writes the flag first, so
the receiver cannot be promoted before `f()` has run.

Two shapes the plan reports and this declines, both to the shared path that owns
them: a spread, which has to be evaluated before anything can be said about which
element lands on the receiver slot, and a plan whose receiver slot is filled by a
spread element.

The planner answers in two shapes and both are handled: with no named argument it
returns a passthrough whose written order IS parameter order, and with one it
returns per-slot bindings carrying `source_index`. Reading only the second is what
made the first attempt silently stop promoting -- caught by the existing
`krsort` packed-array tests.

Verified byte-identical to PHP 8.5.10 across named arguments in both orders,
packed promotion with and without a flag, and the single evaluation of a
side-effecting flag expression. 47 key-sort tests pass.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
…ment

The note said php-src folds Latin-1 "under macOS's C locale". Measured, that is not
where the divergence comes from: under LC_CTYPE=C, php-src folds ASCII only and prints
exactly what the fixture asserts. The CLI forces C.UTF-8 at startup, and it is Darwin's
single-byte table for THAT locale which also maps 0xE0..0xFE, so the same PHP source
orders keys above 0x7F one way on macOS and another on glibc.

Elephc folds a..z on every target, which is the LC_CTYPE=C answer. Folding through a
libc table would make an emitted binary answer for the machine that compiled it.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
… also defines

The flag resolvers and the docs both named `SORT_ASC` and `SORT_DESC` as words PHP
folds to `SORT_REGULAR`, but neither constant was in the catalog, so naming one was
a compile error:

    ksort($a, SORT_DESC);
    error[3:11]: Undefined constant: SORT_DESC

The fixture that covers the ignored words wrote them as the bare numbers `3` and
`4`, which is how it went unnoticed; it now names the constants, which is what a
PHP program would write. They belong to `array_multisort()` and still do not
reverse a key sort -- `[9][10][A][b]` either way, byte-identical to PHP 8.5.10.

Generated docs regenerated: standard constants 155 -> 163.

Claude-Session: https://claude.ai/code/session_01KSAAWPyNBq6dP2b5puN3wr
@Guikingone
Guikingone force-pushed the fix/699-ksort-krsort-sort-flags branch from 1e95e2d to 36f9370 Compare September 19, 2026 07:49
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:magician Touches eval, include execution, or elephc-magician. area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. scope:multi-area Touches more compiler areas than the automatic area-label cap. size:l Large pull request. target:linux-x86_64 Contains behavior specific to the Linux x86_64 target. type:feature Introduces new user-visible behavior or capabilities.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support PHP sort flags in ksort() and krsort()

1 participant