archives

« Bugzilla Issues Index

#596 — Coverage: 15.4.4.11 - identical elements and array holes


Two additional test cases for Array.prototype.sort [15.4.4.11]:

1) The SortCompare abstract operation calls ToString() for identical elements (step 14/15)
2) Array.prototype.sort does not change non-existent elements to undefined elements, that means holes are preserved (cf. spec text about [[Delete]] and sparse arrays)


test case for (1):
---
var counter = 0;
var object = {toString: function(){ counter++; return ""; }};
[object, object].sort();
if (counter < 2) {
// sort calls ToString() for each element at least once
$ERROR("...");
}
---


test case for (2):
---
var array = ['a',,void 0];

if (array.length !== 3) { $ERROR("...") }
if (array.hasOwnProperty('0') !== true) { $ERROR("..."); }
if (array.hasOwnProperty('1') !== false) { $ERROR("..."); }
if (array.hasOwnProperty('2') !== true) { $ERROR("..."); }

array.sort();

if (array.length !== 3) { $ERROR("...") }
if (array.hasOwnProperty('0') !== true) { $ERROR("..."); }
if (array.hasOwnProperty('1') !== true) { $ERROR("..."); }
if (array.hasOwnProperty('2') !== false) { $ERROR("..."); }
---


I think the first test (bug_596_1) is bogus. The spec allows an "arbitrary sequence of calls" to SortCompare. In particular, it is valid to not call it at all, as long as the two given conditions are met for the sorted result. In this particular case, that happens to be possible without ever calling SortCompare (and thus toString). So count===0 is a perfectly legal result. In fact, doing reference comparison before ToString is an important optimisation.

(Arguably, you could check that toString is called an _even_ number of times, though.)

However, there clearly is a spec issue here. I filed it separately:
https://bugs.ecmascript.org/show_bug.cgi?id=3150


(In reply to Andreas Rossberg from comment #1)
> I think the first test (bug_596_1) is bogus. The spec allows an "arbitrary
> sequence of calls" to SortCompare. In particular, it is valid to not call it
> at all, as long as the two given conditions are met for the sorted result.

My interpretation of the algorithm is that SortCompare is called for every comparison operation during the sort. That includes comparing identical elements which means observable side-effects in ToString are possible. But the spec text is fuzzy enough to allow different interpretations, so I guess removing "test/suite/es6/bug_596_1.js" is acceptable.


This issue seems to be fixed. The new contents look valid to me per spec.

----

var counter = 0;
var object = {
toString: function(){
counter++;
return "";
}
};

[object, object].sort();
if (counter < 2) {
// sort calls ToString() for each element at least once
$ERROR('#1: [object, object].sort(); counter < 22. Actual: ' + (counter));
}