[PHP-DEV] [RFC] Change behaviour of array sort functions to return a copy of the sorted array

On 21/10/2024 14:20, Gina P. Banyard wrote:

This RFC does*NOT* change the by-ref parameter passing*NOR* the in-place sorting of these functions.
I don't see why people are thinking this would be changed, as I only ever talk about the return value.
But I added a sentence in the Unaffected PHP Functionality section.

I think maybe people are saying that the RFC *should* aim to do that, in some form.

I absolutely agree that the current design is unfortunate; but I worry that bugs will be introduced by people not realising that the function both modifies and returns its argument.

Take the classic example of why DateTimeImmutable exists:

$now = new DateTime;
$tomorrow = $now->modify('+1 day');
// $tomorrow has the expected value, but $now does not

And then compare to the proposed behaviour of sort():

$names_as_entered = explode(',', $_POST['names']);
$names_sorted = sort($names_as_entered);
// oops, we just lost our original order

If someone wants to write that code (i.e. make a sorted copy), they'd need to force a clone on the array somehow, e.g.

$names_as_entered = explode(',', $_POST['names']);
$names_sorted = sort(iterator_to_array($names_as_entered));

Which is probably less clear than the existing version:

$names_as_entered = explode(',', $_POST['names']);
$names_sorted = $names_as_entered;
sort($names_sorted);

The same bug can easily hide inside less obviously imperative code. This would remove the highest 10 numbers from a list, leaving others untouched:

$result = array_filter(
array: $data,
callback: fn($item) => !in_array($item, array_slice(rsort($data, SORT_NUMERIC), 0, 10))
);

This looks like a safe refactor for performance or readability, but would lose the original keys and order:

$top_10 = array_slice(rsort($data, SORT_NUMERIC), 0, 10);
$result = array_filter(
array: $data,
callback: fn($item) => !in_array($item, $top_10)
);

[Here's a demo: Online PHP editor | output for 9Nieo For anyone confused by this example, the first version works because the fn() closure captures $data by-value, effectively cloning it.]

Regards,

--
Rowan Tommins
[IMSoP]

On 2024-10-22 00:17, mickmackusa wrote:

On Mon, 21 Oct 2024, 18:09 Morgan, <weedpacket@varteg.nz <mailto:weedpacket@varteg.nz>> wrote:

    You can’t use:
      $sorted_datasets = array_map(sort(...), $datasets);
    You want
      $sorted_datasets = $datasets;
      array_walk($sorted_datasets, sort(...));

A warning: no one should ever use array_walk($sorted_datasets, sort(...)); as general-use script.

When sorting a 2d array in this fashion (only non-fatally executed with numeric first level keys Online PHP editor | output for HaU42 <https://3v4l.org/ HaU42>), the first level keys will be used as the sorting flag while sorting each row. This means that different rows may have different sorting flags applied -- effectively corrupting the result. Online PHP editor | output for FeIpj -- notice how rows with keys 2, 5, and 10 are sorted as strings.

Mick

Right, but the issue was that array_walk() is the only callback-parameterised array_* function which is even _compatible_ with sort(). As you point out, even then it's problematic.

On 20.10.2024 at 19:42, Gina P. Banyard wrote:

I would like to propose a short RFC to make the return value of the sort() and similar functions more useful:
PHP: rfc:array-sort-return-array

After having thought a bit about
<Issues · php/php-src · GitHub, I don't think it makes
much sense to stick a band-aid on our sorting functions to make them
more functional. Instead we should better introduce proper functional
functions which do not modify the given array. Less efficient, but that
often may not matter in practise.

Christoph

On Sunday, 20 October 2024 at 18:42, Gina P. Banyard <internals@gpb.moe> wrote:

Hello internals,

I would like to propose a short RFC to make the return value of the sort() and similar functions more useful:
PHP: rfc:array-sort-return-array

I intend for the discussion to last 2 weeks and then open the vote.

Hello internals,

I have decided to withdraw this RFC.
The primary way for this to have been usable would have been to use the engine level "@prefer-ref" parameter "attribute".
However, this is rarely used and adds some minor complexity to the engine that mainly exists to support less than ideal API/functions or other limitations currently with PHP.
And it is probably wise not to introduce more usages of said attribute.

The alternative, that I would support but don't intend to do, is to add a new set of `sorted()` functions.

Best regards,

Gina P. Banyard