Bug 215839 – [JSC] Implement Intl.DateTimeFormat dayPeriod

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Yusuke Suzuki]

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

[Yusuke Suzuki]

Created attachment 407275 [details]
Patch

[Darin Adler]

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 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 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]

Created attachment 407532 [details]
Patch

[Yusuke Suzuki]

Created attachment 407533 [details]
Patch

[Ross Kirsling]

Comment on attachment 407533 [details]
Patch

r=me, though the flag intrinsic makes me super sad.

[Yusuke Suzuki]

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

[Radar WebKit Bug Importer]

<rdar://problem/67985176>