Revert "Fix GH-9296: ksort behaves incorrectly on arrays with mixed…

… keys" This reverts commit cd1aed8, as discussed on internals (<https://externals.io/message/118483>).
php · Aug 29, 2022 · 912dc44 · 912dc44
1 parent 8590731
commit 912dc44
Show file tree

Hide file tree

Showing 7 changed files with 52 additions and 54 deletions.
diff --git a/ext/standard/array.c b/ext/standard/array.c
@@ -166,25 +166,40 @@ static zend_never_inline ZEND_COLD int stable_sort_fallback(Bucket *a, Bucket *b
 
 static zend_always_inline int php_array_key_compare_unstable_i(Bucket *f, Bucket *s) /* {{{ */
 {
-    zval first;
-    zval second;
-
-    if (f->key == NULL && s->key == NULL) {
-        return (zend_long)f->h > (zend_long)s->h ? 1 : -1;
-    } else if (f->key && s->key) {
-        return zendi_smart_strcmp(f->key, s->key);
-    }
-    if (f->key) {
-        ZVAL_STR(&first, f->key);
-    } else {
-        ZVAL_LONG(&first, f->h);
-    }
-    if (s->key) {
-        ZVAL_STR(&second, s->key);
-    } else {
-        ZVAL_LONG(&second, s->h);
-    }
-    return zend_compare(&first, &second);
+	zend_uchar t;
+	zend_long l1, l2;
+	double d;
+
+	if (f->key == NULL) {
+		if (s->key == NULL) {
+			return (zend_long)f->h > (zend_long)s->h ? 1 : -1;
+		} else {
+			l1 = (zend_long)f->h;
+			t = is_numeric_string(s->key->val, s->key->len, &l2, &d, 1);
+			if (t == IS_LONG) {
+				/* pass */
+			} else if (t == IS_DOUBLE) {
+				return ZEND_NORMALIZE_BOOL((double)l1 - d);
+			} else {
+				l2 = 0;
+			}
+		}
+	} else {
+		if (s->key) {
+			return zendi_smart_strcmp(f->key, s->key);
+		} else {
+			l2 = (zend_long)s->h;
+			t = is_numeric_string(f->key->val, f->key->len, &l1, &d, 1);
+			if (t == IS_LONG) {
+				/* pass */
+			} else if (t == IS_DOUBLE) {
+				return ZEND_NORMALIZE_BOOL(d - (double)l2);
+			} else {
+				l1 = 0;
+			}
+		}
+	}
+	return ZEND_NORMALIZE_BOOL(l1 - l2);
 }
 /* }}} */
 

diff --git a/ext/standard/tests/array/002.phpt b/ext/standard/tests/array/002.phpt
@@ -276,8 +276,6 @@ array(8) {
  -- Testing krsort() -- 
 No second argument:
 array(8) {
-  ["test"]=>
-  int(27)
   [16777216]=>
   float(-0.3333333333333333)
   [1001]=>
@@ -290,6 +288,8 @@ array(8) {
   string(4) "Test"
   [0]=>
   string(3) "PHP"
+  ["test"]=>
+  int(27)
   [-1000]=>
   array(2) {
     [0]=>
@@ -300,8 +300,6 @@ array(8) {
 }
 Using SORT_REGULAR:
 array(8) {
-  ["test"]=>
-  int(27)
   [16777216]=>
   float(-0.3333333333333333)
   [1001]=>
@@ -314,6 +312,8 @@ array(8) {
   string(4) "Test"
   [0]=>
   string(3) "PHP"
+  ["test"]=>
+  int(27)
   [-1000]=>
   array(2) {
     [0]=>
@@ -334,10 +334,10 @@ array(8) {
   string(27) "PHP: Hypertext Preprocessor"
   [5]=>
   string(4) "Test"
-  ["test"]=>
-  int(27)
   [0]=>
   string(3) "PHP"
+  ["test"]=>
+  int(27)
   [-1000]=>
   array(2) {
     [0]=>
@@ -383,6 +383,8 @@ array(8) {
   }
   [0]=>
   string(3) "PHP"
+  ["test"]=>
+  int(27)
   [5]=>
   string(4) "Test"
   [17]=>
@@ -393,8 +395,6 @@ array(8) {
   string(6) "monkey"
   [16777216]=>
   float(-0.3333333333333333)
-  ["test"]=>
-  int(27)
 }
 Using SORT_REGULAR:
 array(8) {
@@ -407,6 +407,8 @@ array(8) {
   }
   [0]=>
   string(3) "PHP"
+  ["test"]=>
+  int(27)
   [5]=>
   string(4) "Test"
   [17]=>
@@ -417,8 +419,6 @@ array(8) {
   string(6) "monkey"
   [16777216]=>
   float(-0.3333333333333333)
-  ["test"]=>
-  int(27)
 }
 Using SORT_NUMERIC:
 array(8) {

diff --git a/ext/standard/tests/array/gh9296.phpt b/ext/standard/tests/array/gh9296.phpt
diff --git a/ext/standard/tests/array/krsort_variation8.phpt b/ext/standard/tests/array/krsort_variation8.phpt
diff --git a/ext/standard/tests/array/krsort_variation9.phpt b/ext/standard/tests/array/krsort_variation9.phpt
@@ -82,22 +82,22 @@ array(5) {
 - With default sort flag -
 bool(true)
 array(3) {
-  ["c"]=>
-  string(5) "apple"
   ["a"]=>
   string(6) "orange"
   [0]=>
   string(6) "banana"
+  ["c"]=>
+  string(5) "apple"
 }
 - Sort flag = SORT_REGULAR -
 bool(true)
 array(3) {
-  ["c"]=>
-  string(5) "apple"
   ["a"]=>
   string(6) "orange"
   [0]=>
   string(6) "banana"
+  ["c"]=>
+  string(5) "apple"
 }
 
 -- Iteration 3 --

diff --git a/ext/standard/tests/array/ksort_variation8.phpt b/ext/standard/tests/array/ksort_variation8.phpt
diff --git a/ext/standard/tests/array/ksort_variation9.phpt b/ext/standard/tests/array/ksort_variation9.phpt
@@ -81,20 +81,20 @@ array(5) {
 - With default sort flag -
 bool(true)
 array(3) {
-  [0]=>
-  string(6) "banana"
   ["a"]=>
   string(6) "orange"
+  [0]=>
+  string(6) "banana"
   ["c"]=>
   string(5) "apple"
 }
 - Sort flag = SORT_REGULAR -
 bool(true)
 array(3) {
-  [0]=>
-  string(6) "banana"
   ["a"]=>
   string(6) "orange"
+  [0]=>
+  string(6) "banana"
   ["c"]=>
   string(5) "apple"
 }