Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Negative indices on empty array don't affect next chosen index #11154

Closed
ColinHDev opened this issue Apr 28, 2023 · 2 comments
Closed

Negative indices on empty array don't affect next chosen index #11154

ColinHDev opened this issue Apr 28, 2023 · 2 comments

Comments

@ColinHDev
Copy link
Contributor

Description

I was browsing through php's source code to look at how arrays work and noticed the following:

This code:

<?php
$a = array();
$a[-5] = "-5";
$a[] = "after -5";

behaves differently than this code:

<?php
$b = array(
  -5 => "-5"
);
$b[] = "after -5";

Resulted in this output:

array(2) {
  [-5]=>
  string(2) "-5"
  [0]=>
  string(8) "after -5"
}
array(2) {
  [-5]=>
  string(2) "-5"
  [-4]=>
  string(8) "after -5"
}

But I expected this output instead:

array(2) {
  [-5]=>
  string(2) "-5"
  [-4]=>
  string(8) "after -5"
}
array(2) {
  [-5]=>
  string(2) "-5"
  [-4]=>
  string(8) "after -5"
}

https://3v4l.org/MjbOm

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!

PHP Version

PHP 8.2.3

Operating System

No response

@iluuu1994
Copy link
Member

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.

@ColinHDev
Copy link
Contributor Author

Alright, thank you very much!
I will look into submitting a PR when I'm home.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants