[Webkit-unassigned] [Bug 147602] [INTL] Implement Intl.NumberFormat.prototype.resolvedOptions ()

[bugzilla-daemon at webkit.org]

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

--- Comment #30 from Sukolsak Sakshuwong <sukolsak at gmail.com> ---
(In reply to comment #28)
> Comment on attachment 270532 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=270532&action=review
> 
> > Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:103
> > +    return (currency[0] << 16) + (currency[1] << 8) + currency[2];
> 
> This will do the wrong thing if any characters are in the range 0x80-0xFF on
> platforms where char is signed. Should we assert the three characters are
> all ASCII?

Done. Use isASCIIAlpha (as opposed to isASCII) to be consistent with the spec.

> > Source/JavaScriptCore/runtime/IntlObject.cpp:241
> > +        if (!(doubleValue >= static_cast<double>(minimum) && doubleValue <= static_cast<double>(maximum))) {
> 
> Are these static_cast necessary? Doesn’t the unsigned automatically get
> promoted to a double without explicit conversion?

Removed static_cast.

> > Source/JavaScriptCore/runtime/IntlObject.cpp:242
> > +            state.vm().throwException(&state, createRangeError(&state, property.publicName() + ASCIILiteral(" is out of range")));
> 
> I don’t think you need the ASCIILiteral here, and in fact I think that using
> it makes the code slightly less efficient, causing it to allocate an
> additional temporary string.

Done. Need to add "*" in front of property.publicName().

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20160203/0ee25401/attachment-0001.html>