jerryscript-project / jerryscript

Ultra-lightweight JavaScript engine for the Internet of Things.
https://jerryscript.net
Apache License 2.0
6.89k stars 669 forks source link

A question about Array.prototype.sort #5025

Open AidPaike opened 1 year ago

AidPaike commented 1 year ago

Version

Version: 3.0.0 ( 6fe763f )

Testcase1

var foo = function (parts) {
    parts.sort(function (a, b) {
        //print(a.avalue - b.value);
        return a.avalue - b.value;//NaN
    });
    parts.forEach(function (item) {
        print(item.value)
    })
};

var NISL008 = [
    { name: 'Edward', value: 21 },
    { name: 'Tom', value: 37 },
    { name: 'paike', value: 23 },
    { name: 'Jerry', value: -12 }
];
foo(NISL008);

Execution steps

/root/.jsvu/jerry Testcase.js

Output

-12
23
37
21

Expected behavior

21
37
23
-12

Testcase2

var foo = function (parts) {
    parts.sort(function (a, b) {
        //print(a.avalue - b.value);
        return a.avalue - b.value;//NaN
    });
    parts.forEach(function (item) {
        print(item.value)
    })
};

var NISL008 = [
    { name: 'Jerry', value: -12 },
    { name: 'Edward', value: 21 },
    { name: 'Tom', value: 37 },
    { name: 'paike', value: 23 },
];
foo(NISL008);

Output

23
37
21
-12

Expected behavior

-12
21
37
23

Description

When executing this Testcase, I mistakenly wrote a.value to a.avalue in line 4, which caused all the return values of parts.sort to become NaN. At this point, none of the other engines changed the ordering of the original array, but jerryscript seems treat NaN as +0. Reference (https://tc39.es/ecma262/#sec-array.prototype.sort)

The same bug has reported to SpiderMonkey (https://bugzilla.mozilla.org/show_bug.cgi?id=1763996) and has been fixed.

Looking forward to your reply :)

ossy-szeged commented 1 year ago

Thanks for reporting, the issue is valid. The latest spec clarifies that NaN return value of the comparefn should be changed to +0. JerryScript hasn't implemented the latest spec yet, but it seems to be easily fixable. I'm going to fix it soon.