[Webkit-unassigned] [Bug 147604] [INTL] Implement Collator Compare Functions
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Dec 16 08:57:48 PST 2015
https://bugs.webkit.org/show_bug.cgi?id=147604
Darin Adler <darin at apple.com> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #267384|review? |review+
Flags| |
--- Comment #23 from Darin Adler <darin at apple.com> ---
Comment on attachment 267384
--> https://bugs.webkit.org/attachment.cgi?id=267384
Patch
View in context: https://bugs.webkit.org/attachment.cgi?id=267384&action=review
> Source/JavaScriptCore/runtime/IntlCollator.cpp:277
> + m_numeric = (result.get(ASCIILiteral("kn")) == "true");
Doesnât seem to be checking the type for boolean like the pseudocode says we should. Do we have tests covering that?
> Source/JavaScriptCore/runtime/IntlCollator.cpp:336
> + UColAttributeValue strength;
I suggest initializing this to UCOL_PRIMARY. Would fix a warning on Windows.
> Source/JavaScriptCore/runtime/IntlCollator.cpp:396
> + UCharIterator iteratorX = createIterator(x);
> + UCharIterator iteratorY = createIterator(y);
Looks like there is a linking problem on Windows.
Creating library C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/JavaScriptCore.lib and object C:/cygwin/home/buildbot/WebKit/WebKitBuild/Release/lib32/JavaScriptCore.exp
IntlCollator.obj : error LNK2019: unresolved external symbol "struct UCharIterator __cdecl WTF::createIterator(class WTF::StringView)" (?createIterator at WTF@@YA?AUUCharIterator@@VStringView at 1@@Z) referenced in function "public: class JSC::JSValue __thiscall JSC::IntlCollator::compareStrings(class JSC::ExecState &,class WTF::StringView,class WTF::StringView)" (?compareStrings at IntlCollator@JSC@@QAE?AVJSValue at 2@AAVExecState at 2@VStringView at WTF@@1 at Z) [C:\cygwin\home\buildbot\WebKit\WebKitBuild\Release\Source\JavaScriptCore\JavaScriptCore.vcxproj]
I think this is because the declaration of createIterator in Collator.h needs WTF_EXPORT_PRIVATE.
> Source/JavaScriptCore/runtime/IntlCollator.cpp:397
> + int result = ucol_strcollIter(m_collator, &iteratorX, &iteratorY, &status);
auto is slightly better than int here
> Source/JavaScriptCore/runtime/IntlCollatorConstructor.cpp:111
> + if (state->hadException())
> + return JSValue::encode(jsUndefined());
We donât need this code. There is no harm in returning the collator when thereâs an exception; the return value will be ignored. It would be better to omit this.
> Source/JavaScriptCore/runtime/IntlCollatorConstructor.cpp:133
> + if (state->hadException())
> + return JSValue::encode(jsUndefined());
We donât need this code. There is no harm in returning the collator when thereâs an exception; the return value will be ignored. It would be better to omit this.
> Source/JavaScriptCore/runtime/IntlObject.cpp:118
> + String locale(uloc_getDefault());
We normally prefer the = syntax to the function call style syntax for code like this.
--
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/20151216/6a1ab164/attachment.html>
More information about the webkit-unassigned
mailing list