Array.prototype.sort spec compliance

[ShortDevelopment]

The following poc outputs different results in CC and V8.

const array = [40, 30, 10];
array.sort((x, y) => { array[0] = -1; console.log(array); return x - y; });
console.log(array);

CC

-1,30,10
-1,-1,10
-1,-1,10

V8

[ -1, 30, 10 ]
[ -1, 30, 10 ]
[ 10, 30, 40 ]

That's strange, as there's a test case that enforces this behavior in CC:

ChakraCore/test/Array/array_sort.js

Lines 209 to 226 in 7bcdec7

	{
	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");
	}
	},

[rhuanjl]

I've had a look:The language specification was changed here: tc39/ecma262#1585

v8 is following the up to date spec, ChakraCore is in line with the out of date spec.

[ShortDevelopment]

Does this mean that the compare function works with the untouched original array, while the (native) sort function itself uses its own copy?

[rhuanjl]

Does this mean that the compare function works with the untouched original array, while the (native) sort function itself uses its own copy?

I don't think so, I think it's meant to:

1. copy the array (one element at a time, skipping undefined and empty values but counting them)
2. sort the copy
3. write it back over the original (one element at a time then write the correct number of undefined and empty values to the end)

I'll probably have a go at updating our sort this weekend unless you'd like to do it.

Note if you work on this, for array.prototype.sort the actual sort we use is written in Javascript see Runtime/Library/InJavascript/array_prototype.js (I think typedarrays use something else though - but haven't looked at that in a while) If you update that javascript file you'll need to run the python script tools/regenbytecode.py to incorporate the changes before re-building ChakaCore.

[rhuanjl]

I've drafted an implementation for this change for Array.prototype.sort - but it currently brings a measurable performance penalty so I'm going to look at alternate ways of doing it.

[ShortDevelopment]

I don’t think I’m able to fix this yet, but I’ll have a look at the code and your fix to better understand the engine (JsBuildIns) and how to work with the spec.

By the way: What about JavascriptArray::EntrySort? I never got a breakpoint hit, but it's still an implementation...

[rhuanjl]

JavascriptArray::EntrySort is the old no longer used sort implementation, it is a fall back that is used if you disable the self-hosted javascript which can be done in Test/Debug builds with the runtime ch flag -JsBuiltin-.

It is out of line with the specification on more points than the javascript implementation we're using.

[rhuanjl]

@ShortDevelopment I've put up the PR, the substance of the change is just the array_prototype.js file, the larger diffs in the headers are generated by running the regenBytecode.py script. (They are a serialised form of the JS in array_prototype.js)