From 42ba8943b8ac3c2aca26f12feb3dd4fb40a24dff Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 4 Mar 2020 11:52:55 +0100 Subject: [PATCH 1/9] Make sorting stable --- Zend/zend_hash.c | 32 ++- ext/standard/array.c | 266 ++++++++++-------- .../tests/array/natcasesort_variation9.phpt | Bin 1769 -> 1769 bytes .../tests/array/rsort_variation11.phpt | Bin 2689 -> 2689 bytes .../tests/array/sort_variation11.phpt | Bin 3141 -> 3141 bytes ext/standard/tests/array/usort_stability.phpt | 15 + .../tests/array/usort_variation11.phpt | 25 -- 7 files changed, 188 insertions(+), 150 deletions(-) create mode 100644 ext/standard/tests/array/usort_stability.phpt delete mode 100644 ext/standard/tests/array/usort_variation11.phpt diff --git a/Zend/zend_hash.c b/Zend/zend_hash.c index d57c0a69d2e03..5351216f16932 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/standard/array.c b/ext/standard/array.c index fd0ceaa6006cf..cf075e8c8a12b 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -132,7 +132,32 @@ PHP_MSHUTDOWN_FUNCTION(array) /* {{{ */ } /* }}} */ -static int php_array_key_compare(Bucket *f, Bucket *s) /* {{{ */ +#define RETURN_STABLE_SORT(a, b, result) do { \ + int _result = (result); \ + if (_result) { \ + return _result; \ + } \ + 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; \ + } \ +} while (0) + +#define DEFINE_SORT_VARIANTS(name) \ + static int php_array_reverse_##name##_unstable(Bucket *a, Bucket *b) { \ + return php_array_##name##_unstable(a, b) * -1; \ + } \ + static int php_array_##name(Bucket *a, Bucket *b) { \ + RETURN_STABLE_SORT(a, b, php_array_##name##_unstable(a, b)); \ + } \ + static int php_array_reverse_##name(Bucket *a, Bucket *b) { \ + RETURN_STABLE_SORT(a, b, php_array_reverse_##name##_unstable(a, b)); \ + } \ + +static int php_array_key_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zend_uchar t; zend_long l1, l2; @@ -171,13 +196,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 int php_array_key_compare_numeric_unstable(Bucket *f, Bucket *s) /* {{{ */ { if (f->key == NULL && s->key == NULL) { return (zend_long)f->h > (zend_long)s->h ? 1 : -1; @@ -198,13 +217,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 int php_array_key_compare_string_case_unstable(Bucket *f, Bucket *s) /* {{{ */ { const char *s1, *s2; size_t l1, l2; @@ -229,13 +242,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 int php_array_key_compare_string_unstable(Bucket *f, Bucket *s) /* {{{ */ { const char *s1, *s2; size_t l1, l2; @@ -260,12 +267,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 +294,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 int php_array_key_compare_string_locale_unstable(Bucket *f, Bucket *s) /* {{{ */ { const char *s1, *s2; char buf1[MAX_LENGTH_OF_LONG + 1]; @@ -335,13 +336,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 int php_array_data_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -356,13 +351,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 int php_array_data_compare_numeric_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -378,13 +367,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 int php_array_data_compare_string_case_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -400,13 +383,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 int php_array_data_compare_string_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -422,12 +399,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 +413,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 int php_array_natural_compare_unstable(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 int php_array_natural_case_compare_unstable(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(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; @@ -482,11 +441,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 +582,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,7 +911,7 @@ PHP_FUNCTION(rsort) } /* }}} */ -static int php_array_user_compare(Bucket *f, Bucket *s) /* {{{ */ +static int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval args[2]; zval retval; @@ -907,6 +937,12 @@ static int php_array_user_compare(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ +static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */ +{ + RETURN_STABLE_SORT(a, b, php_array_user_compare_unstable(a, b)); +} +/* }}} */ + /* check if comparison function is valid */ #define PHP_ARRAY_CMP_FUNC_CHECK(func_name) \ if (!zend_is_callable(*func_name, 0, NULL)) { \ @@ -985,7 +1021,7 @@ PHP_FUNCTION(uasort) } /* }}} */ -static int php_array_user_key_compare(Bucket *f, Bucket *s) /* {{{ */ +static int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval args[2]; zval retval; @@ -1020,6 +1056,12 @@ static int php_array_user_key_compare(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ +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) @@ -4662,12 +4704,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; @@ -4692,30 +4734,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; @@ -5063,18 +5105,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; @@ -5099,30 +5141,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; @@ -5572,7 +5614,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; } @@ -5625,7 +5667,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])); diff --git a/ext/standard/tests/array/natcasesort_variation9.phpt b/ext/standard/tests/array/natcasesort_variation9.phpt index 04b36f369537b6f4dacf9645e8de614749019034..3934ea0a9be5065732669e06d4da4ce733592292 100644 GIT binary patch delta 102 zcmaFK`;vFVapuYIm?Z^`V{PrY6cqeIeSCmaw4vqX56oi1MurgKr2PCGjkLs^;#AGa qQY>rLp{k2ZiZb)kH4HTsluVRB+DstI({l0?OEgS0H@{=~%?JP%DIKx^ delta 106 zcmaFK`;vFVab`^;!&qB8E(L|8{QMk^w8Wg^R81f++Rzvx>=)|e1Lhk|{>Usg`8~75 qWI2|V>Lw7`w4D6J5)Biu4ycCWlA_GKbPYpI1tk-u&F@%#GXem3Y95CG diff --git a/ext/standard/tests/array/rsort_variation11.phpt b/ext/standard/tests/array/rsort_variation11.phpt index 5c851b4c4cd3ad04bdfc34eac1e0c1dd18aa82c3..6a629cf6a03c8cb7a581c24536c8b91103d2e8f6 100644 GIT binary patch delta 586 zcmZn^Z4}+$z&bgURY=Du*4Bfy7NF zzv573G@X2xRf^GUvOlMUj5&%*6QIhFqS92bVvEUoY~qZTljm_tFd9rwV3Tq*L{V%2 zR18*V2z5bbUWtaT5!eBSP!~WP1rs!xyqirb$rNIWU#O1{$W4Z35I#hy0Yp30Ss)Xk nA{IF8wuFd-?S^>X$YAnB4$;Z!T%wy**^3xQ!^jX%7)b#DV``1P delta 494 zcmZn^Z4}+$z{;PLpP!>qQdF9%$)%tWZ8UizhlGf6tgRi8mzI;CSfXJ9mN1#TmqVP< zbaEZ5s;U`8Iy0|C!vHL4j;tOcWHI?ShZLjb+a!0DWh9;Q#;t diff --git a/ext/standard/tests/array/sort_variation11.phpt b/ext/standard/tests/array/sort_variation11.phpt index 50d40fca63cf5d5eeb090bd0c9a97922e1e924a0..816345ee90816d1ce0639301489a259a0b77c203 100644 GIT binary patch delta 505 zcmX>qaa3Z%0@lgTIR(vQZSA-e6#PPce1KH65rmtRpP!?VmY7qVstFP`hKOe7m1yXi zfJICu`*2FCn<9xDfyK=rB567Ki9odwG4sg}ImITQ=VYHeiBp)-a&kSJ1f#*^bwJW^ zav+y9qtWDCE=fk?$$#0TCm&3iL|tWCK>0$v-&^hp*3Narm6eB|k5f3jj46fxQ3# delta 575 zcmX>qaa3Z%0@lgLSownuV{PrY6cjS^N;Gs$G=Z#WBZx>^PJSX#%t+7D0tmq}#z<<7 zz#=B7YQbWrlkM3g`OJ`I3?{GPR89ifVE|TQ36U%=Day=C2Z|{uDS>P@G=PZvh5Gn_ z`B29})focSnLrdm-I$c0pQBMyRGJDl)zEnIA5MwMXxEiYbP79x=jAbVK^E%;Ix1oIPA4tF8O(> FTmZegk +--EXPECT-- +bool(true) diff --git a/ext/standard/tests/array/usort_variation11.phpt b/ext/standard/tests/array/usort_variation11.phpt deleted file mode 100644 index d07dfb48e05db..0000000000000 --- a/ext/standard/tests/array/usort_variation11.phpt +++ /dev/null @@ -1,25 +0,0 @@ ---TEST-- -Test usort() function : usage variations - binary return cmp ---FILE-- - $b; -} - -$range = array(2, 4, 8, 16, 32, 64, 128); - -foreach ($range as $r) { - $backup = $array = range(0, $r); - shuffle($array); - usort($array, "ucmp"); - if ($array != $backup) { - var_dump($array); - var_dump($backup); - die("Whatever sorting algo you used, this test should never be broken"); - } -} -echo "okey"; -?> ---EXPECT-- -okey From ef8848a9eacf58b820a4bd244de86360273c299f Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 4 Mar 2020 17:54:41 +0100 Subject: [PATCH 2/9] Optimize array_unique() implementation --- ext/standard/array.c | 61 +++++--------------------------------------- 1 file changed, 7 insertions(+), 54 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index cf075e8c8a12b..1a727fe27ba2c 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -4440,31 +4440,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; @@ -4521,44 +4502,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(); } /* }}} */ From ad17703a11f53fea16a8f551c39b6aaa47aac7a2 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 5 Mar 2020 10:23:27 +0100 Subject: [PATCH 3/9] Optimize --- ext/standard/array.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 1a727fe27ba2c..402b7fdbb4444 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -134,7 +134,7 @@ PHP_MSHUTDOWN_FUNCTION(array) /* {{{ */ #define RETURN_STABLE_SORT(a, b, result) do { \ int _result = (result); \ - if (_result) { \ + if (EXPECTED(_result)) { \ return _result; \ } \ if (Z_EXTRA((a)->val) > Z_EXTRA((b)->val)) { \ @@ -336,7 +336,7 @@ static int php_array_key_compare_string_locale_unstable(Bucket *f, Bucket *s) /* } /* }}} */ -static int php_array_data_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ +static inline int php_array_data_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval *first = &f->val; zval *second = &s->val; From 9b3c42e8dc6464fcfc9a8d056d645ce1a64f3924 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 5 Mar 2020 10:53:00 +0100 Subject: [PATCH 4/9] Carefully control inlining --- ext/standard/array.c | 60 +++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 402b7fdbb4444..0689731a93606 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -132,32 +132,40 @@ PHP_MSHUTDOWN_FUNCTION(array) /* {{{ */ } /* }}} */ +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; \ } \ - 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; \ - } \ + 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 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_##name##_unstable(Bucket *a, Bucket *b) { \ + return php_array_##name##_unstable_i(a, b); \ } \ - static int php_array_##name(Bucket *a, Bucket *b) { \ - RETURN_STABLE_SORT(a, b, php_array_##name##_unstable(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 int php_array_reverse_##name(Bucket *a, Bucket *b) { \ + 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 int php_array_key_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ +static zend_always_inline int php_array_key_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */ { zend_uchar t; zend_long l1, l2; @@ -196,7 +204,7 @@ static int php_array_key_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ } /* }}} */ -static int php_array_key_compare_numeric_unstable(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; @@ -217,7 +225,7 @@ static int php_array_key_compare_numeric_unstable(Bucket *f, Bucket *s) /* {{{ * } /* }}} */ -static int php_array_key_compare_string_case_unstable(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; @@ -242,7 +250,7 @@ static int php_array_key_compare_string_case_unstable(Bucket *f, Bucket *s) /* { } /* }}} */ -static int php_array_key_compare_string_unstable(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; @@ -316,7 +324,7 @@ static int php_array_reverse_key_compare_string_natural(Bucket *a, Bucket *b) /* } /* }}} */ -static int php_array_key_compare_string_locale_unstable(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]; @@ -336,7 +344,7 @@ static int php_array_key_compare_string_locale_unstable(Bucket *f, Bucket *s) /* } /* }}} */ -static inline int php_array_data_compare_unstable(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; @@ -351,7 +359,7 @@ static inline int php_array_data_compare_unstable(Bucket *f, Bucket *s) /* {{{ * } /* }}} */ -static int php_array_data_compare_numeric_unstable(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; @@ -367,7 +375,7 @@ static int php_array_data_compare_numeric_unstable(Bucket *f, Bucket *s) /* {{{ } /* }}} */ -static int php_array_data_compare_string_case_unstable(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; @@ -383,7 +391,7 @@ static int php_array_data_compare_string_case_unstable(Bucket *f, Bucket *s) /* } /* }}} */ -static int php_array_data_compare_string_unstable(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; @@ -413,19 +421,19 @@ static int php_array_natural_general_compare(Bucket *f, Bucket *s, int fold_case } /* }}} */ -static int php_array_natural_compare_unstable(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_natural_case_compare_unstable(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_data_compare_string_locale_unstable(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; @@ -911,7 +919,7 @@ PHP_FUNCTION(rsort) } /* }}} */ -static int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ +static inline int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval args[2]; zval retval; @@ -1021,7 +1029,7 @@ PHP_FUNCTION(uasort) } /* }}} */ -static int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ +static inline int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* {{{ */ { zval args[2]; zval retval; From 0dfee003e3219b402fe8b4c4a5630a54ca250dc4 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Thu, 5 Mar 2020 12:09:11 +0100 Subject: [PATCH 5/9] Deprecate boolean return from comparison But call with reversed operands to make sure it continues working. --- ext/spl/tests/bug67539.phpt | 2 +- ext/standard/array.c | 93 +++++++++++++++---- ext/standard/php_array.h | 1 + ext/standard/tests/array/bug71334.phpt | 2 +- .../tests/array/usort_variation11.phpt | 87 +++++++++++++++++ 5 files changed, 166 insertions(+), 19 deletions(-) create mode 100644 ext/standard/tests/array/usort_variation11.phpt diff --git a/ext/spl/tests/bug67539.phpt b/ext/spl/tests/bug67539.phpt index 8bab2a8c217bb..1001112e12744 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 0689731a93606..493275145ebb9 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -923,6 +923,7 @@ 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); @@ -931,17 +932,41 @@ static inline int php_array_user_compare_unstable(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 one of -1, 0 or 1 instead"); + 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); } /* }}} */ @@ -974,6 +999,7 @@ static int php_array_user_compare(Bucket *a, Bucket *b) /* {{{ */ #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() \ @@ -1033,7 +1059,7 @@ 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); @@ -1050,16 +1076,49 @@ static inline int php_array_user_key_compare_unstable(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 one of -1, 0 or 1 instead"); + 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); } /* }}} */ diff --git a/ext/standard/php_array.h b/ext/standard/php_array.h index c513539dd2a2e..a49e12488f169 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/bug71334.phpt b/ext/standard/tests/array/bug71334.phpt index ba14f08c18106..96dacba2f6452 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/usort_variation11.phpt b/ext/standard/tests/array/usort_variation11.phpt new file mode 100644 index 0000000000000..a9c8d6e31747b --- /dev/null +++ b/ext/standard/tests/array/usort_variation11.phpt @@ -0,0 +1,87 @@ +--TEST-- +Test usort() function : usage variations - malformed comparison function returning boolean +--FILE-- + $b; +} + +$range = array(2, 4, 8, 16, 32, 64, 128); + +foreach ($range as $r) { + $backup = $array = range(0, $r); + shuffle($array); + usort($array, "ucmp"); + if ($array != $backup) { + var_dump($array); + var_dump($backup); + die("Array not sorted (usort)"); + } + + shuffle($array); + $array = array_flip($array); + uksort($array, "ucmp"); + $array = array_keys($array); + if ($array != $backup) { + var_dump($array); + var_dump($backup); + die("Array not sorted (uksort)"); + } + + shuffle($array); + $array = array_combine($array, $array); + uasort($array, "ucmp"); + $array = array_keys($array); + if ($array != $backup) { + var_dump($array); + var_dump($backup); + die("Array not sorted (uasort)"); + } +} +echo "okey"; + +?> +--EXPECTF-- +Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d + +Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +okey From 983c9551184025ca3f1b161ced83a278e40392ac Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 11 Mar 2020 11:13:21 +0100 Subject: [PATCH 6/9] Add test for asort stability --- ext/standard/tests/array/asort_stability.phpt | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 ext/standard/tests/array/asort_stability.phpt diff --git a/ext/standard/tests/array/asort_stability.phpt b/ext/standard/tests/array/asort_stability.phpt new file mode 100644 index 0000000000000..aeb3b9c1c7a0f --- /dev/null +++ b/ext/standard/tests/array/asort_stability.phpt @@ -0,0 +1,12 @@ +--TEST-- +asort() is stable +--FILE-- + +--EXPECT-- +bool(true) From dba18222dfcb0e1f3a634f324ac800a6972ab9d8 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 11 Mar 2020 11:22:10 +0100 Subject: [PATCH 7/9] Improve error message --- ext/standard/array.c | 4 +- .../tests/array/usort_variation11.phpt | 42 +++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index 493275145ebb9..a38243c48764d 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -943,7 +943,7 @@ static inline int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ * if (!ARRAYG(compare_deprecation_thrown)) { php_error_docref(NULL, E_DEPRECATED, "Returning bool from comparison function is deprecated, " - "return one of -1, 0 or 1 instead"); + "return an integer less than, equal to, or larger than zero"); ARRAYG(compare_deprecation_thrown) = 1; } @@ -1087,7 +1087,7 @@ static inline int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* { if (!ARRAYG(compare_deprecation_thrown)) { php_error_docref(NULL, E_DEPRECATED, "Returning bool from comparison function is deprecated, " - "return one of -1, 0 or 1 instead"); + "return an integer less than, equal to, or larger than zero"); ARRAYG(compare_deprecation_thrown) = 1; } diff --git a/ext/standard/tests/array/usort_variation11.phpt b/ext/standard/tests/array/usort_variation11.phpt index a9c8d6e31747b..f2a0674da7bc3 100644 --- a/ext/standard/tests/array/usort_variation11.phpt +++ b/ext/standard/tests/array/usort_variation11.phpt @@ -43,45 +43,45 @@ echo "okey"; ?> --EXPECTF-- -Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: usort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uksort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uksort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d -Deprecated: uasort(): Returning bool from comparison function is deprecated, return one of -1, 0 or 1 instead in %s on line %d +Deprecated: uasort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger than zero in %s on line %d okey From 1db9c247db273d9a84bcf79afc0b511913d5fcd8 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Wed, 11 Mar 2020 13:51:14 +0100 Subject: [PATCH 8/9] s/larger/greater --- ext/standard/array.c | 4 +- .../tests/array/usort_variation11.phpt | 42 +++++++++---------- 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/ext/standard/array.c b/ext/standard/array.c index a38243c48764d..6764b201d6e82 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -943,7 +943,7 @@ static inline int php_array_user_compare_unstable(Bucket *f, Bucket *s) /* {{{ * 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 larger than zero"); + "return an integer less than, equal to, or greater than zero"); ARRAYG(compare_deprecation_thrown) = 1; } @@ -1087,7 +1087,7 @@ static inline int php_array_user_key_compare_unstable(Bucket *f, Bucket *s) /* { 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 larger than zero"); + "return an integer less than, equal to, or greater than zero"); ARRAYG(compare_deprecation_thrown) = 1; } diff --git a/ext/standard/tests/array/usort_variation11.phpt b/ext/standard/tests/array/usort_variation11.phpt index f2a0674da7bc3..88797e5f24fb7 100644 --- a/ext/standard/tests/array/usort_variation11.phpt +++ b/ext/standard/tests/array/usort_variation11.phpt @@ -43,45 +43,45 @@ echo "okey"; ?> --EXPECTF-- -Deprecated: usort(): Returning bool from comparison function is deprecated, return an integer less than, equal to, or larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 larger 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 From 8c1a96f5305bdebe25629d9a1a8949fb39f15994 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Tue, 12 May 2020 10:58:01 +0200 Subject: [PATCH 9/9] Make array_multisort() stable --- ext/standard/array.c | 7 ++++--- .../tests/array/array_multisort_stability.phpt | 15 +++++++++++++++ 2 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 ext/standard/tests/array/array_multisort_stability.phpt diff --git a/ext/standard/array.c b/ext/standard/array.c index 6764b201d6e82..4205a0dd8a952 100644 --- a/ext/standard/array.c +++ b/ext/standard/array.c @@ -5570,7 +5570,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]); } /* }}} */ @@ -5707,8 +5707,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); @@ -5724,6 +5724,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/tests/array/array_multisort_stability.phpt b/ext/standard/tests/array/array_multisort_stability.phpt new file mode 100644 index 0000000000000..67814254ec422 --- /dev/null +++ b/ext/standard/tests/array/array_multisort_stability.phpt @@ -0,0 +1,15 @@ +--TEST-- +array_multisort() is stable +--FILE-- + +--EXPECT-- +bool(true)