The easiest way to use mozilla/Char16.h without doing a bitrotty sed change is to force-include it early and drop our old attempts. [David Baron mentioned he wanted an unsignedness check for char16_t, so I'm adding that in part 1. It's not strictly necessary for the rest of the code, but if we want to start using it for jschar or PRUnichar, we need it.]

I tested this much on try, and it at least builds: <https://tbpl.mozilla.org/?tree=Try&rev=1b2c20962f8e>. And we get to lose -fshort-wchar and *two* configure checks! \o/

MOZ_UTF16 obviates the need for it. Actually, we could probably kill the distinction between multiline and not multiline, since MOZ_UTF16("a") "b" should be equivalent to MOZ_UTF16("ab")...

Comment on attachment 789984 [details] [diff] [review] Use Char16.h for NS_LITERAL_STRING and friends nscore.h: It's not clear to me which option we want for gcc on Windows. Maybe it doesn't matter -- but at least maybe leave a comment that maybe it should use wchar_t, unless it's obvious to you one way or the other which one it currently uses. (I'm not in the mood to figure out whether it runs the autoconf tests.) Why does nscore.h need to include Char16.h? (You need it in nsStringAPI.h and nsLiteralString.h though.) nsStaticAtom: Could you just remove the NS_STATIC_ATOM_USE_WIDE_STRINGS macro and remove the unused half of the ifdef? nsStringAPI.h: need to fix the comment above the code you're changing Otherwise looks good. Though I suppose you need a build peer's review on the configure.in changes?

(In reply to David Baron [:dbaron] (don't cc:, use needinfo? instead) from comment #3) > nscore.h: It's not clear to me which option we want for gcc on > Windows. Maybe it doesn't matter -- but at least maybe leave a comment > that maybe it should use wchar_t, unless it's obvious to you one way or > the other which one it currently uses. (I'm not in the mood to figure > out whether it runs the autoconf tests.) I was under the impression that mingw-gcc was using uint16_t for PRUnichar, although on further reflection, it's not, so I'll go change that header include. > Why does nscore.h need to include Char16.h? (You need it in nsStringAPI.h > and nsLiteralString.h though.) I wanted to flesh out any problems with using char16_t globally early on. It doesn't look like there's an easy way to use char16_t instead of PRUnichar without sed'ing it globally, and making jschar and PRUnichar not be the same underlying type has turned out to be a code-rewriting headache. (Granted, nsStringAPI.h/nsLiteralString.h are pretty global headers themselves...) > nsStaticAtom: Could you just remove the NS_STATIC_ATOM_USE_WIDE_STRINGS > macro > and remove the unused half of the ifdef? Er, yes, I could, thanks for remidning me.

Comment on attachment 789980 [details] [diff] [review] Part 1: Better support for char16_t in mfbt Review of attachment 789980 [details] [diff] [review]: ----------------------------------------------------------------- ::: mfbt/Char16.h @@ +39,5 @@ > + /** > + * This macro is used to distinguish when char16_t would be a distinct > + * typedef from wchar_t. > + */ > +# define CHAR16_IS_NOT_WCHAR MOZ_CHAR16_IS_NOT_WCHAR, please.

Comment on attachment 791972 [details] [diff] [review] Part 2: Use Char16.h for NS_LITERAL_STRING and friends Review of attachment 791972 [details] [diff] [review]: ----------------------------------------------------------------- ::: xpcom/base/nscore.h @@ +334,4 @@ > typedef wchar_t PRUnichar; > #else > typedef uint16_t PRUnichar; > #endif Any reason not to use char16_t there as well?

(In reply to Mike Hommey [:glandium] from comment #7) > Comment on attachment 791972 [details] [diff] [review] > Part 2: Use Char16.h for NS_LITERAL_STRING and friends > > Review of attachment 791972 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: xpcom/base/nscore.h > @@ +334,4 @@ > > typedef wchar_t PRUnichar; > > #else > > typedef uint16_t PRUnichar; > > #endif > > Any reason not to use char16_t there as well? Typedef'ing char16_t to PRUnichar at this point breaks _a lot_ of code.

(In reply to Joshua Cranmer [:jcranmer] from comment #8) > Typedef'ing char16_t to PRUnichar at this point breaks _a lot_ of code. You should be able to do it at least on windows. (Char16.h actually makes it mean wchar_t)

(In reply to Mike Hommey [:glandium] from comment #9) > (In reply to Joshua Cranmer [:jcranmer] from comment #8) > > Typedef'ing char16_t to PRUnichar at this point breaks _a lot_ of code. > > You should be able to do it at least on windows. (Char16.h actually makes it > mean wchar_t) The char16_t define in Char16.h is keyed off of _MSC_VER IIRC, this one is on _WIN32. mingw gcc is going to get screwed (I don't know what the correct solution is there).

Comment on attachment 791972 [details] [diff] [review] Part 2: Use Char16.h for NS_LITERAL_STRING and friends >diff --git a/xpcom/glue/nsStringAPI.h b/xpcom/glue/nsStringAPI.h >+static_assert(sizeof(char16_t) == 2, "size of char16_t must be 2"); Could you also toss in the static_assert that char16_t(-1) > char16_t(0), "char16_t must be unsigned" And could you add the same static_assert in nsString.h for both PRUnichar and nsString::char_type (for the internal API), next to the assertions about being size 2? r=dbaron with that, and sorry about the delay

https://hg.mozilla.org/integration/mozilla-inbound/rev/11af1e7c053f https://hg.mozilla.org/integration/mozilla-inbound/rev/0a62484ff46b https://hg.mozilla.org/integration/mozilla-inbound/rev/b0479039a286

https://hg.mozilla.org/mozilla-central/rev/11af1e7c053f https://hg.mozilla.org/mozilla-central/rev/0a62484ff46b https://hg.mozilla.org/mozilla-central/rev/b0479039a286