[webkit-reviews] review granted: [Bug 185557] [INTL] improve efficiency of Intl.NumberFormat formatToParts : [Attachment 360910] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Feb 4 15:08:51 PST 2019


Mark Lam <mark.lam at apple.com> has granted Andy VanWagoner
<andy at vanwagoner.family>'s request for review:
Bug 185557: [INTL] improve efficiency of Intl.NumberFormat formatToParts
https://bugs.webkit.org/show_bug.cgi?id=185557

Attachment 360910: Patch

https://bugs.webkit.org/attachment.cgi?id=360910&action=review




--- Comment #5 from Mark Lam <mark.lam at apple.com> ---
Comment on attachment 360910
  --> https://bugs.webkit.org/attachment.cgi?id=360910
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=360910&action=review

r=me with suggested improvements.

> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:513
> +    auto literalField = std::make_pair(-1, resultLength);

Instead of using a literal -1 like this, can you give it a name e.g.
    static const int32_t defaultFieldType = -1;
    auto literalField = std::make_pair(defaultFieldType, resultLength);

> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:514
> +    Vector<std::pair<int32_t, int32_t>> fields(resultLength, literalField);

Why not use a struct instead of a pair here?

> Source/JavaScriptCore/runtime/IntlNumberFormat.cpp:544
> +	   auto partType = fieldType < 0 ? literalString : jsString(&exec,
partTypeString(UNumberFormatFields(fieldType), value));

nit: use fieldType == defaultFieldType instead of fieldType < 0.


More information about the webkit-reviews mailing list