[INTL] Implement Intl.getCanonicalLocales
Created attachment 290292 [details] Patch
Comment on attachment 290292 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290292&action=review r=me > Source/JavaScriptCore/runtime/IntlObject.cpp:59 > +static EncodedJSValue JSC_HOST_CALL IntlObjectFuncGetCanonicalLocales(ExecState*); First letter should be lower case.
Created attachment 290330 [details] Patch
Comment on attachment 290330 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290330&action=review > Source/JavaScriptCore/runtime/IntlObject.cpp:1032 > + JSArray* localeArray = JSArray::tryCreateUninitialized(vm, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithUndecided), 0); You could create an array of shape ArrayWithContiguous and the initial size already set to localeList.size(). That way you avoid growing the butterfly dynamically on push(). > Source/JavaScriptCore/runtime/IntlObject.cpp:1040 > + auto len = localeList.size(); > + for (size_t i = 0; i < len; ++i) { > + localeArray->push(state, jsString(state, localeList[i])); If you use push instead of setting the element at index, you can use the for-in loop of C++11: for (const String& locale : localeList) { ...
Created attachment 290751 [details] Patch
Comment on attachment 290330 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290330&action=review >> Source/JavaScriptCore/runtime/IntlObject.cpp:1032 >> + JSArray* localeArray = JSArray::tryCreateUninitialized(vm, globalObject->arrayStructureForIndexingTypeDuringAllocation(ArrayWithUndecided), 0); > > You could create an array of shape ArrayWithContiguous and the initial size already set to localeList.size(). > That way you avoid growing the butterfly dynamically on push(). done. >> Source/JavaScriptCore/runtime/IntlObject.cpp:1040 >> + localeArray->push(state, jsString(state, localeList[i])); > > If you use push instead of setting the element at index, you can use the for-in loop of C++11: > for (const String& locale : localeList) { > ... Setting the initial size required this to switch to initializeIndex, which needs the index number.
Comment on attachment 290751 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=290751&action=review > Source/JavaScriptCore/runtime/IntlObject.cpp:118 > + putDirectNativeFunction(vm, globalObject, Identifier::fromString(&vm, ASCIILiteral("getCanonicalLocales")), 1, intlObjectFuncGetCanonicalLocales, NoIntrinsic, DontEnum); Identifier has a constructor taking a static string, you do not have to use ASCIILiteral with them. > Source/JavaScriptCore/runtime/IntlObject.cpp:1039 > + auto len = localeList.size(); > + for (size_t i = 0; i < len; ++i) { In WebKit, we avoid shortening names. "len" should be "length".
Created attachment 290762 [details] Patch for landing
Comment on attachment 290762 [details] Patch for landing Clearing flags on attachment: 290762 Committed r206837: <http://trac.webkit.org/changeset/206837>
All reviewed patches have been landed. Closing bug.