From e12b9df05d238ec0f3cf47b28a49a4df1b1e3442 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 4 Mar 2020 11:52:55 +0100 Subject: [PATCH] Make sorting stable Make user-exposed sorts stable, by storing the position of elements in the original array, and using those positions as a fallback comparison criterion. The base sort is still hybrid q/insert. The use of true/false comparison functions is deprecated (but still supported) and should be replaced by -1/0/1 comparison functions, driven by the <=> operator. RFC: https://wiki.php.net/rfc/stable_sorting Closes GH-5236. --- UPGRADING | 16 +- Zend/zend_hash.c | 32 +- ext/spl/tests/bug67539.phpt | 2 +- ext/standard/array.c | 435 ++++++++++-------- ext/standard/php_array.h | 1 + .../array/array_multisort_stability.phpt | 15 + ext/standard/tests/array/asort_stability.phpt | 12 + ext/standard/tests/array/bug71334.phpt | 2 +- .../tests/array/natcasesort_variation9.phpt | Bin 1609 -> 1609 bytes .../tests/array/rsort_variation11.phpt | Bin 2534 -> 2534 bytes .../tests/array/sort_variation11.phpt | Bin 2891 -> 2891 bytes ext/standard/tests/array/usort_stability.phpt | 15 + .../tests/array/usort_variation11.phpt | 68 ++- 13 files changed, 393 insertions(+), 205 deletions(-) create mode 100644 ext/standard/tests/array/array_multisort_stability.phpt create mode 100644 ext/standard/tests/array/asort_stability.phpt create mode 100644 ext/standard/tests/array/usort_stability.phpt diff --git a/UPGRADING b/UPGRADING index baad2ce1f88..cf96463b49a 100644 --- a/UPGRADING +++ b/UPGRADING @@ -437,9 +437,19 @@ PHP 8.0 UPGRADE NOTES respect the inherited locale without an explicit setlocale() call. An explicit setlocale() call is now always required if you wish to change any locale component from the default. - . Remove deprecated DES fallback in crypt(). If an unknown salt format is + . Removed deprecated DES fallback in crypt(). If an unknown salt format is passed to crypt(), the function will fail with *0 instead of falling back to a weak DES hash now. + . The result of sorting functions may have changed, if the array contains + equal-comparing elements. + . Sort comparison functions return true/false will now throw a deprecation + warning and should be replaced with an implementation return an integer + smaller than, equal to, or greater than zero. + + // Replace + usort($array, fn($a, $b) => $a > $b); + // With + usort($array, fn($a, $b) => $a <=> $b); - Sysvmsg: . msg_get_queue() will now return an SysvMessageQueue object rather than a @@ -584,6 +594,10 @@ PHP 8.0 UPGRADE NOTES $proc = proc_open($command, [['pty'], ['pty'], ['pty']], $pipes); + . Sorting functions are now stable, which means that equal-comparing elements + will retain their original order. + RFC: https://wiki.php.net/rfc/stable_sorting + - Zip: . Extension updated to version 1.19.0 . New ZipArchive::lastId property to get index value of last added entry. diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index 125fc2457b2..1723833b1fa 100644 --- a/Zend/zend_hash.c +++ b/Zend/zend_hash.c @@ -2453,15 +2453,15 @@ ZEND_API void zend_hash_bucket_swap(Bucket *p, Bucket *q) zend_ulong h; zend_string *key; - ZVAL_COPY_VALUE(&val, &p->val); + val = p->val; h = p->h; key = p->key; - ZVAL_COPY_VALUE(&p->val, &q->val); + p->val = q->val; p->h = q->h; p->key = q->key; - ZVAL_COPY_VALUE(&q->val, &val); + q->val = val; q->h = h; q->key = key; } @@ -2470,9 +2470,9 @@ ZEND_API void zend_hash_bucket_renum_swap(Bucket *p, Bucket *q) { zval val; - ZVAL_COPY_VALUE(&val, &p->val); - ZVAL_COPY_VALUE(&p->val, &q->val); - ZVAL_COPY_VALUE(&q->val, &val); + val = p->val; + p->val = q->val; + q->val = val; } ZEND_API void zend_hash_bucket_packed_swap(Bucket *p, Bucket *q) @@ -2480,13 +2480,13 @@ ZEND_API void zend_hash_bucket_packed_swap(Bucket *p, Bucket *q) zval val; zend_ulong h; - ZVAL_COPY_VALUE(&val, &p->val); + val = p->val; h = p->h; - ZVAL_COPY_VALUE(&p->val, &q->val); + p->val = q->val; p->h = q->h; - ZVAL_COPY_VALUE(&q->val, &val); + q->val = val; q->h = h; } @@ -2498,28 +2498,34 @@ ZEND_API void ZEND_FASTCALL zend_hash_sort_ex(HashTable *ht, sort_func_t sort, b IS_CONSISTENT(ht); HT_ASSERT_RC1(ht); - if (!(ht->nNumOfElements>1) && !(renumber && ht->nNumOfElements>0)) { /* Doesn't require sorting */ + if (!(ht->nNumOfElements>1) && !(renumber && ht->nNumOfElements>0)) { + /* Doesn't require sorting */ return; } if (HT_IS_WITHOUT_HOLES(ht)) { - i = ht->nNumUsed; + /* Store original order of elements in extra space to allow stable sorting. */ + for (i = 0; i < ht->nNumUsed; i++) { + Z_EXTRA(ht->arData[i].val) = i; + } } else { + /* Remove holes and store original order. */ for (j = 0, i = 0; j < ht->nNumUsed; j++) { p = ht->arData + j; if (UNEXPECTED(Z_TYPE(p->val) == IS_UNDEF)) continue; if (i != j) { ht->arData[i] = *p; } + Z_EXTRA(ht->arData[i].val) = i; i++; } + ht->nNumUsed = i; } - sort((void *)ht->arData, i, sizeof(Bucket), (compare_func_t) compar, + sort((void *)ht->arData, ht->nNumUsed, sizeof(Bucket), (compare_func_t) compar, (swap_func_t)(renumber? zend_hash_bucket_renum_swap : ((HT_FLAGS(ht) & HASH_FLAG_PACKED) ? zend_hash_bucket_packed_swap : zend_hash_bucket_swap))); - ht->nNumUsed = i; ht->nInternalPointer = 0; if (renumber) { diff --git a/ext/spl/tests/bug67539.phpt b/ext/spl/tests/bug67539.phpt index 8bab2a8c217..1001112e127 100644 --- a/ext/spl/tests/bug67539.phpt +++ b/ext/spl/tests/bug67539.phpt @@ -7,7 +7,7 @@ $it = new ArrayIterator(array_fill(0,2,'X'), 1 ); function badsort($a, $b) { $GLOBALS['it']->unserialize($GLOBALS['it']->serialize()); - return TRUE; + return 0; } $it->uksort('badsort'); diff --git a/ext/standard/array.c b/ext/standard/array.c index 0a8484f9996..9b0efaf301f 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -132,7 +132,40 @@ PHP_MSHUTDOWN_FUNCTION(array) /* {{{ */ } /* }}} */ -static int php_array_key_compare(Bucket *f, Bucket *s) /* {{{ */ +static zend_never_inline ZEND_COLD int stable_sort_fallback(Bucket *a, Bucket *b) { + if (Z_EXTRA(a->val) > Z_EXTRA(b->val)) { + return 1; + } else if (Z_EXTRA(a->val) < Z_EXTRA(b->val)) { + return -1; + } else { + return 0; + } +} + +#define RETURN_STABLE_SORT(a, b, result) do { \ + int _result = (result); \ + if (EXPECTED(_result)) { \ + return _result; \ + } \ + return stable_sort_fallback((a), (b)); \ +} while (0) + +/* Generate inlined unstable and stable variants, and non-inlined reversed variants. */ +#define DEFINE_SORT_VARIANTS(name) \ + static zend_never_inline int php_array_##name##_unstable(Bucket *a, Bucket *b) { \ + return php_array_##name##_unstable_i(a, b); \ + } \ + static zend_never_inline int php_array_##name(Bucket *a, Bucket *b) { \ + RETURN_STABLE_SORT(a, b, php_array_##name##_unstable_i(a, b)); \ + } \ + static zend_never_inline int php_array_reverse_##name##_unstable(Bucket *a, Bucket *b) { \ + return php_array_##name##_unstable(a, b) * -1; \ + } \ + static zend_never_inline int php_array_reverse_##name(Bucket *a, Bucket *b) { \ + RETURN_STABLE_SORT(a, b, php_array_reverse_##name##_unstable(a, b)); \ + } \ + +static zend_always_inline int php_array_key_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { zend_uchar t; zend_long l1, l2; @@ -171,13 +204,7 @@ static int php_array_key_compare(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_key_compare(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_key_compare(b, a); -} -/* }}} */ - -static int php_array_key_compare_numeric(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_key_compare_numeric_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { if (f->key == NULL && s->key == NULL) { return (zend_long)f->h > (zend_long)s->h ? 1 : -1; @@ -198,13 +225,7 @@ static int php_array_key_compare_numeric(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_key_compare_numeric(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_key_compare_numeric(b, a); -} -/* }}} */ - -static int php_array_key_compare_string_case(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_key_compare_string_case_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { const char *s1, *s2; size_t l1, l2; @@ -229,13 +250,7 @@ static int php_array_key_compare_string_case(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_key_compare_string_case(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_key_compare_string_case(b, a); -} -/* }}} */ - -static int php_array_key_compare_string(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_key_compare_string_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { const char *s1, *s2; size_t l1, l2; @@ -260,12 +275,6 @@ static int php_array_key_compare_string(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_key_compare_string(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_key_compare_string(b, a); -} -/* }}} */ - static int php_array_key_compare_string_natural_general(Bucket *f, Bucket *s, int fold_case) /* {{{ */ { const char *s1, *s2; @@ -293,29 +302,29 @@ static int php_array_key_compare_string_natural_general(Bucket *f, Bucket *s, in static int php_array_key_compare_string_natural_case(Bucket *a, Bucket *b) /* {{{ */ { - return php_array_key_compare_string_natural_general(a, b, 1); + RETURN_STABLE_SORT(a, b, php_array_key_compare_string_natural_general(a, b, 1)); } /* }}} */ static int php_array_reverse_key_compare_string_natural_case(Bucket *a, Bucket *b) /* {{{ */ { - return php_array_key_compare_string_natural_general(b, a, 1); + RETURN_STABLE_SORT(a, b, php_array_key_compare_string_natural_general(b, a, 1)); } /* }}} */ static int php_array_key_compare_string_natural(Bucket *a, Bucket *b) /* {{{ */ { - return php_array_key_compare_string_natural_general(a, b, 0); + RETURN_STABLE_SORT(a, b, php_array_key_compare_string_natural_general(a, b, 0)); } /* }}} */ static int php_array_reverse_key_compare_string_natural(Bucket *a, Bucket *b) /* {{{ */ { - return php_array_key_compare_string_natural_general(b, a, 0); + RETURN_STABLE_SORT(a, b, php_array_key_compare_string_natural_general(b, a, 0)); } /* }}} */ -static int php_array_key_compare_string_locale(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_key_compare_string_locale_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { const char *s1, *s2; char buf1[MAX_LENGTH_OF_LONG + 1]; @@ -335,13 +344,7 @@ static int php_array_key_compare_string_locale(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_key_compare_string_locale(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_key_compare_string_locale(b, a); -} -/* }}} */ - -static int php_array_data_compare(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_data_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -356,13 +359,7 @@ static int php_array_data_compare(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_data_compare(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_data_compare(a, b) * -1; -} -/* }}} */ - -static int php_array_data_compare_numeric(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_data_compare_numeric_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -378,13 +375,7 @@ static int php_array_data_compare_numeric(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_data_compare_numeric(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_data_compare_numeric(b, a); -} -/* }}} */ - -static int php_array_data_compare_string_case(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_data_compare_string_case_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -400,13 +391,7 @@ static int php_array_data_compare_string_case(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_data_compare_string_case(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_data_compare_string_case(b, a); -} -/* }}} */ - -static int php_array_data_compare_string(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_data_compare_string_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -422,12 +407,6 @@ static int php_array_data_compare_string(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_data_compare_string(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_data_compare_string(b, a); -} -/* }}} */ - static int php_array_natural_general_compare(Bucket *f, Bucket *s, int fold_case) /* {{{ */ { zend_string *tmp_str1, *tmp_str2; @@ -442,31 +421,19 @@ static int php_array_natural_general_compare(Bucket *f, Bucket *s, int fold_case } /* }}} */ -static int php_array_natural_compare(Bucket *a, Bucket *b) /* {{{ */ +static zend_always_inline int php_array_natural_compare_unstable_i(Bucket *a, Bucket *b) /* {{{ */ { return php_array_natural_general_compare(a, b, 0); } /* }}} */ -static int php_array_reverse_natural_compare(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_natural_general_compare(b, a, 0); -} -/* }}} */ - -static int php_array_natural_case_compare(Bucket *a, Bucket *b) /* {{{ */ +static zend_always_inline int php_array_natural_case_compare_unstable_i(Bucket *a, Bucket *b) /* {{{ */ { return php_array_natural_general_compare(a, b, 1); } /* }}} */ -static int php_array_reverse_natural_case_compare(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_natural_general_compare(b, a, 1); -} -/* }}} */ - -static int php_array_data_compare_string_locale(Bucket *f, Bucket *s) /* {{{ */ +static int php_array_data_compare_string_locale_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -482,11 +449,18 @@ static int php_array_data_compare_string_locale(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_reverse_data_compare_string_locale(Bucket *a, Bucket *b) /* {{{ */ -{ - return php_array_data_compare_string_locale(b, a); -} -/* }}} */ +DEFINE_SORT_VARIANTS(key_compare); +DEFINE_SORT_VARIANTS(key_compare_numeric); +DEFINE_SORT_VARIANTS(key_compare_string_case); +DEFINE_SORT_VARIANTS(key_compare_string); +DEFINE_SORT_VARIANTS(key_compare_string_locale); +DEFINE_SORT_VARIANTS(data_compare); +DEFINE_SORT_VARIANTS(data_compare_numeric); +DEFINE_SORT_VARIANTS(data_compare_string_case); +DEFINE_SORT_VARIANTS(data_compare_string); +DEFINE_SORT_VARIANTS(data_compare_string_locale); +DEFINE_SORT_VARIANTS(natural_compare); +DEFINE_SORT_VARIANTS(natural_case_compare); static bucket_compare_func_t php_get_key_compare_func(zend_long sort_type, int reverse) /* {{{ */ { @@ -616,6 +590,70 @@ static bucket_compare_func_t php_get_data_compare_func(zend_long sort_type, int } /* }}} */ +static bucket_compare_func_t php_get_data_compare_func_unstable(zend_long sort_type, int reverse) /* {{{ */ +{ + switch (sort_type & ~PHP_SORT_FLAG_CASE) { + case PHP_SORT_NUMERIC: + if (reverse) { + return php_array_reverse_data_compare_numeric_unstable; + } else { + return php_array_data_compare_numeric_unstable; + } + break; + + case PHP_SORT_STRING: + if (sort_type & PHP_SORT_FLAG_CASE) { + if (reverse) { + return php_array_reverse_data_compare_string_case_unstable; + } else { + return php_array_data_compare_string_case_unstable; + } + } else { + if (reverse) { + return php_array_reverse_data_compare_string_unstable; + } else { + return php_array_data_compare_string_unstable; + } + } + break; + + case PHP_SORT_NATURAL: + if (sort_type & PHP_SORT_FLAG_CASE) { + if (reverse) { + return php_array_reverse_natural_case_compare_unstable; + } else { + return php_array_natural_case_compare_unstable; + } + } else { + if (reverse) { + return php_array_reverse_natural_compare_unstable; + } else { + return php_array_natural_compare_unstable; + } + } + break; + + case PHP_SORT_LOCALE_STRING: + if (reverse) { + return php_array_reverse_data_compare_string_locale_unstable; + } else { + return php_array_data_compare_string_locale_unstable; + } + break; + + case PHP_SORT_REGULAR: + default: + if (reverse) { + return php_array_reverse_data_compare_unstable; + } else { + return php_array_data_compare_unstable; + } + break; + } + return NULL; +} +/* }}} */ + /* {{{ proto bool krsort(array &array_arg [, int sort_flags]) Sort an array by key value in reverse order */ PHP_FUNCTION(krsort) @@ -881,10 +919,11 @@ PHP_FUNCTION(rsort) } /* }}} */ -static int php_array_user_compare(Bucket *f, Bucket *s) /* {{{ */ +static inline int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval args[2]; zval retval; + zend_bool call_failed; ZVAL_COPY(&args[0], &f->val); ZVAL_COPY(&args[1], &s->val); @@ -893,17 +932,47 @@ static int php_array_user_compare(Bucket *f, Bucket *s) /* {{{ */ BG(user_compare_fci).params = args; BG(user_compare_fci).retval = &retval; BG(user_compare_fci).no_separation = 0; - if (zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) { - zend_long ret = zval_get_long(&retval); - zval_ptr_dtor(&retval); - zval_ptr_dtor(&args[1]); - zval_ptr_dtor(&args[0]); - return ZEND_NORMALIZE_BOOL(ret); - } else { - zval_ptr_dtor(&args[1]); - zval_ptr_dtor(&args[0]); + call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF; + zval_ptr_dtor(&args[1]); + zval_ptr_dtor(&args[0]); + if (UNEXPECTED(call_failed)) { return 0; } + + if (UNEXPECTED(Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE)) { + if (!ARRAYG(compare_deprecation_thrown)) { + php_error_docref(NULL, E_DEPRECATED, + "Returning bool from comparison function is deprecated, " + "return an integer less than, equal to, or greater than zero"); + ARRAYG(compare_deprecation_thrown) = 1; + } + + if (Z_TYPE(retval) == IS_FALSE) { + /* Retry with swapped operands. */ + ZVAL_COPY(&args[0], &s->val); + ZVAL_COPY(&args[1], &f->val); + call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF; + zval_ptr_dtor(&args[1]); + zval_ptr_dtor(&args[0]); + if (call_failed) { + return 0; + } + + zend_long ret = zval_get_long(&retval); + zval_ptr_dtor(&retval); + return -ZEND_NORMALIZE_BOOL(ret); + } + } + + zend_long ret = zval_get_long(&retval); + zval_ptr_dtor(&retval); + return ZEND_NORMALIZE_BOOL(ret); +} +/* }}} */ + +static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */ +{ + RETURN_STABLE_SORT(a, b, php_array_user_compare_unstable(a, b)); } /* }}} */ @@ -930,6 +999,7 @@ static int php_array_user_compare(Bucket *f, Bucket *s) /* {{{ */ #define PHP_ARRAY_CMP_FUNC_BACKUP() \ old_user_compare_fci = BG(user_compare_fci); \ old_user_compare_fci_cache = BG(user_compare_fci_cache); \ + ARRAYG(compare_deprecation_thrown) = 0; \ BG(user_compare_fci_cache) = empty_fcall_info_cache; \ #define PHP_ARRAY_CMP_FUNC_RESTORE() \ @@ -985,11 +1055,11 @@ PHP_FUNCTION(uasort) } /* }}} */ -static int php_array_user_key_compare(Bucket *f, Bucket *s) /* {{{ */ +static inline int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval args[2]; zval retval; - zend_long result; + zend_bool call_failed; if (f->key == NULL) { ZVAL_LONG(&args[0], f->h); @@ -1006,20 +1076,59 @@ static int php_array_user_key_compare(Bucket *f, Bucket *s) /* {{{ */ BG(user_compare_fci).params = args; BG(user_compare_fci).retval = &retval; BG(user_compare_fci).no_separation = 0; - if (zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == SUCCESS && Z_TYPE(retval) != IS_UNDEF) { - result = zval_get_long(&retval); - zval_ptr_dtor(&retval); - } else { - result = 0; + call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF; + zval_ptr_dtor(&args[1]); + zval_ptr_dtor(&args[0]); + if (UNEXPECTED(call_failed)) { + return 0; } - zval_ptr_dtor(&args[0]); - zval_ptr_dtor(&args[1]); + if (UNEXPECTED(Z_TYPE(retval) == IS_FALSE || Z_TYPE(retval) == IS_TRUE)) { + if (!ARRAYG(compare_deprecation_thrown)) { + php_error_docref(NULL, E_DEPRECATED, + "Returning bool from comparison function is deprecated, " + "return an integer less than, equal to, or greater than zero"); + ARRAYG(compare_deprecation_thrown) = 1; + } + if (Z_TYPE(retval) == IS_FALSE) { + /* Retry with swapped operands. */ + if (s->key == NULL) { + ZVAL_LONG(&args[0], s->h); + } else { + ZVAL_STR_COPY(&args[0], s->key); + } + if (f->key == NULL) { + ZVAL_LONG(&args[1], f->h); + } else { + ZVAL_STR_COPY(&args[1], f->key); + } + + call_failed = zend_call_function(&BG(user_compare_fci), &BG(user_compare_fci_cache)) == FAILURE || Z_TYPE(retval) == IS_UNDEF; + zval_ptr_dtor(&args[1]); + zval_ptr_dtor(&args[0]); + if (call_failed) { + return 0; + } + + zend_long ret = zval_get_long(&retval); + zval_ptr_dtor(&retval); + return -ZEND_NORMALIZE_BOOL(ret); + } + } + + zend_long result = zval_get_long(&retval); + zval_ptr_dtor(&retval); return ZEND_NORMALIZE_BOOL(result); } /* }}} */ +static int php_array_user_key_compare(Bucket *a, Bucket *b) /* {{{ */ +{ + RETURN_STABLE_SORT(a, b, php_array_user_key_compare_unstable(a, b)); +} +/* }}} */ + /* {{{ proto bool uksort(array array_arg, string cmp_function) Sort an array by keys using a user-defined comparison function */ PHP_FUNCTION(uksort) @@ -4397,31 +4506,12 @@ PHP_FUNCTION(array_change_key_case) } /* }}} */ -struct bucketindex { - Bucket b; - unsigned int i; -}; - -static void array_bucketindex_swap(void *p, void *q) /* {{{ */ -{ - struct bucketindex *f = (struct bucketindex *)p; - struct bucketindex *g = (struct bucketindex *)q; - struct bucketindex t; - t = *f; - *f = *g; - *g = t; -} -/* }}} */ - /* {{{ proto array array_unique(array input [, int sort_flags]) Removes duplicate values from array */ PHP_FUNCTION(array_unique) { zval *array; - uint32_t idx; - Bucket *p; - struct bucketindex *arTmp, *cmpdata, *lastkept; - unsigned int i; + Bucket *p, *lastkept = NULL; zend_long sort_type = PHP_SORT_STRING; bucket_compare_func_t cmp; @@ -4478,44 +4568,16 @@ PHP_FUNCTION(array_unique) cmp = php_get_data_compare_func(sort_type, 0); RETVAL_ARR(zend_array_dup(Z_ARRVAL_P(array))); + zend_hash_sort(Z_ARRVAL_P(return_value), cmp, /* reindex */ 0); - /* create and sort array with pointers to the target_hash buckets */ - arTmp = (struct bucketindex *) pemalloc((Z_ARRVAL_P(array)->nNumOfElements + 1) * sizeof(struct bucketindex), GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT); - for (i = 0, idx = 0; idx < Z_ARRVAL_P(array)->nNumUsed; idx++) { - p = Z_ARRVAL_P(array)->arData + idx; - if (Z_TYPE(p->val) == IS_UNDEF) continue; - if (Z_TYPE(p->val) == IS_INDIRECT && Z_TYPE_P(Z_INDIRECT(p->val)) == IS_UNDEF) continue; - arTmp[i].b = *p; - arTmp[i].i = i; - i++; - } - ZVAL_UNDEF(&arTmp[i].b.val); - zend_sort((void *) arTmp, i, sizeof(struct bucketindex), - (compare_func_t) cmp, (swap_func_t)array_bucketindex_swap); /* go through the sorted array and delete duplicates from the copy */ - lastkept = arTmp; - for (cmpdata = arTmp + 1; Z_TYPE(cmpdata->b.val) != IS_UNDEF; cmpdata++) { - if (cmp(&lastkept->b, &cmpdata->b)) { - lastkept = cmpdata; + ZEND_HASH_FOREACH_BUCKET(Z_ARRVAL_P(return_value), p) { + if (!lastkept || cmp(lastkept, p)) { + lastkept = p; } else { - if (lastkept->i > cmpdata->i) { - p = &lastkept->b; - lastkept = cmpdata; - } else { - p = &cmpdata->b; - } - if (p->key == NULL) { - zend_hash_index_del(Z_ARRVAL_P(return_value), p->h); - } else { - if (Z_ARRVAL_P(return_value) == &EG(symbol_table)) { - zend_delete_global_variable(p->key); - } else { - zend_hash_del(Z_ARRVAL_P(return_value), p->key); - } - } + zend_hash_del_bucket(Z_ARRVAL_P(return_value), p); } - } - pefree(arTmp, GC_FLAGS(Z_ARRVAL_P(array)) & IS_ARRAY_PERSISTENT); + } ZEND_HASH_FOREACH_END(); } /* }}} */ @@ -4661,12 +4723,12 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int /* array_intersect() */ req_args = 2; param_spec = "+"; - intersect_data_compare_func = php_array_data_compare_string; + intersect_data_compare_func = php_array_data_compare_string_unstable; } else if (data_compare_type == INTERSECT_COMP_DATA_USER) { /* array_uintersect() */ req_args = 3; param_spec = "+f"; - intersect_data_compare_func = php_array_user_compare; + intersect_data_compare_func = php_array_user_compare_unstable; } else { ZEND_ASSERT(0 && "Invalid data_compare_type"); return; @@ -4691,30 +4753,30 @@ static void php_array_intersect(INTERNAL_FUNCTION_PARAMETERS, int behavior, int /* array_intersect_assoc() or array_intersect_key() */ req_args = 2; param_spec = "+"; - intersect_key_compare_func = php_array_key_compare_string; - intersect_data_compare_func = php_array_data_compare_string; + intersect_key_compare_func = php_array_key_compare_string_unstable; + intersect_data_compare_func = php_array_data_compare_string_unstable; } else if (data_compare_type == INTERSECT_COMP_DATA_USER && key_compare_type == INTERSECT_COMP_KEY_INTERNAL) { /* array_uintersect_assoc() */ req_args = 3; param_spec = "+f"; - intersect_key_compare_func = php_array_key_compare_string; - intersect_data_compare_func = php_array_user_compare; + intersect_key_compare_func = php_array_key_compare_string_unstable; + intersect_data_compare_func = php_array_user_compare_unstable; fci_data = &fci1; fci_data_cache = &fci1_cache; } else if (data_compare_type == INTERSECT_COMP_DATA_INTERNAL && key_compare_type == INTERSECT_COMP_KEY_USER) { /* array_intersect_uassoc() or array_intersect_ukey() */ req_args = 3; param_spec = "+f"; - intersect_key_compare_func = php_array_user_key_compare; - intersect_data_compare_func = php_array_data_compare_string; + intersect_key_compare_func = php_array_user_key_compare_unstable; + intersect_data_compare_func = php_array_data_compare_string_unstable; fci_key = &fci1; fci_key_cache = &fci1_cache; } else if (data_compare_type == INTERSECT_COMP_DATA_USER && key_compare_type == INTERSECT_COMP_KEY_USER) { /* array_uintersect_uassoc() */ req_args = 4; param_spec = "+ff"; - intersect_key_compare_func = php_array_user_key_compare; - intersect_data_compare_func = php_array_user_compare; + intersect_key_compare_func = php_array_user_key_compare_unstable; + intersect_data_compare_func = php_array_user_compare_unstable; fci_data = &fci1; fci_data_cache = &fci1_cache; fci_key = &fci2; @@ -5062,18 +5124,18 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ bucket_compare_func_t diff_data_compare_func; if (behavior == DIFF_NORMAL) { - diff_key_compare_func = php_array_key_compare_string; + diff_key_compare_func = php_array_key_compare_string_unstable; if (data_compare_type == DIFF_COMP_DATA_INTERNAL) { /* array_diff */ req_args = 2; param_spec = "+"; - diff_data_compare_func = php_array_data_compare_string; + diff_data_compare_func = php_array_data_compare_string_unstable; } else if (data_compare_type == DIFF_COMP_DATA_USER) { /* array_udiff */ req_args = 3; param_spec = "+f"; - diff_data_compare_func = php_array_user_compare; + diff_data_compare_func = php_array_user_compare_unstable; } else { ZEND_ASSERT(0 && "Invalid data_compare_type"); return; @@ -5098,30 +5160,30 @@ static void php_array_diff(INTERNAL_FUNCTION_PARAMETERS, int behavior, int data_ /* array_diff_assoc() or array_diff_key() */ req_args = 2; param_spec = "+"; - diff_key_compare_func = php_array_key_compare_string; - diff_data_compare_func = php_array_data_compare_string; + diff_key_compare_func = php_array_key_compare_string_unstable; + diff_data_compare_func = php_array_data_compare_string_unstable; } else if (data_compare_type == DIFF_COMP_DATA_USER && key_compare_type == DIFF_COMP_KEY_INTERNAL) { /* array_udiff_assoc() */ req_args = 3; param_spec = "+f"; - diff_key_compare_func = php_array_key_compare_string; - diff_data_compare_func = php_array_user_compare; + diff_key_compare_func = php_array_key_compare_string_unstable; + diff_data_compare_func = php_array_user_compare_unstable; fci_data = &fci1; fci_data_cache = &fci1_cache; } else if (data_compare_type == DIFF_COMP_DATA_INTERNAL && key_compare_type == DIFF_COMP_KEY_USER) { /* array_diff_uassoc() or array_diff_ukey() */ req_args = 3; param_spec = "+f"; - diff_key_compare_func = php_array_user_key_compare; - diff_data_compare_func = php_array_data_compare_string; + diff_key_compare_func = php_array_user_key_compare_unstable; + diff_data_compare_func = php_array_data_compare_string_unstable; fci_key = &fci1; fci_key_cache = &fci1_cache; } else if (data_compare_type == DIFF_COMP_DATA_USER && key_compare_type == DIFF_COMP_KEY_USER) { /* array_udiff_uassoc() */ req_args = 4; param_spec = "+ff"; - diff_key_compare_func = php_array_user_key_compare; - diff_data_compare_func = php_array_user_compare; + diff_key_compare_func = php_array_user_key_compare_unstable; + diff_data_compare_func = php_array_user_compare_unstable; fci_data = &fci1; fci_data_cache = &fci1_cache; fci_key = &fci2; @@ -5507,7 +5569,7 @@ PHPAPI int php_multisort_compare(const void *a, const void *b) /* {{{ */ r++; } while (Z_TYPE(ab[r].val) != IS_UNDEF); - return 0; + return stable_sort_fallback(&ab[r], &bb[r]); } /* }}} */ @@ -5571,7 +5633,7 @@ PHP_FUNCTION(array_multisort) /* We see the next array, so we update the sort flags of * the previous array and reset the sort flags. */ if (i > 0) { - ARRAYG(multisort_func)[num_arrays - 1] = php_get_data_compare_func(sort_type, sort_order != PHP_SORT_ASC); + ARRAYG(multisort_func)[num_arrays - 1] = php_get_data_compare_func_unstable(sort_type, sort_order != PHP_SORT_ASC); sort_order = PHP_SORT_ASC; sort_type = PHP_SORT_REGULAR; } @@ -5624,7 +5686,7 @@ PHP_FUNCTION(array_multisort) } } /* Take care of the last array sort flags. */ - ARRAYG(multisort_func)[num_arrays - 1] = php_get_data_compare_func(sort_type, sort_order != PHP_SORT_ASC); + ARRAYG(multisort_func)[num_arrays - 1] = php_get_data_compare_func_unstable(sort_type, sort_order != PHP_SORT_ASC); /* Make sure the arrays are of the same size. */ array_size = zend_hash_num_elements(Z_ARRVAL_P(arrays[0])); @@ -5644,8 +5706,8 @@ PHP_FUNCTION(array_multisort) /* Create the indirection array. This array is of size MxN, where * M is the number of entries in each input array and N is the number - * of the input arrays + 1. The last column is NULL to indicate the end - * of the row. */ + * of the input arrays + 1. The last column is UNDEF to indicate the end + * of the row. It also stores the original position for stable sorting. */ indirect = (Bucket **)safe_emalloc(array_size, sizeof(Bucket *), 0); for (i = 0; i < array_size; i++) { indirect[i] = (Bucket *)safe_emalloc((num_arrays + 1), sizeof(Bucket), 0); @@ -5661,6 +5723,7 @@ PHP_FUNCTION(array_multisort) } for (k = 0; k < array_size; k++) { ZVAL_UNDEF(&indirect[k][num_arrays].val); + Z_EXTRA_P(&indirect[k][num_arrays].val) = k; } /* Do the actual sort magic - bada-bim, bada-boom. */ diff --git a/ext/standard/php_array.h b/ext/standard/php_array.h index c513539dd2a..a49e12488f1 100644 --- a/ext/standard/php_array.h +++ b/ext/standard/php_array.h @@ -46,6 +46,7 @@ PHPAPI zend_long php_count_recursive(HashTable *ht); ZEND_BEGIN_MODULE_GLOBALS(array) bucket_compare_func_t *multisort_func; + zend_bool compare_deprecation_thrown; ZEND_END_MODULE_GLOBALS(array) #define ARRAYG(v) ZEND_MODULE_GLOBALS_ACCESSOR(array, v) diff --git a/ext/standard/tests/array/array_multisort_stability.phpt b/ext/standard/tests/array/array_multisort_stability.phpt new file mode 100644 index 00000000000..67814254ec4 --- /dev/null +++ b/ext/standard/tests/array/array_multisort_stability.phpt @@ -0,0 +1,15 @@ +--TEST-- +array_multisort() is stable +--FILE-- + +--EXPECT-- +bool(true) diff --git a/ext/standard/tests/array/asort_stability.phpt b/ext/standard/tests/array/asort_stability.phpt new file mode 100644 index 00000000000..aeb3b9c1c7a --- /dev/null +++ b/ext/standard/tests/array/asort_stability.phpt @@ -0,0 +1,12 @@ +--TEST-- +asort() is stable +--FILE-- + +--EXPECT-- +bool(true) diff --git a/ext/standard/tests/array/bug71334.phpt b/ext/standard/tests/array/bug71334.phpt index ba14f08c181..96dacba2f64 100644 --- a/ext/standard/tests/array/bug71334.phpt +++ b/ext/standard/tests/array/bug71334.phpt @@ -21,7 +21,7 @@ class myClass throw new Exception('Missing Y: "' . $y . '"'); } - return $x < $y; + return $x <=> $y; } public function __construct() diff --git a/ext/standard/tests/array/natcasesort_variation9.phpt b/ext/standard/tests/array/natcasesort_variation9.phpt index f7a1d7581f411fb5986bf78b8e70b603ece3ff5c..6bd10338b6bed54f7cedb0c6de01e92f3493cfb1 100644 GIT binary patch delta 102 zcmX@fbCPF+I`d>dW=TQgSX(hW8BHep zbEwLgLZpjJiZb)kf$|DUiAl*RN+4xslVe#W7|kc2x*ZtHk6o zR=&xaoV<*NlOM2(YeH?z%q!8*H3C~`XpHO#n4rmI1vV*jr~~{$eSAQc8k!+1HGpV0 vhlqhpgo;>zY!b0Vi0eYUW@IpVK8xt)Qnq%+(XarA#pF^pQC64yyi_g#)`o{R delta 552 zcmaDR{7iVmQkKbSZ2Xhwvq?vI!6&i^&NbQj?Ey@bMY}btgi!8yZZuWtCzwG@QJi zL!8ObXmSCo1f%if2OLt4s5Tk^^?)rjG(|RA*9dH#A(8`NLgtgLIi*Z3AgcXBeSAR9 qHME5Aq5goIYGeQr2l+tPWOF`SFXL!vC_qAEaz2|ht4n@fDi;7BX@v9u diff --git a/ext/standard/tests/array/sort_variation11.phpt b/ext/standard/tests/array/sort_variation11.phpt index 5209d21a67478cd705de78daf1f8aa2530288fd8..fce546bdbcc0e87720f1503d13e84b34413b3a5f 100644 GIT binary patch delta 559 zcmX>tc3N!1GM33lIRpd@V{PrY6cqeIeSCmaw9(`RETX=~5W&p65)EAwO^~<=L?kUI zKM^Qqq-SXXgkTv{B(+9h5i?Y^ll57ZMT$#`GV{_kj5QUMboDHiKt@_l=I4}PG?;7$ zBn>B@WtEmTf@p-QH3F(lOad!4G@jhcDLpxilW+1}PHsi0l@P}mfbBGdS_*Qqf#zgM zHe)`BG{pIamXrT-N-`R3?q$hGld{2YyvqS92&$vfH0M{0=3Vhs_lS}vFT Hyi_g#v<8dR delta 481 zcmX>tc3N!1GM33<9Q?wDv9@+x3JRHdB^tUWnm|^x(d2h5lIq4t;znR`6NpG!PJSX# zEkw+8@_7zP5i?{N1F&NA$qQJ-CZFSA=LVS!5wiq}i3=J)6#Iqx_<(FPG@KmIDj{eD z5lza^&(SC;Doxd#e2`U`$52y2$y5of%w+OJR%u4l$!l398O +--EXPECT-- +bool(true) diff --git a/ext/standard/tests/array/usort_variation11.phpt b/ext/standard/tests/array/usort_variation11.phpt index d07dfb48e05..88797e5f24f 100644 --- a/ext/standard/tests/array/usort_variation11.phpt +++ b/ext/standard/tests/array/usort_variation11.phpt @@ -1,5 +1,5 @@ --TEST-- -Test usort() function : usage variations - binary return cmp +Test usort() function : usage variations - malformed comparison function returning boolean --FILE-- ---EXPECT-- +--EXPECTF-- +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or greater than zero in %s on line %d okey