Reviewers: Christian Plesner Hansen, Description: Fix the order in which ToNumber is called for some Math functions. Avoid divisions when doing Math.min(0, 0).
Please review this at http://codereview.chromium.org/149188 SVN Base: http://v8.googlecode.com/svn/branches/bleeding_edge/ Affected files: M src/math.js A test/mjsunit/to_number_order.js Index: test/mjsunit/to_number_order.js =================================================================== --- test/mjsunit/to_number_order.js (revision 0) +++ test/mjsunit/to_number_order.js (revision 0) @@ -0,0 +1,24 @@ +var x = ""; +var v = new Object(); +var w = new Object(); +var vv = function() { x += "hest"; return 1; } +var ww = function() { x += "fisk"; return 2; } +v.valueOf = vv; +w.valueOf = ww; +assertEquals(1, Math.min(v,w)); +assertEquals("hestfisk", x, "min"); + +x = ""; +assertEquals(2, Math.max(v,w)); +assertEquals("hestfisk", x, "max"); + +x = ""; +assertEquals(Math.atan2(1, 2), Math.atan2(v, w)); +// Hestfisk is correct. We match JSC in evaluating in the other order. +assertEquals("fiskhest", x, "atan2"); + +x = ""; +assertEquals(1, Math.pow(v, w)); +assertEquals("hestfisk", x, "pow"); + +print("ok"); Index: src/math.js =================================================================== --- src/math.js (revision 2356) +++ src/math.js (working copy) @@ -68,10 +68,12 @@ } // ECMA 262 - 15.8.2.5 -function MathAtan2(x, y) { +// The naming of y and x matches the spec. +// The order of calling ToNumber matches JSC. +function MathAtan2(y, x) { if (!IS_NUMBER(x)) x = ToNumber(x); if (!IS_NUMBER(y)) y = ToNumber(y); - return %Math_atan2(x, y); + return %Math_atan2(y, x); } // ECMA 262 - 15.8.2.6 @@ -117,11 +119,12 @@ // ECMA 262 - 15.8.2.11 function MathMax(arg1, arg2) { // length == 2 var r = -$Infinity; - for (var i = %_ArgumentsLength() - 1; i >= 0; --i) { + var length = %_ArgumentsLength(); + for (var i = 0; i < length; i++) { var n = ToNumber(%_Arguments(i)); if (NUMBER_IS_NAN(n)) return n; - // Make sure +0 is consider greater than -0. - if (n > r || (n === 0 && r === 0 && (1 / n) > (1 / r))) r = n; + // Make sure +0 is considered greater than -0. + if (n > r || (r === 0 && n === 0 && !%_IsSmi(r))) r = n; } return r; } @@ -129,11 +132,12 @@ // ECMA 262 - 15.8.2.12 function MathMin(arg1, arg2) { // length == 2 var r = $Infinity; - for (var i = %_ArgumentsLength() - 1; i >= 0; --i) { + var length = %_ArgumentsLength(); + for (var i = 0; i < length; i++) { var n = ToNumber(%_Arguments(i)); if (NUMBER_IS_NAN(n)) return n; - // Make sure -0 is consider less than +0. - if (n < r || (n === 0 && r === 0 && (1 / n) < (1 / r))) r = n; + // Make sure -0 is considered less than +0. + if (n < r || (r === 0 && n === 0 && !%_IsSmi(n))) r = n; } return r; } --~--~---------~--~----~------------~-------~--~----~ v8-dev mailing list [email protected] http://groups.google.com/group/v8-dev -~----------~----~----~----~------~----~------~--~---
