Attachment 346811 Details for Bug 188382 – Patch

[patch] Patch

bug-188382-20180808190446.patch (text/plain), 12.69 KB, created by Keith Miller on 2018-08-08 19:04:47 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Keith Miller

Created: 2018-08-08 19:04:47 PDT

Size: 12.69 KB

patch

obsolete

>Subversion Revision: 234620
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index f9e61d61e6cc925044f863bc4e228dd6951570f9..8a67b5f64847889eac4ab10834ed8d98de0b3a93 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-04  David Kilzer  <ddkilzer@apple.com>
> 
>         REGRESSION (r208953): TemplateObjectDescriptor constructor calculates m_hash on use-after-move variable
>diff --git a/Source/JavaScriptCore/builtins/ArrayPrototype.js b/Source/JavaScriptCore/builtins/ArrayPrototype.js
>index 8a29ed0287257fc47715ac2b6142d68bc079e5a7..a8796540b43b98c8035a4cc67239e4885bd12618 100644
>--- a/Source/JavaScriptCore/builtins/ArrayPrototype.js
>+++ b/Source/JavaScriptCore/builtins/ArrayPrototype.js
>@@ -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;
> }
> 
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index 952ef3a5d4210f4fd20342bcb2c005615a3fdb08..263f3fe610e35f10f734d3fab8edb3ed58a02c5c 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        * test262/expectations.yaml:
>+
> 2018-08-01  Andy VanWagoner  <andy@vanwagoner.family>
> 
>         [INTL] Implement hourCycle in DateTimeFormat
>diff --git a/JSTests/test262/expectations.yaml b/JSTests/test262/expectations.yaml
>index 908cec025e5a4c6d9d7b65bf44f6ec0255567d66..6b8a120a91feec7be405b3e678cef3d9767ed791 100644
>--- a/JSTests/test262/expectations.yaml
>+++ b/JSTests/test262/expectations.yaml
>@@ -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'

Actions: View | Formatted Diff | Diff

Attachments on bug 188382: 346811