Bug 6259 - Handle negative, FP numbers with non-10 radix in toString
Summary: Handle negative, FP numbers with non-10 radix in toString
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 420+
Hardware: Other Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2005-12-27 13:57 PST by Maks Orlovich
Modified: 2005-12-30 00:13 PST (History)
0 users

See Also:


Attachments
patch (2.29 KB, patch)
2005-12-27 13:58 PST, Maks Orlovich
mjs: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Maks Orlovich 2005-12-27 13:57:31 PST
r260470 | porten | 2003-10-20 10:45:55 -0400 (Mon, 20 Oct 2003) | 2 lines 
This is a merge of: 
       * number_object.cpp: rewrote Number.toString(radix) to work with  
       negative numbers, floating point and very large numbers.  
 
As ammended by:  
r290299 | porten | 2004-02-22 13:32:05 -0500 (Sun, 22 Feb 2004) | 4 lines  
fixed crash if toString() is called on NaN or Inf with a radix != 10.)  
  
It's somewhat covered by KJS tests.
Comment 1 Maks Orlovich 2005-12-27 13:58:10 PST
Created attachment 5309 [details]
patch
Comment 2 Maciej Stachowiak 2005-12-28 21:23:38 PST
Comment on attachment 5309 [details]
patch

I'll grant review because I think  it's better to have this change than not.
However:

> +      const double eps = 0.001; // TODO: guessed. base on radix ?

That seems like a pretty high value for epsilon.

> int(d)
> double(radix)

(in various places) - it would be better to avoid this cast syntax for numbers,
I think. In some cases I think it is unnecesary, in other cases a normal
c-style cast or assigning first to a variable of the appropriate type would be
better.

I think these improvements can be made in a separate pass though.