[JSC] Array.prototype.sort should rejects null comparator
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Jun 2018 07:36:40 +0000 (07:36 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 10 Jun 2018 07:36:40 +0000 (07:36 +0000)
https://bugs.webkit.org/show_bug.cgi?id=186458

Reviewed by Keith Miller.

JSTests:

* ChakraCore/test/Array/array_sort.baseline-jsc:
* stress/array-sort-bad-comparator.js:
(test):
* stress/sort-null-comparator.js: Removed.
* test262/expectations.yaml:

Source/JavaScriptCore:

This relaxed behavior is once introduced in r216169 to fix some pages by aligning
the behavior to Chrome and Firefox.

However, now Chrome, Firefox and Edge reject a null comparator. So only JavaScriptCore
accepts it. This patch reverts r216169 to align JSC to the other engines and fix
the spec issue.

* builtins/ArrayPrototype.js:
(sort):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@232666 268f45cc-cd09-0410-ab3c-d52691b4dbfc

JSTests/ChakraCore/test/Array/array_sort.baseline-jsc
JSTests/ChangeLog
JSTests/stress/array-sort-bad-comparator.js
JSTests/stress/sort-null-comparator.js [deleted file]
JSTests/test262/expectations.yaml
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/ArrayPrototype.js

index 6ddc098..885663a 100644 (file)
@@ -7,4 +7,5 @@ for,sample,some,strings,testing - Output different in cscript due to NaN
 1,2,3
 10
 1,1.2,4,4.8,12
+TypeError: Array.prototype.sort requires the comparsion function be a function or undefined
 1,2,3
index 64119c9..585a6b8 100644 (file)
@@ -1,3 +1,16 @@
+2018-06-10  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Array.prototype.sort should rejects null comparator
+        https://bugs.webkit.org/show_bug.cgi?id=186458
+
+        Reviewed by Keith Miller.
+
+        * ChakraCore/test/Array/array_sort.baseline-jsc:
+        * stress/array-sort-bad-comparator.js:
+        (test):
+        * stress/sort-null-comparator.js: Removed.
+        * test262/expectations.yaml:
+
 2018-06-07  Saam Barati  <sbarati@apple.com>
 
         Make DFG to FTL OSR entry code more sane by removing bad RELEASE_ASSERTS and making it trigger compiles in outer loops before inner ones
index 0d9b27c..c08203a 100644 (file)
@@ -2,9 +2,13 @@
 
 function test() {
     try {
+        [1,2].sort(null);
+        return false;
+        } catch (enull) {}
+    try {
         [1,2].sort(true);
         return false;
-    } catch (etrue) {}
+        } catch (etrue) {}
     try {
         [1,2].sort({});
         return false;
diff --git a/JSTests/stress/sort-null-comparator.js b/JSTests/stress/sort-null-comparator.js
deleted file mode 100644 (file)
index 47398f0..0000000
+++ /dev/null
@@ -1,10 +0,0 @@
-// While this is not required by the spec. It looks like some websites rely on the comparator being null.
-
-function assertEq(a, b) {
-    if (a !== b)
-        throw new Error();
-}
-
-let array = [2,1].sort(null);
-assertEq(array[0], 1);
-assertEq(array[1], 2);
index 34ece81..769122d 100644 (file)
@@ -705,9 +705,6 @@ test/built-ins/Array/prototype/slice/target-array-with-non-writable-property.js:
 test/built-ins/Array/prototype/sort/S15.4.4.11_A4_T3.js:
   default: 'Test262Error: #3: var obj = {}; obj.sort = Array.prototype.sort; obj[0] = "z"; obj[1] = "y"; obj[2] = "x"; obj.length = -4294967294; obj.sort(); obj[0] === "z". Actual: y'
   strict mode: 'Test262Error: #3: var obj = {}; obj.sort = Array.prototype.sort; obj[0] = "z"; obj[1] = "y"; obj[2] = "x"; obj.length = -4294967294; obj.sort(); obj[0] === "z". Actual: y'
-test/built-ins/Array/prototype/sort/comparefn-nonfunction-call-throws.js:
-  default: 'Test262Error: sample.sort(null); Expected a TypeError to be thrown but no exception was thrown at all'
-  strict mode: 'Test262Error: sample.sort(null); Expected a TypeError to be thrown but no exception was thrown at all'
 test/built-ins/Array/prototype/splice/S15.4.4.12_A3_T1.js:
   default: 'Test262Error: #1: var obj = {}; obj.splice = Array.prototype.splice; obj[0] = "x"; obj[4294967295] = "y"; obj.length = 4294967296; var arr = obj.splice(4294967295,1); arr.length === 1. Actual: 0'
   strict mode: 'Test262Error: #1: var obj = {}; obj.splice = Array.prototype.splice; obj[0] = "x"; obj[4294967295] = "y"; obj.length = 4294967296; var arr = obj.splice(4294967295,1); arr.length === 1. Actual: 0'
index 5dd36cf..537da6a 100644 (file)
@@ -1,3 +1,20 @@
+2018-06-10  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] Array.prototype.sort should rejects null comparator
+        https://bugs.webkit.org/show_bug.cgi?id=186458
+
+        Reviewed by Keith Miller.
+
+        This relaxed behavior is once introduced in r216169 to fix some pages by aligning
+        the behavior to Chrome and Firefox.
+
+        However, now Chrome, Firefox and Edge reject a null comparator. So only JavaScriptCore
+        accepts it. This patch reverts r216169 to align JSC to the other engines and fix
+        the spec issue.
+
+        * builtins/ArrayPrototype.js:
+        (sort):
+
 2018-06-09  Dan Bernstein  <mitz@apple.com>
 
         [Xcode] Clean up and modernize some build setting definitions
index e6d4f10..8a29ed0 100644 (file)
@@ -602,7 +602,7 @@ function sort(comparator)
 
     if (typeof comparator == "function")
         comparatorSort(array, length, comparator);
-    else if (comparator === null || comparator === @undefined)
+    else if (comparator === @undefined)
         stringSort(array, length);
     else
         @throwTypeError("Array.prototype.sort requires the comparsion function be a function or undefined");