| Summary: | [JSC] Implement Intl.DateTimeFormat dayPeriod | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||||
| Component: | JavaScriptCore | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | darin, ews-watchlist, joepeck, keith_miller, mark.lam, msaboff, ross.kirsling, saam, tzagallo, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 213425 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Yusuke Suzuki
2020-08-25 23:15:44 PDT
Created attachment 407275 [details]
Patch
Comment on attachment 407275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407275&action=review > Source/JavaScriptCore/ChangeLog:9 > + and it shows how "AM" etc. is represented. "it shows" -> "it determines" > Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:633 > + skeletonBuilder.appendLiteral("B"); Slightly more efficient to append 'B' than "B". Also, we should get rid of StringBuilder::appendLiteral. For one thing, the syntax for that optimization should just be append("this is a literal"_s). But for another, the "literal" optimization only affects cases where it’s the first thing appended to a string builder. Otherwise, it’s no different from append("this is a literal"). Comment on attachment 407275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407275&action=review >> Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp:633 >> + skeletonBuilder.appendLiteral("B"); > > Slightly more efficient to append 'B' than "B". > > Also, we should get rid of StringBuilder::appendLiteral. For one thing, the syntax for that optimization should just be append("this is a literal"_s). But for another, the "literal" optimization only affects cases where it’s the first thing appended to a string builder. Otherwise, it’s no different from append("this is a literal"). Changed to `append('B')`. For now, using appendLiteral, but I agree. append("...") would be the same because `strlen()` will be constant-folded anyway. Comment on attachment 407275 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=407275&action=review >> Source/JavaScriptCore/ChangeLog:9 >> + and it shows how "AM" etc. is represented. > > "it shows" -> "it determines" Fixed. Created attachment 407532 [details]
Patch
Created attachment 407533 [details]
Patch
Comment on attachment 407533 [details]
Patch
r=me, though the flag intrinsic makes me super sad.
Committed r266323: <https://trac.webkit.org/changeset/266323> |