php-ds / ext-ds

An extension providing efficient data structures for PHP 7
https://medium.com/p/9dda7af674cd
MIT License
2.11k stars 95 forks source link

Passing array item by reference breaks \DS\Set behaviour #193

Closed Feolius closed 11 months ago

Feolius commented 1 year ago

php -v: PHP 8.1.12 (cli) (built: Oct 28 2022 19:11:07) (NTS) ds version: 1.4.0

Initial problematic codeblock looks like this.

$data = [
    [
        'id' => 1,
        'data' => 'test',
    ],
    [
         'id' => 2,
         'data' => 'test',
    ]
];
foreach ($data as &$item) {
    unset($item['id']);
}
$s = new \Ds\Set($data);

Please pay attention on foreach cycle where we are passing array item by reference to get rid of unique id field. After that I expect the following code

$res = $s->toArray();
var_dump($res);

prints me an array with single item ['data' => 'test']. However instead I have the following output:

array(2) {
  [0]=>
  &array(1) {
    ["data"]=>
    string(4) "test"
  }
  [1]=>
  &array(1) {
    ["data"]=>
    string(4) "test"
  }
}

So, we get an array of references which are indeed different. But this is not what I would expect to see. Then I decided to slightly change initial code:

$data = [
    [
        'data' => 'test',
    ],
    [
        'data' => 'test',
    ]
];
foreach ($data as &$item) {
    // Do nothing
}
$s = new \Ds\Set($data);

And result is still the same. But removing '&' from cycle actually makes it work correctly.

It's worth noting that polyfill works correctly in all cases. So, that leads me here for behaviour explanation. I know that passing array element by reference can be potentially dangerous in case of further normal array assignment (as described here https://www.php.net/manual/en/language.references.whatdo.php). However it doesn't make sense how it can be related to these changes in a Set behaviour and why polyfill behaviour is different from extension implementation.

rtheunissen commented 11 months ago

Almost a year later, I'm finally coming back to this project and will take a look at this bug. 👀