Update Array Sort for revised spec which requires:

- cloning the array and using Has property on every element
- sorting the clone
- writing back over the top of the original

Author: rhuanjl

lib/Runtime/Library/InJavascript/Array_prototype.js

@@ -1,6 +1,6 @@
 //-------------------------------------------------------------------------------------------------------
 // Copyright (C) Microsoft. All rights reserved.
-// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
+// Copyright (c) ChakraCore Project Contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 
@@ -230,30 +230,6 @@
         return `${left}` < `${right}` ? -1 : 0;
     });
 
-    platform.registerChakraLibraryFunction("FillArrayHoles", function(array, length, offset) {
-        let i = offset, j = offset, holes = 0;
-        let value;
-        while (i < length) {
-            value = array[i];
-            if (value !== undefined) {
-                array[j++] = value;
-            } else if (!(i in array)) {
-                ++holes;
-            }
-            ++i;
-        }
-
-        const valuesLength = j;
-        const hasLength = length - holes;
-        while (j < hasLength) {
-            array[j++] = undefined;
-        }
-        while (j < length) {
-            delete array[j++];
-        }
-        return valuesLength;
-    });
-
     platform.registerFunction('sort', function (compareFn) {
         //#sec-array.prototype.sort
         if (compareFn !== undefined) {
@@ -265,32 +241,32 @@
         }
 
         const {o, len} = __chakraLibrary.CheckArrayAndGetLen(this, "Array.prototype.sort");
-
-        if (len < 2) { // early return if length < 2
-            return o;
-        }
+        const buffer = [];
+        buffer.__proto__ = null;
 
         // check for if the array has any missing values
         // also pull in any values from the prototype
-        let i = 0, length = len;
-        while (i < len) {
-            if (o[i] === undefined) {
-                length = __chakraLibrary.FillArrayHoles(o, len, i);
-                break;
+        let length = 0, undefinedCount = 0;
+        for (let i = 0; i < len; ++i) {
+            if (i in o) {
+                const temp = o[i];
+                if (temp !== undefined) {
+                    buffer[length++] = o[i];
+                } else {
+                    ++undefinedCount;
+                }
             }
-            o[i] = o[i++];
         }
 
-        // for short arrays perform an insertion sort
         if (length < 2048) {
             let sortedCount = 1, lowerBound = 0, insertPoint = 0, upperBound = 0;
             while (sortedCount < length) {
-                const item = o[sortedCount];
+                const item = buffer[sortedCount];
                 upperBound = sortedCount;
                 insertPoint = sortedCount - 1; // this lets us check for already ordered first
                 lowerBound = 0;
                 for (;;) {
-                    if (compareFn (item, o[insertPoint]) < 0) {
+                    if (compareFn (item, buffer[insertPoint]) < 0) {
                         upperBound = insertPoint;
                     } else {
                         lowerBound = insertPoint + 1;
@@ -302,13 +278,27 @@
                 }
                 insertPoint = sortedCount;
                 while (insertPoint > lowerBound) {
-                    o[insertPoint--] = o[insertPoint];
+                    buffer[insertPoint--] = buffer[insertPoint];
                 }
-                o[lowerBound] = item;
+                buffer[lowerBound] = item;
                 ++sortedCount;
             }
         } else {
-            __chakraLibrary.MergeSort(o, length, compareFn);
+            __chakraLibrary.MergeSort(buffer, length, compareFn);
+        }
+
+        let i = 0;
+        for (; i < length; ++i)
+        {
+            o[i] = buffer[i];
+        }
+        for (let j = 0; j < undefinedCount; ++j)
+        {
+            o[i++] = undefined;
+        }
+        for (; i < len; ++i)
+        {
+            delete o[i];
         }
 
         return o;

test/Array/array_sort.js

@@ -1,5 +1,6 @@
 //-------------------------------------------------------------------------------------------------------
 // Copyright (C) Microsoft. All rights reserved.
+// Copyright (c) ChakraCore Project Contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 
@@ -134,6 +135,35 @@ const tests = [
         }
     },
     {
+        name : "Array.prototype.sort with edited prototypes and compare function side effects",
+        body () {
+            const arrayOne = new Array(2);
+            arrayOne[0] = 12;
+            arrayOne[1] = 10;
+            const resultOne = [10, 12];
+            compareSparseArrays(resultOne, arrayOne.sort((x,y) => { arrayOne[0] = "test"; return x - y; }), "Compare function set element does not effect array");
+            compareSparseArrays(resultOne, arrayOne, "Compare function side effect does not effect array");
+
+            const arrayTwo = new Array(3);
+            arrayTwo[0] = 12;
+            arrayTwo[2] = 10;
+            const resultTwo = [10,12,,];
+            compareSparseArrays(resultTwo, arrayTwo.sort((x, y) => { delete arrayTwo[0]; return x - y; }), "Compare function delete element does not effect array");
+            compareSparseArrays(resultTwo, arrayTwo, "Compare function delete element does not effect array");
+        }
+    },
+    {
+        name : "Array.prototype.sort default comparison should not call valueOf",
+        body () {
+            valueOf = false;
+            const arr = [{
+                valueOf() { valueOf = true; return 0; }
+            }, 1, 1, 1,];
+            arr.sort();
+            assert.isFalse(valueOf);
+        }
+    },
+    { // NOTE this test edits the array prototype and hence must be placed after other tests
         name : "Array.prototype.sort prototype lookups",
         body () {
             for (let i = 0; i < 20; i = i+4) {
@@ -196,6 +226,7 @@ const tests = [
     },
     {
         //Test that bug 5719 is fixed https://github.com/Microsoft/ChakraCore/issues/5719
+        //NOTE this test depends upon the prototype editing done in the previous test
         name : "Array-like object does not pull values from Array.prototype",
         body () {
             const arrayLike = {
@@ -205,35 +236,6 @@ const tests = [
             assert.areEqual("o0", arrayLike[0], "Array.prototype.sort pulls values form Object.prototype for array-like object");
             assert.areNotEqual("p3", arrayLike[1], "Array.prototype.sort does not pull values form Array.prototype for array-like object");
         }
-    },
-    {
-        name : "Array.prototype.sort with edited prototypes and compare function side effects",
-        body () {
-            const arrayOne = new Array(2);
-            arrayOne[0] = 12;
-            arrayOne[1] = 10;
-            const resultOne = [10, "test"];
-            compareSparseArrays(resultOne, arrayOne.sort((x,y) => { arrayOne[0] = "test"; return x - y; }), "Compare function set element effects array");
-            compareSparseArrays(resultOne, arrayOne, "Compare function side effect effects array");
-
-            const arrayTwo = new Array(3);
-            arrayTwo[0] = 12;
-            arrayTwo[2] = 10;
-            const resultTwo = [0,0,10];
-            compareSparseArrays(resultTwo, arrayTwo.sort((x, y) => { delete arrayTwo[0]; return x - y; }), "Compare function delete element effects array");
-            compareSparseArrays(resultTwo, arrayTwo, "Compare function delete element effects array");
-        }
-    },
-    {
-        name : "Array.prototype.sort default comparison should not call valueOf",
-        body () {
-            valueOf = false;
-            const arr = [{
-                valueOf() { valueOf = true; return 0; }
-            }, 1, 1, 1,];
-            arr.sort();
-            assert.isFalse(valueOf);
-        }
     }
 ];
 

tools/regenByteCode.py

@@ -1,7 +1,7 @@
 #!/usr/bin/env python
 #-------------------------------------------------------------------------------------------------------
 # Copyright (C) Microsoft. All rights reserved.
-# Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
+# Copyright (c) ChakraCore Project Contributors. All rights reserved.
 # Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 #-------------------------------------------------------------------------------------------------------
 
@@ -116,7 +116,7 @@ def run_sub(message, commands, error):
 # this header text will be placed at the top of the generated files
 header_text = '''//-------------------------------------------------------------------------------------------------------
 // Copyright (C) Microsoft. All rights reserved.
-// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
+// Copyright (c) ChakraCore Project Contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 
@@ -263,7 +263,7 @@ def bytecode_job(out_path, command, in_path, error):
 
         output_str = '''//-------------------------------------------------------------------------------------------------------
 // Copyright (C) Microsoft. All rights reserved.
-// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
+// Copyright (c) ChakraCore Project Contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
 //-------------------------------------------------------------------------------------------------------
 // NOTE: If there is a merge conflict the correct fix is to make a new GUID.