Prepare for ICU's switch to char16_t

Prepare for ICU's switch to char16_t

ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching
to char16_t in both C/C++ API. It needs some changes. Fortunately,
v8 needs only a couple of changes because v8 has been using
reinterpret_cast in many places calling ICU API.

This change was confirmed to work fine with ICU-59-to-be.

BUG=v8:6062
TEST=trybot

Review-Url: https://codereview.chromium.org/2738503008
Cr-Commit-Position: refs/heads/master@{#43707}
Committed: https://chromium.googlesource.com/v8/v8/+/fd5b3e755df541b44128caed625215017ef59989

[jungshik at Google, 2017-03-08 21:10:50]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 21:10:54]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungshik at Google, 2017-03-08 21:16:24]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 21:16:34]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-08 21:19:25]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 21:19:26]

Dry run: Try jobs failed on following builders:
  v8_linux64_asan_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_linux64_asan_rel_ng/buil...)
  v8_win_nosnap_shared_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_nosnap_shared_rel_ng...)
  v8_win_rel_ng on master.tryserver.v8 (JOB_FAILED,
http://build.chromium.org/p/tryserver.v8/builders/v8_win_rel_ng/builds/23868)

[jungshik at Google, 2017-03-08 22:04:11]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 22:04:16]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungshik at Google, 2017-03-08 22:06:07]

jshin@chromium.org changed reviewers:
+ adamk@chromium.org, yangguo@chromium.org

[jungshik at Google, 2017-03-08 22:06:07]

Hi Adam and Yang, 

Can you take a look?  This is no-op as long as ICU is not updated to 59, but we
need to do this to prepare for ICU 59 update. 

https://codereview.chromium.org/2738453005/ is a test CL with ICU rolled to
ICU-59-to-be along with this CL. Build failures are due to gyp file not being
updated, I believe. GN builds are fine modulo test failures due to timezone name
changes in ICU.

[jungshik at Google, 2017-03-08 22:07:54]

jshin@chromium.org changed reviewers:
+ littledan@chromium.org

[jungshik at Google, 2017-03-08 22:07:54]

Or, Dan can take a look, too. 

Dan, for your i18n work and ICU API usage, please see
https://codereview.chromium.org/2740673002/ and this CL (and the bug referred
to).

[jungshik at Google, 2017-03-08 22:07:57]

Or, Dan can take a look, too. 

Dan, for your i18n work and ICU API usage, please see
https://codereview.chromium.org/2740673002/ and this CL (and the bug referred
to).

[jungshik at Google, 2017-03-08 22:09:14]

On 2017/03/08 22:07:57, jungshik at Google wrote:
> Or, Dan can take a look, too. 
> 
> Dan, for your i18n work and ICU API usage, please see
> https://codereview.chromium.org/2740673002/ and this CL (and the bug referred
> to).

If this CL is landed, I can roll v8 to a revision with this CL to try the entire
Chromium on all platforms (I locally checked it on Linux) in my test CL (
https://codereview.chromium.org/2740673002/ ). 

Thanks

[jungshik at Google, 2017-03-08 22:09:37]

Description was changed from

==========
Prepare for ICU's switch to char16_t

ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching
to char16_t in both C/C++ API. It needs some changes. Fortunately,
v8 needs only a couple of changes because v8 has been using
reinterpret_cast in many places calling ICU API.

This change was confirmed to work fine with ICU-59-to-be.

BUG=v8:6062
TEST=trybot
==========

to

==========
Prepare for ICU's switch to char16_t

ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching
to char16_t in both C/C++ API. It needs some changes. Fortunately,
v8 needs only a couple of changes because v8 has been using
reinterpret_cast in many places calling ICU API.

This change was confirmed to work fine with ICU-59-to-be.

BUG=v8:6062
TEST=trybot
==========

[commit-bot: I haz the power, 2017-03-08 22:26:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-08 22:26:46]

Dry run: This issue passed the CQ dry run.

[jungshik at Google, 2017-03-08 22:56:53]

https://codereview.chromium.org/2738503008/diff/40001/src/runtime/runtime-i18...
File src/runtime/runtime-i18n.cc (right):

https://codereview.chromium.org/2738503008/diff/40001/src/runtime/runtime-i18...
src/runtime/runtime-i18n.cc:621: GetUCharBufferFromFlat(flat2, &sap2, length2));
Markus, can you change (or add an overload to) icu::Collator::compare to take
Char16Ptr instead of char16_t?  Then, I can get rid of this reinterpret_cast
(with an undefined behavior when the spec is interpreted strictly. ubsan may
catch this as well)?

[jungshik at Google, 2017-03-08 23:47:11]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-08 23:47:21]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungshik at Google, 2017-03-08 23:48:09]

On 2017/03/08 22:56:53, jungshik at Google wrote:
>
https://codereview.chromium.org/2738503008/diff/40001/src/runtime/runtime-i18...
> File src/runtime/runtime-i18n.cc (right):
> 
>
https://codereview.chromium.org/2738503008/diff/40001/src/runtime/runtime-i18...
> src/runtime/runtime-i18n.cc:621: GetUCharBufferFromFlat(flat2, &sap2,
length2));
> Markus, can you change (or add an overload to) icu::Collator::compare to take
> Char16Ptr instead of char16_t?  Then, I can get rid of this reinterpret_cast
> (with an undefined behavior when the spec is interpreted strictly. ubsan may
> catch this as well)?

PS 4 took a different approach to resolve an issue without the above (which is
not possible anyway).

[commit-bot: I haz the power, 2017-03-09 00:10:14]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 00:10:15]

Dry run: This issue passed the CQ dry run.

[jungshik at Google, 2017-03-09 01:07:48]

The CQ bit was checked by jshin@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-03-09 01:07:53]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[jungshik at Google, 2017-03-09 01:09:18]

PS #5 should be good to go. Can you take a look?

[commit-bot: I haz the power, 2017-03-09 01:32:41]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-03-09 01:32:42]

Dry run: This issue passed the CQ dry run.

[mscherer, 2017-03-09 17:11:11]

mscherer@google.com changed reviewers:
+ mscherer@google.com

[mscherer, 2017-03-09 17:11:12]

LGTM

An alternative to the version checks might be to clone char16ptr.h, change it to
take/return UChar * rather than char16_t *, and use its conversions directly.
When ICU 59 comes in, you would only have to delete your temporary header file.

[adamk, 2017-03-09 19:56:04]

lgtm

[jungshik at Google, 2017-03-09 19:59:26]

The CQ bit was checked by jshin@chromium.org

[commit-bot: I haz the power, 2017-03-09 19:59:37]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-03-09 20:01:16]

CQ is committing da patch.

Bot data: {"patchset_id": 80001, "attempt_start_ts": 1489089566952720,
"parent_rev": "9ac64ca19157ce679299a4b9bcc9cfb5372e4818", "commit_rev":
"fd5b3e755df541b44128caed625215017ef59989"}

[commit-bot: I haz the power, 2017-03-09 20:01:21]

Description was changed from

==========
Prepare for ICU's switch to char16_t

ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching
to char16_t in both C/C++ API. It needs some changes. Fortunately,
v8 needs only a couple of changes because v8 has been using
reinterpret_cast in many places calling ICU API.

This change was confirmed to work fine with ICU-59-to-be.

BUG=v8:6062
TEST=trybot
==========

to

==========
Prepare for ICU's switch to char16_t

ICU's UChar was uint16_t (non-Win) or wchar_t (Windows). It's switching
to char16_t in both C/C++ API. It needs some changes. Fortunately,
v8 needs only a couple of changes because v8 has been using
reinterpret_cast in many places calling ICU API.

This change was confirmed to work fine with ICU-59-to-be.

BUG=v8:6062
TEST=trybot

Review-Url: https://codereview.chromium.org/2738503008
Cr-Commit-Position: refs/heads/master@{#43707}
Committed:
https://chromium.googlesource.com/v8/v8/+/fd5b3e755df541b44128caed625215017ef...
==========

[commit-bot: I haz the power, 2017-03-09 20:01:22]

Committed patchset #5 (id:80001) as
https://chromium.googlesource.com/v8/v8/+/fd5b3e755df541b44128caed625215017ef...