Summary: | Improvements to Intl code | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Sukolsak Sakshuwong <sukolsak> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | andy, benjamin, commit-queue, darin, ggaren, keith_miller, mark.lam, msaboff, saam | ||||||||
Priority: | P2 | ||||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Sukolsak Sakshuwong
2016-02-19 18:24:25 PST
Created attachment 271835 [details]
Patch
Oops, wrong link. For the comment about std::unique_ptr, see Comment 10 in https://bugs.webkit.org/show_bug.cgi?id=147605#c10 Comment on attachment 271835 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271835&action=review > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:674 > + m_dateFormat = std::unique_ptr<UDateFormat, UDateFormatDeleter>(udat_open(UDAT_IGNORE, UDAT_IGNORE, m_locale.utf8().data(), timeZoneView.upconvertedCharacters(), timeZoneView.length(), pattern.upconvertedCharacters(), pattern.length(), &status)); Since ICU headers were updated to v52, UDAT_PATTERN is preferred over UDAT_IGNORE. Mind changing that while you are in here? Created attachment 271839 [details]
Patch
Created attachment 271840 [details]
Patch
(In reply to comment #3) > Comment on attachment 271835 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271835&action=review > > > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:674 > > + m_dateFormat = std::unique_ptr<UDateFormat, UDateFormatDeleter>(udat_open(UDAT_IGNORE, UDAT_IGNORE, m_locale.utf8().data(), timeZoneView.upconvertedCharacters(), timeZoneView.length(), pattern.upconvertedCharacters(), pattern.length(), &status)); > > Since ICU headers were updated to v52, UDAT_PATTERN is preferred over > UDAT_IGNORE. Mind changing that while you are in here? Done. Thanks! Comment on attachment 271840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271840&action=review > Source/JavaScriptCore/runtime/IntlCollator.cpp:329 > + auto collator = std::unique_ptr<UCollator, UCollatorDeleter>(ucol_open(m_locale.utf8().data(), &status)); Use std::make_unique from StdLibExtras.h (In reply to comment #7) > Comment on attachment 271840 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=271840&action=review > > > Source/JavaScriptCore/runtime/IntlCollator.cpp:329 > > + auto collator = std::unique_ptr<UCollator, UCollatorDeleter>(ucol_open(m_locale.utf8().data(), &status)); > > Use std::make_unique from StdLibExtras.h I don't think that works, because ucol_open is not a C++ constructor. Comment on attachment 271840 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271840&action=review >>> Source/JavaScriptCore/runtime/IntlCollator.cpp:329 >>> + auto collator = std::unique_ptr<UCollator, UCollatorDeleter>(ucol_open(m_locale.utf8().data(), &status)); >> >> Use std::make_unique from StdLibExtras.h > > I don't think that works, because ucol_open is not a C++ constructor. That’s right. Can’t use make_unique here. Comment on attachment 271840 [details] Patch Clearing flags on attachment: 271840 Committed r196887: <http://trac.webkit.org/changeset/196887> All reviewed patches have been landed. Closing bug. |