You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Negative indices on empty array don't affect next chosen index #11154
I am not sure, if this is a bug or intended, so I wanted to create an issue regarding that.
The PR #3772 changed that appending to arrays would also work if the previous element has a negative index without defaulting to 0 in that case.
It did so by setting the default value for nNextFreeElement from 0 to ZEND_LONG_MIN, while also adding the following "guard clause" to the _zend_hash_index_add_or_update_i( ) function, which makes sure that the chosen index defaults to 0 if it's still at ZEND_LONG_MIN and the element should be put at the next index. https://github.com/php/php-src/blob/master/Zend/zend_hash.c#L1027-L1029.
But it did not change that for the _zend_empty_array constant: https://github.com/php/php-src/blob/master/Zend/zend_hash.c#L258. Because of that, the following if statement is always false for empty arrays since (any_negative_key >= 0) == false: https://github.com/php/php-src/blob/master/Zend/zend_hash.c#L1110-L1112.
My question is, is this intended? Because it looks like a bug to me, especially since that "guard clause" would ensure that running $a[] = "" on an empty array, would still use index 0.
But I am not familiar with the PHP internals, so I might have completely missed something. So if there is a reason for this behaviour or why _zend_empty_array's nNextFreeElement can't start at ZEND_LONG_MIN, I would be happy to know why!
Thank you for your analysis! This does indeed sound like a bug, as it doesn't behave as described in https://wiki.php.net/rfc/negative_array_index. Your suggestion to adjust .nNextFreeElement = ZEND_LONG_MIN of _zend_empty_array sounds correct.
Unfortunately, this behavior is long-standing and I'm afraid of BC breaks for currently supported PHP versions. Targetting master with a note in UPGRADING should be ok.
Activity
iluuu1994 commentedon Apr 28, 2023
Thank you for your analysis! This does indeed sound like a bug, as it doesn't behave as described in https://wiki.php.net/rfc/negative_array_index. Your suggestion to adjust
.nNextFreeElement = ZEND_LONG_MINof_zend_empty_arraysounds correct.Unfortunately, this behavior is long-standing and I'm afraid of BC breaks for currently supported PHP versions. Targetting
masterwith a note inUPGRADINGshould be ok.ColinHDev commentedon Apr 28, 2023
Alright, thank you very much!
I will look into submitting a PR when I'm home.
Fix negative indices on empty array not affecting next chosen index