Bug 215839

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:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

213425

Attachments:

Description	Flags
Patch	none
Patch	none
Patch	ross.kirsling: review+

Yusuke Suzuki

Reported 2020-08-25 23:15:44 PDT

https://github.com/tc39/ecma402/pull/346

Attachments
Patch (14.52 KB, patch) 2020-08-25 23:19 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (17.34 KB, patch) 2020-08-28 23:42 PDT, Yusuke Suzuki	no flags	Details Formatted Diff Diff
Patch (17.40 KB, patch) 2020-08-28 23:43 PDT, Yusuke Suzuki	ross.kirsling: review+	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Yusuke Suzuki

Comment 1 2020-08-25 23:19:11 PDT

Created attachment 407275 [details] Patch

Darin Adler

Comment 2 2020-08-26 15:15:09 PDT

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").

Yusuke Suzuki

Comment 3 2020-08-28 23:22:15 PDT

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.

Yusuke Suzuki

Comment 4 2020-08-28 23:42:25 PDT

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.

Yusuke Suzuki

Comment 5 2020-08-28 23:42:37 PDT

Created attachment 407532 [details] Patch

Yusuke Suzuki

Comment 6 2020-08-28 23:43:44 PDT

Created attachment 407533 [details] Patch

Ross Kirsling

Comment 7 2020-08-29 00:01:45 PDT

Comment on attachment 407533 [details] Patch r=me, though the flag intrinsic makes me super sad.

Yusuke Suzuki

Comment 8 2020-08-29 01:32:50 PDT

Committed r266323: <https://trac.webkit.org/changeset/266323>

Radar WebKit Bug Importer

Comment 9 2020-08-29 01:33:37 PDT

<rdar://problem/67985176>

Note You need to log in before you can comment on or make changes to this bug.