Bug 34462 - Fix a bug that Math.round() returns incorrect results for huge integers
Summary: Fix a bug that Math.round() returns incorrect results for huge integers
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-02-01 19:37 PST by Kent Tamura
Modified: 2010-02-01 20:51 PST (History)
1 user (show)

See Also:


Attachments
Patch (3.64 KB, patch)
2010-02-01 19:38 PST, Kent Tamura
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kent Tamura 2010-02-01 19:37:14 PST
Fix a bug that Math.round() retunrs incorrect results for huge integers
Comment 1 Kent Tamura 2010-02-01 19:38:48 PST
Created attachment 47901 [details]
Patch
Comment 2 Darin Adler 2010-02-01 19:46:59 PST
Comment on attachment 47901 [details]
Patch

How about this instead?

-    if (signbit(arg) && arg >= -0.5)
-         return jsNumber(exec, -0.0);
-    return jsNumber(exec, floor(arg + 0.5));
+    double integer = ceil(arg);
+    return jsNumber(exec, integer - (integer - arg > 0.5));
Comment 3 Darin Adler 2010-02-01 19:47:56 PST
Comment on attachment 47901 [details]
Patch

Patch seems fine. But you should consider my patch with ceil that avoids the need for the separate if statement. You could use ? : instead of the way I did it. Or use my code exactly as is (as long as it passes your tests).
Comment 4 Darin Adler 2010-02-01 19:48:58 PST
Would be nice to have tests cases for the maximum and minimum double values as well to make sure they don't turn into infinity when rounded.
Comment 5 Kent Tamura 2010-02-01 20:51:16 PST
Thank you for the quick review.

Ok, I followed the proposed code, and add test cases for minimum/maximum values.

Landed as r54197 and r54198.