1446051 - Use CallICU for str_normalize

Status: RESOLVED FIXED

(Core :: JavaScript: Standard Library, enhancement, P3)

Comment 1

[André Bargull [:anba]]

Attachment: bug1446051.patch

Use |intl::CallICU| for str_normalize, so we don't need to handle |U_BUFFER_OVERFLOW_ERROR| manually in str_normalize. I've moved |remainingStart| and |remainingLength| into the lambda, because otherwise the capture list gets too large and can no longer be properly formatted.

Comment 2

[Jeff Walden [:Waldo]]

...we have proper formatting for lambdas, particularly those with lengthy capture lists?

Comment 3

[Jeff Walden [:Waldo]]

Comment on attachment 8959223 [details] [diff] [review]
bug1446051.patch

Review of attachment 8959223 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/builtin/String.cpp
@@ +1644,5 @@
> +        intl::CallICU(cx, [normalizer, &srcChars, spanLength](UChar* chars, uint32_t size,
> +                                                              UErrorCode* status)
> +        {
> +            mozilla::RangedPtr<const char16_t> remainingStart = srcChars.begin() + spanLength;
> +            size_t remainingLength = srcChars.length() - size_t(spanLength);

All these size_t(spanLength) in this function overall are a bit silly.  We probably should get a size_t local going after we've passed the unorm2_spanQuickCheckYes status-check.  Maybe in a followup patch, rs=me on that if you want.

Comment 4

[André Bargull [:anba]]

(In reply to Jeff Walden [:Waldo] from comment #3)
> All these size_t(spanLength) in this function overall are a bit silly.  We
> probably should get a size_t local going after we've passed the
> unorm2_spanQuickCheckYes status-check.  Maybe in a followup patch, rs=me on
> that if you want.

I'll need to update intl::CallICU() for this patch, because I forgot that intl::CallICU() asserts that the input chars-vector is empty. And as part of that update, I'll also add a local variable for size_t(spanLength).

Comment 5

[André Bargull [:anba]]

Attachment: bug1446051.patch

Updated patch to change |intl::CallICU()| assertions to allow non-empty chars-vector. Tentatively carrying r+.

Comment 6

[André Bargull [:anba]]

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=fda78b7e464578358e20764819cee47cb68ccdd7

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/2356a4fada56
Use CallICU helper for str_normalize. r=Waldo

Comment 8

[Arthur Iakab [arthur_iakab]]

https://hg.mozilla.org/mozilla-central/rev/2356a4fada56