[Webkit-unassigned] [Bug 31330] Expose dtoa() for ECMA-262

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=31330


Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #43034|review?                     |review-
               Flag|                            |




--- Comment #7 from Darin Adler <darin at apple.com>  2009-11-12 09:36:48 PST ---
(From update of attachment 43034)
>  UString UString::from(double d)
>  {
>      char buf[80];
> +    unsigned len = ecma262dtoa(d, buf);
> +    buf[len] = 0;
>      return UString(buf);
>  }

We could make this even more efficient by making a new UString constructor that
takes a const char* and a length to avoid the unnecessary strlen. That's not
strictly necessary, but sure would be nice.

>  #include <wtf/Assertions.h>
>  #include <wtf/FastMalloc.h>
> +//#include <wtf/MathExtras.h>
>  #include <wtf/Vector.h>
>  #include <wtf/Threading.h>

Why the added-but-commented-out include?

> +static ALWAYS_INLINE char* append(char* next, const char* src, unsigned size)
> +{
> +    for (unsigned i = 0; i < size; ++i)
> +        *next++ = static_cast<unsigned char>(*src++);
> +    return next;
> +}
> +
> +static ALWAYS_INLINE char* append(char* next, const char* src)
> +{
> +    while (*src)
> +        *next++ = static_cast<unsigned char>(*src++);
> +    return next;
> +}

I would have made the "next" argument be a char*& rather than using a return
value.

Why the casts to unsigned char? Those are not at all helpful.

> +    // dtoa() for ECMA-262 'ToString Applied to the Number Type.'
> +    // The buffer pointed by buf must have at least 80 elements.

I'd like to see a constant with that value 80. It's not good to have a comment
in one file and then the number typed in some other source file. Maybe
something like this?

    typedef char dtoaECMA262Buffer[80];

> +    // The return value is the length of the resultant string in buf.
> +    // The resultant string isn't terminated by 0.
> +    unsigned ecma262dtoa(double d, char* buf);

I don't think it's great to name the function dtoa when it returns a length.
Maybe the length could be an out argument instead. Or somehow we could name the
function in a way that makes it clear its return value is the length of the
string.

It might be better to give this a name with a nicer format.I'm not sure having
ECMA262 in the name is good. The colloquial name for this is JavaScript, so
maybe this super-long name would be OK, or maybe some shorter variant.

    doubleToStringInJavaScriptFormatReturningLength

The argument name "d" doesn't add anything here, so I'd suggest leaving it out.
The argument name "buf" could instead be a word, "buffer".

It would be clearer if the buffer argument was declared as an 80-character
array rather than a char*. Because of how C handles arrays, the two are
probably equivalent at runtime, but declaring it that way is a clearer way to
indicate it needs the 80 characters than just the comment.

review- mainly because of that stray commented out include, but please consider
my other comments and suggestions

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.