[webkit-reviews] review granted: [Bug 206791] Move singleton Intl string locales out of JSGlobalObject. : [Attachment 388755] proposed patch.

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jan 24 20:59:14 PST 2020


Yusuke Suzuki <ysuzuki at apple.com> has granted Mark Lam <mark.lam at apple.com>'s
request for review:
Bug 206791: Move singleton Intl string locales out of JSGlobalObject.
https://bugs.webkit.org/show_bug.cgi?id=206791

Attachment 388755: proposed patch.

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




--- Comment #9 from Yusuke Suzuki <ysuzuki at apple.com> ---
Comment on attachment 388755
  --> https://bugs.webkit.org/attachment.cgi?id=388755
proposed patch.

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

r=me, nice.

> Source/JavaScriptCore/runtime/IntlObject.h:62
> +const HashSet<String>& intlNumberFormatAvailableLocales();

Define intlPluralRulesAvailableLocales.

>> Source/JavaScriptCore/runtime/IntlPluralRules.cpp:124
>> +	const HashSet<String>& availableLocales =
intlNumberFormatAvailableLocales();
> 
> Andy, can you confirm that it is intentional to use
intlNumberFormatAvailableLocales() instead of intlPluralRulesAvailableLocales()
here?  I noticed that intlPluralRulesAvailableLocales() is called anywhere,
which may me suspicious of this code.  Thought you might know better what is
correct.

Thanks, Andy. Yeah, seems that,

defining intlPluralRulesAvailableLocales, which just calls
intlNumberFormatAvailableLocales() internally.

Would be nice.

> Source/JavaScriptCore/runtime/IntlPluralRulesConstructor.cpp:121
> +    const HashSet<String>& availableLocales =
intlNumberFormatAvailableLocales();

Yeah, use intlPluralRulesAvailableLocales, which should just call
intlNumberFormatAvailableLocales() internally.


More information about the webkit-reviews mailing list