Fix SORT_REGULAR with new transitive comparison functions #20517
Conversation
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
Introduces localized transitive comparators (including array/object recursion and HashTable helpers) so SORT_REGULAR sorting and array_unique() can enforce consistent ordering without touching zend_compare(). The sort entry points now detect when mixed numeric string semantics are needed and delegate to php_array_sort_regular(), while array_unique() switches to the same comparator when required. Fixes: phpGH-20262
Implement scalar/string fast paths and enum/object ordering rules so both regular and transitive SORT_REGULAR comparators skip redundant zend_compare() calls. Introduce php_array_compare_non_numeric_strings() and dedicated transitive string/number helpers to minimize numeric parsing.
Avoid temporary zend_string allocations in `compare_long_to_string()` and `compare_double_to_string()` by using stack buffers, and streamline `zendi_smart_strcmp()` with non-numeric short-circuits plus single-pass numeric parsing to cut redundant work in general comparisons.
- Introduce the transitive-aware compare_long/double/string helpers in Zend. - Switch array sorts and array_unique to use them and adjust php_hash_values_need_transitivity().
This was referenced Nov 18, 2025
Girgias
requested changes
Nov 18, 2025
Member
Girgias
left a comment
There was a problem hiding this comment.
Various comments and questions and this needs a rebase as I refactored the sorting code to remove a bunch of duplication.
Author
|
@Girgias thank you for taking the time to provide the careful review! Looks like I was able to capture your sorting code refactor when I created this new branch. I'll push a fresh commit what I addressed in your code comments. Thanks again for the help! |
- Call zend_compare_symbol_tables() for array pairs and keep the enum-specific path without the ZEND_UNCOMPARABLE check. - Point the regular key/data variants directly at their impls via DEFINE_SORT_VARIANTS_USING(), retaining php_array_hash_compare_transitive() as the required compare_func_t adapter. - Make php_hash_has_string_keys() const-qualified and keep php_array_compare_transitive() for nested arrays/objects/enums while scalar fast paths stay in the main comparator.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Summary
Fixes #20262 by making
SORT_REGULARfall back to a fully transitive comparator whenever loose comparisons would otherwise be non-transitive (e.g. numeric strings vs numeric zvals). That keeps duplicates grouped soarray_unique()and the sort family behave consistently.Highlights
php_array_sort_regular()fast path so only the arrays that need this comparator pay for it.array_unique()to call the fast path automatically when required.sort()andarray_unique()