924011 - Do not assume that PRUnichar and UChar are the same type

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[(no longer active)]

Attachment: Patch (v1)

Comment 1

[Joshua Cranmer [:jcranmer]]

It's worth noting that there's a macro  you can set to make ICU define UChar to char16_t instead.

Comment 2

[(no longer active)]

(In reply to Joshua Cranmer [:jcranmer] from comment #1)
> It's worth noting that there's a macro  you can set to make ICU define UChar
> to char16_t instead.

What is that macro?  That sounds fine to me, personally.

Comment 3

[Jeff Walden [:Waldo]]

The macro would be fine, if it won't break Linux distros that link against a system ICU.  I don't know if that'll be a problem or not, exactly.

Comment 4

[Jeff Walden [:Waldo]]

Comment on attachment 814016 [details] [diff] [review]
Patch (v1)

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

Let's consolidate all these casts into a single cast in a utility method in Intl.h.

inline const UChar *
JSCharToUChar(const jschar *chars)
{
    return reinterpret_cast<const UChar*>(chars);
}

Or something along those lines.

Given leeriness about the system ICU thing, let's punt on the macro bit until distros have implemented a system-ICU option (which presumably they've held off on because bug 853301 hasn't landed yet).

Comment 5

[(no longer active)]

Attachment: Patch (v2)

Comment 6

[Jeff Walden [:Waldo]]

Comment on attachment 814525 [details] [diff] [review]
Patch (v2)

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

::: js/src/builtin/Intl.h
@@ +186,5 @@
> +JSCharToUChar(const jschar *chars)
> +{
> +  return reinterpret_cast<const UChar *>(chars);
> +}
> +inline UChar *

Blank line before this.

Comment 7

[(no longer active)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce463593acc6

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ce463593acc6