Array.prototype.sort should throw TypeError if param is a not callable object
authorkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2018 03:13:20 +0000 (03:13 +0000)
committerkeith_miller@apple.com <keith_miller@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 9 Aug 2018 03:13:20 +0000 (03:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=188382

Reviewed by Saam Barati.

JSTests:

* test262/expectations.yaml:

Source/JavaScriptCore:

Improve spec compatability by checking if the Array.prototype.sort comparator is a function
before doing anything else.

Also, refactor the various helper functions to use let instead of var.

* builtins/ArrayPrototype.js:
(sort.stringComparator):
(sort.compactSparse):
(sort.compactSlow):
(sort.compact):
(sort.merge):
(sort.mergeSort):
(sort.bucketSort):
(sort.comparatorSort):
(sort.stringSort):
(sort):

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

JSTests/ChangeLog
JSTests/test262/expectations.yaml
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/ArrayPrototype.js

index 952ef3a..9b3e59c 100644 (file)
@@ -1,3 +1,12 @@
+2018-08-08  Keith Miller  <keith_miller@apple.com>
+
+        Array.prototype.sort should throw TypeError if param is a not callable object
+        https://bugs.webkit.org/show_bug.cgi?id=188382
+
+        Reviewed by Saam Barati.
+
+        * test262/expectations.yaml:
+
 2018-08-01  Andy VanWagoner  <andy@vanwagoner.family>
 
         [INTL] Implement hourCycle in DateTimeFormat
index 908cec0..6b8a120 100644 (file)
@@ -672,9 +672,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 2a81f79..47b02f0 100644 (file)
@@ -1,3 +1,27 @@
+2018-08-08  Keith Miller  <keith_miller@apple.com>
+
+        Array.prototype.sort should throw TypeError if param is a not callable object
+        https://bugs.webkit.org/show_bug.cgi?id=188382
+
+        Reviewed by Saam Barati.
+
+        Improve spec compatability by checking if the Array.prototype.sort comparator is a function
+        before doing anything else.
+
+        Also, refactor the various helper functions to use let instead of var.
+
+        * builtins/ArrayPrototype.js:
+        (sort.stringComparator):
+        (sort.compactSparse):
+        (sort.compactSlow):
+        (sort.compact):
+        (sort.merge):
+        (sort.mergeSort):
+        (sort.bucketSort):
+        (sort.comparatorSort):
+        (sort.stringSort):
+        (sort):
+
 2018-08-08  Michael Saboff  <msaboff@apple.com>
 
         Yarr JIT should include annotations with dumpDisassembly=true
index 8a29ed0..a879654 100644 (file)
@@ -391,16 +391,16 @@ function sort(comparator)
 
     function stringComparator(a, b)
     {
-        var aString = a.string;
-        var bString = b.string;
+        let aString = a.string;
+        let bString = b.string;
 
-        var aLength = aString.length;
-        var bLength = bString.length;
-        var length = min(aLength, bLength);
+        let aLength = aString.length;
+        let bLength = bString.length;
+        let length = min(aLength, bLength);
 
-        for (var i = 0; i < length; ++i) {
-            var aCharCode = aString.@charCodeAt(i);
-            var bCharCode = bString.@charCodeAt(i);
+        for (let i = 0; i < length; ++i) {
+            let aCharCode = aString.@charCodeAt(i);
+            let bCharCode = bString.@charCodeAt(i);
 
             if (aCharCode == bCharCode)
                 continue;
@@ -414,25 +414,25 @@ function sort(comparator)
     // Move undefineds and holes to the end of a sparse array. Result is [values..., undefineds..., holes...].
     function compactSparse(array, dst, src, length)
     {
-        var values = [ ];
-        var seen = { };
-        var valueCount = 0;
-        var undefinedCount = 0;
+        let values = [ ];
+        let seen = { };
+        let valueCount = 0;
+        let undefinedCount = 0;
 
         // Clean up after the in-progress non-sparse compaction that failed.
-        for (var i = dst; i < src; ++i)
+        for (let i = dst; i < src; ++i)
             delete array[i];
 
-        for (var object = array; object; object = @Object.@getPrototypeOf(object)) {
-            var propertyNames = @Object.@getOwnPropertyNames(object);
-            for (var i = 0; i < propertyNames.length; ++i) {
-                var index = propertyNames[i];
+        for (let object = array; object; object = @Object.@getPrototypeOf(object)) {
+            let propertyNames = @Object.@getOwnPropertyNames(object);
+            for (let i = 0; i < propertyNames.length; ++i) {
+                let index = propertyNames[i];
                 if (index < length) { // Exclude non-numeric properties and properties past length.
                     if (seen[index]) // Exclude duplicates.
                         continue;
                     seen[index] = 1;
 
-                    var value = array[index];
+                    let value = array[index];
                     delete array[index];
 
                     if (value === @undefined) {
@@ -445,7 +445,7 @@ function sort(comparator)
             }
         }
 
-        for (var i = valueCount; i < valueCount + undefinedCount; ++i)
+        for (let i = valueCount; i < valueCount + undefinedCount; ++i)
             array[i] = @undefined;
 
         return valueCount;
@@ -453,9 +453,11 @@ function sort(comparator)
 
     function compactSlow(array, length)
     {
-        var holeCount = 0;
+        let holeCount = 0;
 
-        for (var dst = 0, src = 0; src < length; ++src) {
+        let dst = 0;
+        let src = 0;
+        for (; src < length; ++src) {
             if (!(src in array)) {
                 ++holeCount;
                 if (holeCount < 256)
@@ -463,20 +465,20 @@ function sort(comparator)
                 return compactSparse(array, dst, src, length);
             }
 
-            var value = array[src];
+            let value = array[src];
             if (value === @undefined)
                 continue;
 
             array[dst++] = value;
         }
 
-        var valueCount = dst;
-        var undefinedCount = length - valueCount - holeCount;
+        let valueCount = dst;
+        let undefinedCount = length - valueCount - holeCount;
 
-        for (var i = valueCount; i < valueCount + undefinedCount; ++i)
+        for (let i = valueCount; i < valueCount + undefinedCount; ++i)
             array[i] = @undefined;
 
-        for (var i = valueCount + undefinedCount; i < length; ++i)
+        for (let i = valueCount + undefinedCount; i < length; ++i)
             delete array[i];
 
         return valueCount;
@@ -485,7 +487,7 @@ function sort(comparator)
     // Move undefineds and holes to the end of an array. Result is [values..., undefineds..., holes...].
     function compact(array, length)
     {
-        for (var i = 0; i < array.length; ++i) {
+        for (let i = 0; i < array.length; ++i) {
             if (array[i] === @undefined)
                 return compactSlow(array, length);
         }
@@ -495,12 +497,12 @@ function sort(comparator)
 
     function merge(dst, src, srcIndex, srcEnd, width, comparator)
     {
-        var left = srcIndex;
-        var leftEnd = min(left + width, srcEnd);
-        var right = leftEnd;
-        var rightEnd = min(right + width, srcEnd);
+        let left = srcIndex;
+        let leftEnd = min(left + width, srcEnd);
+        let right = leftEnd;
+        let rightEnd = min(right + width, srcEnd);
 
-        for (var dstIndex = left; dstIndex < rightEnd; ++dstIndex) {
+        for (let dstIndex = left; dstIndex < rightEnd; ++dstIndex) {
             if (right < rightEnd) {
                 if (left >= leftEnd) {
                     dst[dstIndex] = src[right++];
@@ -521,22 +523,22 @@ function sort(comparator)
 
     function mergeSort(array, valueCount, comparator)
     {
-        var buffer = [ ];
+        let buffer = [ ];
         buffer.length = valueCount;
 
-        var dst = buffer;
-        var src = array;
-        for (var width = 1; width < valueCount; width *= 2) {
-            for (var srcIndex = 0; srcIndex < valueCount; srcIndex += 2 * width)
+        let dst = buffer;
+        let src = array;
+        for (let width = 1; width < valueCount; width *= 2) {
+            for (let srcIndex = 0; srcIndex < valueCount; srcIndex += 2 * width)
                 merge(dst, src, srcIndex, valueCount, width, comparator);
 
-            var tmp = src;
+            let tmp = src;
             src = dst;
             dst = tmp;
         }
 
         if (src != array) {
-            for(var i = 0; i < valueCount; i++)
+            for(let i = 0; i < valueCount; i++)
                 array[i] = src[i];
         }
     }
@@ -545,27 +547,27 @@ function sort(comparator)
     {
         if (bucket.length < 32 || depth > 32) {
             mergeSort(bucket, bucket.length, stringComparator);
-            for (var i = 0; i < bucket.length; ++i)
+            for (let i = 0; i < bucket.length; ++i)
                 array[dst++] = bucket[i].value;
             return dst;
         }
 
-        var buckets = [ ];
-        for (var i = 0; i < bucket.length; ++i) {
-            var entry = bucket[i];
-            var string = entry.string;
+        let buckets = [ ];
+        for (let i = 0; i < bucket.length; ++i) {
+            let entry = bucket[i];
+            let string = entry.string;
             if (string.length == depth) {
                 array[dst++] = entry.value;
                 continue;
             }
 
-            var c = string.@charCodeAt(depth);
+            let c = string.@charCodeAt(depth);
             if (!buckets[c])
                 buckets[c] = [ ];
             buckets[c][buckets[c].length] = entry;
         }
 
-        for (var i = 0; i < buckets.length; ++i) {
+        for (let i = 0; i < buckets.length; ++i) {
             if (!buckets[i])
                 continue;
             dst = bucketSort(array, dst, buckets[i], depth + 1);
@@ -576,37 +578,39 @@ function sort(comparator)
 
     function comparatorSort(array, length, comparator)
     {
-        var valueCount = compact(array, length);
+        let valueCount = compact(array, length);
         mergeSort(array, valueCount, comparator);
     }
 
     function stringSort(array, length)
     {
-        var valueCount = compact(array, length);
+        let valueCount = compact(array, length);
 
-        var strings = @newArrayWithSize(valueCount);
-        for (var i = 0; i < valueCount; ++i)
+        let strings = @newArrayWithSize(valueCount);
+        for (let i = 0; i < valueCount; ++i)
             strings[i] = { string: @toString(array[i]), value: array[i] };
 
         bucketSort(array, 0, strings, 0);
     }
 
-    var array = @toObject(this, "Array.prototype.sort requires that |this| not be null or undefined");
+    let sortFunction;
+    if (typeof comparator == "function")
+        sortFunction = comparatorSort;
+    else if (comparator === @undefined)
+        sortFunction = stringSort;
+    else
+        @throwTypeError("Array.prototype.sort requires the comparsion function be a function or undefined");
+
+    let array = @toObject(this, "Array.prototype.sort requires that |this| not be null or undefined");
 
-    var length = array.length >>> 0;
+    let length = array.length >>> 0;
 
     // For compatibility with Firefox and Chrome, do nothing observable
     // to the target array if it has 0 or 1 sortable properties.
     if (length < 2)
         return array;
 
-    if (typeof comparator == "function")
-        comparatorSort(array, length, comparator);
-    else if (comparator === @undefined)
-        stringSort(array, length);
-    else
-        @throwTypeError("Array.prototype.sort requires the comparsion function be a function or undefined");
-
+    sortFunction(array, length, comparator);
     return array;
 }