-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Array.prototype.sort
spec compliance
#6843
Comments
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. |
Array.prototype.sort
(Closure)Array.prototype.sort
spec compliance
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:
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 |
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. |
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 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 It is out of line with the specification on more points than the javascript implementation we're using. |
@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) |
The following poc outputs different results in CC and V8.
CC
V8
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
The text was updated successfully, but these errors were encountered: