archives

« Bugzilla Issues Index

#3137 — 22.1.3.24 Array.prototype.sort: apply ToNumber on the result of compareFn in order to match web reality


Array.prototype.sort() is specced to have an implementation-defined behaviour if the provided comparison function does not return a number. Testing current versions of popular web browsers reveals that, in reality, all of them do a `ToNumber()` conversion on the result of `compareFn` (see Comment #1 for the testcase).

Therefore, step 17 of the SortCompare abstract operation (22.1.3.24.1 and 22.2.3.26) should be modified as follows:

17. If the argument comparefn is not undefined, then
(steps a, b, c unchanged)
d. Let vNum be ToNumber(v).
e. ReturnIfAbrupt(vNum).
f. If vNum < 0, then return -1.
g. If vNum > 0, then return 1.
h. Return +0.

Steps f-h are a further refinement (maybe not really needed), ensuring that all results > 0, resp. < 0, are considered equivalent. With the above changes, the SortCompare() operation returns either -1, 1 or +0, or an abrupt completion.


The testcase I used was the following:

var a = []
for (var i = 0; i < 5; i++) {
a[i] = Math.random()
}

function compareFn(a, b) {
console.log("compareFn called with arguments ", a, b)
var result = a - b
return { valueOf: function() {
console.log("valueOf called!");
return result
} }
}

a.sort(compareFn)


(In reply to Claude Pache from comment #1)
In order to convince oneself that implementations really do a ToNumber() and not just a ToPrimitive(), further test is needed. In the testcase of Comment #1, try to replace the following line

return result

with one of the below:

(1) return ' 000' + result + ' '

In that case, ToNumber() produces the value of `result`, and the array is correctly sorted.

(2) return result + ' f'

In that case, ToNumber() produces NaN, and the array may not be sorted. Experience shows that implementations indeed call `compareFn` a fixed number of times (4 or 8 times, depending of the implementation, for an array of length 5), and do not change the order of the elements of array.


fixed in rev34 editor's draft

Added the ToNumber

didn't add the normalization to -1, 1. That seems like an "improvement" that isn't strictly motivated trying to match current implementation behavior.


fixed in rev34