How about a regression test? I realize we found it via a regression test already, but it was for another bug. Seems like maybe it would be good to have a test for incorrect custom sort comparators?
On Fri, Sep 26, 2008 at 12:08 PM, Christian Plesner Hansen <[EMAIL PROTECTED]> wrote: > > Tak så mykket! > > On Fri, Sep 26, 2008 at 11:56 AM, <[EMAIL PROTECTED]> wrote: >> Reviewers: christian.plesner.hansen, >> >> Description: >> Adding comment to the fix of issue 95. >> >> Please review this at http://codereview.chromium.org/5003 >> >> Affected files: >> M src/array.js >> >> >> Index: src/array.js >> =================================================================== >> --- src/array.js (revision 382) >> +++ src/array.js (working copy) >> @@ -672,6 +672,8 @@ >> if (from >= to - 1) return; >> var pivot_index = $floor($random() * (to - from)) + from; >> var pivot = a[pivot_index]; >> + // Issue 95: Keep the pivot element out of the comparisons to avoid >> + // infinite recursion if comparefn(pivot, pivot) != 0. >> a[pivot_index] = a[to - 1]; >> a[to - 1] = pivot; >> var low_end = from; // Upper bound of the elements lower than pivot. >> @@ -692,6 +694,7 @@ >> i++; >> } >> } >> + // Restore the pivot element to its rightful place. >> a[to - 1] = a[high_start]; >> a[high_start] = pivot; >> high_start++; >> >> >> > > > > --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
