Array.sort method doesn't work when sort function is specified

[yurivict]

This code:

var l = [{a: 0.5}, {a: -0.1}, {a: 0.7}];
l.sort(function(x,y) {return x.a - y.a;});
print(JSON.stringify(l));

prints still unsorted elements:

[{"a":0.5},{"a":-0.1},{"a":0.7}]

[isRyven]

The reason is pretty simple, mujs internally uses qsort to perform sorting, and qsort essentially expects integer value to be returned from its callback, in your case, when you return a subtract result, it will be coerced to 0, so no sorting is done. To overcome this, manually return -1, 0 or 1.

l.sort(function(x, y) { return (x.a === y.a) ? 0: (x.a > y.a) ? 1 : -1; });

[yurivict]

Thanks for troubleshooting this. I see.

The same code run with node returns the correct result (when print is replaced with console.log):

$ node sort.js 
[{"a":-0.1},{"a":0.5},{"a":0.7}]

I think MuJS should do the same (first convert to 'double', or, better, at the time of compilation analyze the function if it always returns int/double/or this varies, and perform the sort accordingly).

[sgbeal]

Firefox also handles this using floating-point comparison of the results, rather than integers, and presumably it is following the spec. C's qsort() uses integers but does not do the sorting itself: it uses a comparison function which may internally use whatever comparison it likes, including floating-point.

It seems that my own scripting engine gets this wrong in the same way (and presumably for the same reason) muJS does (but i'm mine's not confined by a formal spec ;)).

[isRyven]

i'll do a pr for a quick fix

[sgbeal]

FWIW, here's a fix for my owns scripting engine which very probably looks quite close to what muJS will need (but i'm not familiar with muJS's internals):

https://fossil.wanderinghorse.net/r/cwal/info/4f0d98c3294778c6

[avih]

It seems to be a mujs bug. The ES5.1 spec says:

If comparefn is not undefined, it should be a function that accepts two arguments x and y and returns a negative value if x < y, zero if x = y, or a positive value if x > y.

This should fix it (untested):

diff --git a/jsarray.c b/jsarray.c
index b6cc343..136a487 100644
--- a/jsarray.c
+++ b/jsarray.c
@@ -259,7 +259,7 @@ static int sortcmp(const void *avoid, const void *bvoid)
        const js_Value *a = &aslot->v, *b = &bslot->v;
        js_State *J = aslot->J;
        const char *sx, *sy;
-       int c;
+       double c;

        int unx = (a->type == JS_TUNDEFINED);
        int uny = (b->type == JS_TUNDEFINED);
@@ -282,7 +282,7 @@ static int sortcmp(const void *avoid, const void *bvoid)
                c = strcmp(sx, sy);
                js_pop(J, 2);
        }
-       return c;
+       return c < 0 ? -1 : c > 0 ? 1 : 0;
 }

 static void Ap_sort(js_State *J)

[isRyven]

Well, based on spec quote this clearly is not mujs bug. It defines rules for the sort callback function, and clearly we are just trying to use it incorrectly.

[yurivict]

No, based on this quote from the standard this is clearly a bug in MuJS, because it talks about "negative value" and "positive value" expectation, and MuJS first rounds it to the integer which it shouldn't do.

[isRyven]

Good point.