Skip to content

Commit 0434134

Browse files
rhuanjlrhuanjl
committed
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
1 parent b4f1bb4 commit 0434134

File tree

3 files changed

+65
-73
lines changed

3 files changed

+65
-73
lines changed

‎lib/Runtime/Library/InJavascript/Array_prototype.js‎

Lines changed: 31 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3-
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
3+
// Copyright (c) ChakraCore Project Contributors. All rights reserved.
44
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
55
//-------------------------------------------------------------------------------------------------------
66

@@ -230,30 +230,6 @@
230230
return `${left}` < `${right}` ? -1 : 0;
231231
});
232232

233-
platform.registerChakraLibraryFunction("FillArrayHoles", function(array, length, offset) {
234-
let i = offset, j = offset, holes = 0;
235-
let value;
236-
while (i < length) {
237-
value = array[i];
238-
if (value !== undefined) {
239-
array[j++] = value;
240-
} else if (!(i in array)) {
241-
++holes;
242-
}
243-
++i;
244-
}
245-
246-
const valuesLength = j;
247-
const hasLength = length - holes;
248-
while (j < hasLength) {
249-
array[j++] = undefined;
250-
}
251-
while (j < length) {
252-
delete array[j++];
253-
}
254-
return valuesLength;
255-
});
256-
257233
platform.registerFunction('sort', function (compareFn) {
258234
//#sec-array.prototype.sort
259235
if (compareFn !== undefined) {
@@ -265,32 +241,32 @@
265241
}
266242

267243
const {o, len} = __chakraLibrary.CheckArrayAndGetLen(this, "Array.prototype.sort");
268-
269-
if (len < 2) { // early return if length < 2
270-
return o;
271-
}
244+
const buffer = [];
245+
buffer.__proto__ = null;
272246

273247
// check for if the array has any missing values
274248
// also pull in any values from the prototype
275-
let i = 0, length = len;
276-
while (i < len) {
277-
if (o[i] === undefined) {
278-
length = __chakraLibrary.FillArrayHoles(o, len, i);
279-
break;
249+
let length = 0, undefinedCount = 0;
250+
for (let i = 0; i < len; ++i) {
251+
if (i in o) {
252+
const temp = o[i];
253+
if (temp !== undefined) {
254+
buffer[length++] = o[i];
255+
} else {
256+
++undefinedCount;
257+
}
280258
}
281-
o[i] = o[i++];
282259
}
283260

284-
// for short arrays perform an insertion sort
285261
if (length < 2048) {
286262
let sortedCount = 1, lowerBound = 0, insertPoint = 0, upperBound = 0;
287263
while (sortedCount < length) {
288-
const item = o[sortedCount];
264+
const item = buffer[sortedCount];
289265
upperBound = sortedCount;
290266
insertPoint = sortedCount - 1; // this lets us check for already ordered first
291267
lowerBound = 0;
292268
for (;;) {
293-
if (compareFn (item, o[insertPoint]) < 0) {
269+
if (compareFn (item, buffer[insertPoint]) < 0) {
294270
upperBound = insertPoint;
295271
} else {
296272
lowerBound = insertPoint + 1;
@@ -302,13 +278,27 @@
302278
}
303279
insertPoint = sortedCount;
304280
while (insertPoint > lowerBound) {
305-
o[insertPoint--] = o[insertPoint];
281+
buffer[insertPoint--] = buffer[insertPoint];
306282
}
307-
o[lowerBound] = item;
283+
buffer[lowerBound] = item;
308284
++sortedCount;
309285
}
310286
} else {
311-
__chakraLibrary.MergeSort(o, length, compareFn);
287+
__chakraLibrary.MergeSort(buffer, length, compareFn);
288+
}
289+
290+
let i = 0;
291+
for (; i < length; ++i)
292+
{
293+
o[i] = buffer[i];
294+
}
295+
for (let j = 0; j < undefinedCount; ++j)
296+
{
297+
o[i++] = undefined;
298+
}
299+
for (; i < len; ++i)
300+
{
301+
delete o[i];
312302
}
313303

314304
return o;

‎test/Array/array_sort.js‎

Lines changed: 31 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
//-------------------------------------------------------------------------------------------------------
22
// Copyright (C) Microsoft. All rights reserved.
3+
// Copyright (c) ChakraCore Project Contributors. All rights reserved.
34
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
45
//-------------------------------------------------------------------------------------------------------
56

@@ -134,6 +135,35 @@ const tests = [
134135
}
135136
},
136137
{
138+
name : "Array.prototype.sort with edited prototypes and compare function side effects",
139+
body () {
140+
const arrayOne = new Array(2);
141+
arrayOne[0] = 12;
142+
arrayOne[1] = 10;
143+
const resultOne = [10, 12];
144+
compareSparseArrays(resultOne, arrayOne.sort((x,y) => { arrayOne[0] = "test"; return x - y; }), "Compare function set element does not effect array");
145+
compareSparseArrays(resultOne, arrayOne, "Compare function side effect does not effect array");
146+
147+
const arrayTwo = new Array(3);
148+
arrayTwo[0] = 12;
149+
arrayTwo[2] = 10;
150+
const resultTwo = [10,12,,];
151+
compareSparseArrays(resultTwo, arrayTwo.sort((x, y) => { delete arrayTwo[0]; return x - y; }), "Compare function delete element does not effect array");
152+
compareSparseArrays(resultTwo, arrayTwo, "Compare function delete element does not effect array");
153+
}
154+
},
155+
{
156+
name : "Array.prototype.sort default comparison should not call valueOf",
157+
body () {
158+
valueOf = false;
159+
const arr = [{
160+
valueOf() { valueOf = true; return 0; }
161+
}, 1, 1, 1,];
162+
arr.sort();
163+
assert.isFalse(valueOf);
164+
}
165+
},
166+
{ // NOTE this test edits the array prototype and hence must be placed after other tests
137167
name : "Array.prototype.sort prototype lookups",
138168
body () {
139169
for (let i = 0; i < 20; i = i+4) {
@@ -196,6 +226,7 @@ const tests = [
196226
},
197227
{
198228
//Test that bug 5719 is fixed https://github.com/Microsoft/ChakraCore/issues/5719
229+
//NOTE this test depends upon the prototype editing done in the previous test
199230
name : "Array-like object does not pull values from Array.prototype",
200231
body () {
201232
const arrayLike = {
@@ -205,35 +236,6 @@ const tests = [
205236
assert.areEqual("o0", arrayLike[0], "Array.prototype.sort pulls values form Object.prototype for array-like object");
206237
assert.areNotEqual("p3", arrayLike[1], "Array.prototype.sort does not pull values form Array.prototype for array-like object");
207238
}
208-
},
209-
{
210-
name : "Array.prototype.sort with edited prototypes and compare function side effects",
211-
body () {
212-
const arrayOne = new Array(2);
213-
arrayOne[0] = 12;
214-
arrayOne[1] = 10;
215-
const resultOne = [10, "test"];
216-
compareSparseArrays(resultOne, arrayOne.sort((x,y) => { arrayOne[0] = "test"; return x - y; }), "Compare function set element effects array");
217-
compareSparseArrays(resultOne, arrayOne, "Compare function side effect effects array");
218-
219-
const arrayTwo = new Array(3);
220-
arrayTwo[0] = 12;
221-
arrayTwo[2] = 10;
222-
const resultTwo = [0,0,10];
223-
compareSparseArrays(resultTwo, arrayTwo.sort((x, y) => { delete arrayTwo[0]; return x - y; }), "Compare function delete element effects array");
224-
compareSparseArrays(resultTwo, arrayTwo, "Compare function delete element effects array");
225-
}
226-
},
227-
{
228-
name : "Array.prototype.sort default comparison should not call valueOf",
229-
body () {
230-
valueOf = false;
231-
const arr = [{
232-
valueOf() { valueOf = true; return 0; }
233-
}, 1, 1, 1,];
234-
arr.sort();
235-
assert.isFalse(valueOf);
236-
}
237239
}
238240
];
239241

‎tools/regenByteCode.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
#!/usr/bin/env python
22
#-------------------------------------------------------------------------------------------------------
33
# Copyright (C) Microsoft. All rights reserved.
4-
# Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
4+
# Copyright (c) ChakraCore Project Contributors. All rights reserved.
55
# Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
66
#-------------------------------------------------------------------------------------------------------
77

@@ -116,7 +116,7 @@ def run_sub(message, commands, error):
116116
# this header text will be placed at the top of the generated files
117117
header_text = '''//-------------------------------------------------------------------------------------------------------
118118
// Copyright (C) Microsoft. All rights reserved.
119-
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
119+
// Copyright (c) ChakraCore Project Contributors. All rights reserved.
120120
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
121121
//-------------------------------------------------------------------------------------------------------
122122
@@ -263,7 +263,7 @@ def bytecode_job(out_path, command, in_path, error):
263263

264264
output_str = '''//-------------------------------------------------------------------------------------------------------
265265
// Copyright (C) Microsoft. All rights reserved.
266-
// Copyright (c) 2021 ChakraCore Project Contributors. All rights reserved.
266+
// Copyright (c) ChakraCore Project Contributors. All rights reserved.
267267
// Licensed under the MIT license. See LICENSE.txt file in the project root for full license information.
268268
//-------------------------------------------------------------------------------------------------------
269269
// NOTE: If there is a merge conflict the correct fix is to make a new GUID.

0 commit comments

Comments
 (0)