Repository navigation
Conversation
GromNaN
force-pushed
the
fix/array-merge-empty-array-result
branch
from
October 18, 2026 10:26
47ea5d5 to
eb27320
Compare
GromNaN
marked this pull request as ready for review
October 18, 2026 11:58
When one of the two arguments of array_merge() is empty, the other one is returned as is instead of being copied. The returned array is the argument itself, shared through copy-on-write, so it also kept the next free index of the argument. array_merge() renumbers integer keys from zero, so the next append to the result must use the element count. An argument whose next free index was stale, for example after unset() of its last element, made $result[] = $value use a wrong key. The shortcut now applies only when the argument already behaves like the result of the general code path: a packed array without holes, or an array with string keys only, whose next free index is the one the general path would produce. Everything else goes through the general path, which builds a fresh array. Clean arrays are still returned as is, so the copy is still avoided. array_merge_recursive() shares the same code path and had the same bug.
GromNaN
force-pushed
the
fix/array-merge-empty-array-result
branch
from
October 18, 2026 12:49
eb27320 to
f7b26a5
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
array_merge()returns the non-empty argument as is when the other argument is empty. The returned array is the argument itself, shared through copy-on-write, so it also kept the next free index of the argument, whicharray_merge()is expected to reset because integer keys are renumbered from 0. The general code path does exactly that, and the shortcut is an optimization, so it is not supposed to be observable.array_values()had the same shortcut and got the same guard in cc96166 ("Don't optimize input arrays with suffix holes", 2017), for the same reason, in the same file, without an RFC.array_merge()just never got it.Today the same call site returns a differently shaped array depending on whether a variable happens to be empty at runtime:
The shortcut now applies only when the argument already behaves like the result of the general path: a packed array without holes, or an array with string keys only, whose next free index is the one the general path would produce. Everything else goes through the general path, which builds a fresh array. Clean arrays are still returned as is, so the copy is still avoided.
array_merge_recursive()shares the code path and gets the same fix.A regression test is added. Output before and after the change:
array(3) { [0]=> int(1) [1]=> int(2) - [3]=> + [2]=> int(4) } array(2) { ["a"]=> int(1) - [1]=> + [0]=> int(3) }