patch that fixes an obscure problem with array.sort

Darin Adler darin at apple.com
Mon Nov 10 17:31:28 CET 2003


Here's a patch for a small array.sort bug in kjs.

In the next week or two if I can find the time I plan to make changes 
so that JavaScriptCore is much closer to kjs tip of tree. For any 
places where the two differ in a way that's irrelevant to the generated 
code, I'm going to adopt the current kjs.

-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/ChangeLog,v
retrieving revision 1.379
diff -p -u -u -p -r1.379 ChangeLog
--- ChangeLog	2003/11/06 20:05:29	1.379
+++ ChangeLog	2003/11/10 16:29:43
@@ -1,3 +1,16 @@
+2003-11-08  Darin Adler  <darin at apple.com>
+
+        Reviewed by John.
+
+        - fixed 3477528 -- array.sort(function) fails if the function returns a non-zero value that rounds to zero
+
+        * kjs/array_object.cpp:
+        (compareByStringForQSort): Added checks for undefined values to match what the specification calls for.
+        (compareWithCompareFunctionForQSort): Added checks for undefined values as above, and also changed the
+        code that looks at the compare function result to look at the number returned without rounding to an integer.
+        (ArrayProtoFuncImp::call): Changed the code that looks at the compare function result to look at the number
+        returned without rounding to an integer.
+
 === Safari-113 ===
 
 2003-11-03  Vicki Murley <vicki at apple.com>
Index: kjs/array_object.cpp
===================================================================
RCS file: /local/home/cvs/Labyrinth/JavaScriptCore/kjs/array_object.cpp,v
retrieving revision 1.29
diff -p -u -u -p -r1.29 kjs/array_object.cpp
--- kjs/array_object.cpp	2003/08/08 15:21:12	1.29
+++ kjs/array_object.cpp	2003/11/10 16:29:44
@@ -279,7 +279,15 @@ static ExecState *execForCompareByString
 static int compareByStringForQSort(const void *a, const void *b)
 {
     ExecState *exec = execForCompareByStringForQSort;
-    return compare(Value(*(ValueImp **)a).toString(exec), Value(*(ValueImp **)b).toString(exec));
+    ValueImp *va = *(ValueImp **)a;
+    ValueImp *vb = *(ValueImp **)b;
+    if (va->dispatchType() == UndefinedType) {
+        return vb->dispatchType() == UndefinedType ? 0 : 1;
+    }
+    if (vb->dispatchType() == UndefinedType) {
+        return -1;
+    }
+    return compare(va->dispatchToString(exec), vb->dispatchToString(exec));
 }
 
 void ArrayInstanceImp::sort(ExecState *exec)
@@ -312,12 +320,22 @@ static CompareWithCompareFunctionArgumen
 static int compareWithCompareFunctionForQSort(const void *a, const void *b)
 {
     CompareWithCompareFunctionArguments *args = compareWithCompareFunctionArguments;
-    
+
+    ValueImp *va = *(ValueImp **)a;
+    ValueImp *vb = *(ValueImp **)b;
+    if (va->dispatchType() == UndefinedType) {
+        return vb->dispatchType() == UndefinedType ? 0 : 1;
+    }
+    if (vb->dispatchType() == UndefinedType) {
+        return -1;
+    }
+
     args->arguments.clear();
-    args->arguments.append(*(ValueImp **)a);
-    args->arguments.append(*(ValueImp **)b);
-    return args->compareFunction->call(args->exec, args->globalObject, args->arguments)
-        .toInt32(args->exec);
+    args->arguments.append(va);
+    args->arguments.append(vb);
+    double compareResult = args->compareFunction->call
+        (args->exec, args->globalObject, args->arguments).toNumber(args->exec);
+    return compareResult < 0 ? -1 : compareResult > 0 ? 1 : 0;
 }
 
 void ArrayInstanceImp::sort(ExecState *exec, Object &compareFunction)
@@ -625,16 +643,16 @@ Value ArrayProtoFuncImp::call(ExecState 
         for ( unsigned int j = i+1 ; j<length ; ++j )
           {
             Value jObj = thisObj.get(exec,j);
-            int cmp;
+            double cmp;
             if (jObj.type() == UndefinedType) {
-              cmp = 1;
+              cmp = 1; // don't check minObj because there's no need to differentiate == (0) from > (1)
             } else if (minObj.type() == UndefinedType) {
               cmp = -1;
             } else if (useSortFunction) {
                 List l;
                 l.append(jObj);
                 l.append(minObj);
-                cmp = sortFunction.call(exec, exec->interpreter()->globalObject(), l).toInt32(exec);
+                cmp = sortFunction.call(exec, exec->interpreter()->globalObject(), l).toNumber(exec);
             } else {
               cmp = (jObj.toString(exec) < minObj.toString(exec)) ? -1 : 1;
             }
-------------- next part --------------


     -- Darin


More information about the Khtml-devel mailing list