1288355 - Remove MOZ_UTF16() macro in favor of using u"string" directly

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Edmund Wong (:ewong)]

There's quite a few mentions in mailnews/ etc..  

+++ This bug was initially created as a clone of Bug #1277106 +++

We are currently using MOZ_UTF16() macro to workaround the difference between VC compiler (which uses L"") and other compilers (which accepts u""). This macro can be removed once we drop the support of VS2013, because VS2015 supports u"string" directly.

Comment 1

[Jorg K (CEST = GMT+2)]

Attachment: [see comment #8 and comment #10 for patches landed]

This was created with:

find . -name *.cpp -exec sed -i -e 's/MOZ_UTF16(\(\".*\"\))/u\1/g' {} \;

run on /mail and /mailnews.

Try:
https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=05713963be5a

Comment 2

[Jorg K (CEST = GMT+2)]

Comment on attachment 8773459 [details] [diff] [review]
[see comment #8 and comment #10 for patches landed]

Anyone wants to review this? It's boring. sed will have done the right thing.

Comment 3

[Jorg K (CEST = GMT+2)]

There were two in /suite. I've got them covered ;-)

Comment 4

[Kent James (:rkent)]

The parent patch had also changes to NS_LITERAL_STRING, like:

-      Write(NS_LITERAL_STRING("\xFFFC"));
+      Write(NS_LITERAL_STRING(u"\xFFFC"));

Do you intent to also do a patch for that?

Comment 5

[Jorg K (CEST = GMT+2)]

Kent, no.

Joshua pointed out that I missed a few lines where there were two strings on a line. Fixed now:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=c802c2cc66df

Comment 6

[Jorg K (CEST = GMT+2)]

Missed some more.

I got smart and did a local compile on Windows first ;-) Since that worked, here another try:

https://treeherder.mozilla.org/#/jobs?repo=try-comm-central&revision=14569ad7ebf1

Comment 7

[Jorg K (CEST = GMT+2)]

(In reply to Kent James (:rkent) from comment #4)
> The parent patch had also changes to NS_LITERAL_STRING, like:
> -      Write(NS_LITERAL_STRING("\xFFFC"));
> +      Write(NS_LITERAL_STRING(u"\xFFFC"));
> Do you intent to also do a patch for that?

We don't have that anywhere. I only found this in Mailnews:
  int32_t offset = val.Find(NS_LITERAL_STRING("\x0D\x0A"));
    offset = val.Find(NS_LITERAL_STRING("\x0D\x0A"), offset + 2);
./import/text/src/nsTextImport.cpp

So I don't think they need treatment.

Comment 8

[Jorg K (CEST = GMT+2)]

Landed:
https://hg.mozilla.org/comm-central/rev/a09c05db1261

The patch is 4422 lines long, so frankly, I don't thing anyone would have had the time or inclination to go through it with a fine comb. Plus, apart from a few manual tweaks, it was all sed.

Let's leave the bug open to see whether there are any build/test failures. Built OK on Windows and Linux.

Comment 9

[Jorg K (CEST = GMT+2)]

Comment on attachment 8773459 [details] [diff] [review]
[see comment #8 and comment #10 for patches landed]

The patch does *not* reflect exactly what was pushed!

Comment 10

[Jorg K (CEST = GMT+2)]

Linux went through alright, Windows didn't build for some reason and Mac failed since I had forgotten to fix up .mm files as well. So here's another push:
https://hg.mozilla.org/comm-central/rev/66ffb3c06576

Comment 11

[Jorg K (CEST = GMT+2)]

Mac still doesn't compile, looks like more bustage, but in M-C(??):

/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/Promise.cpp:744:31: error: no member named 'PromiseNativeHandler' in namespace 'mozilla::dom::prototypes::id'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseDebugging.cpp:402:27: error: no viable conversion from 'JS::RootedObject' (aka 'Rooted<JSObject *>') to 'mozilla::dom::Promise'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseDebugging.cpp:419:23: error: no viable conversion from 'JS::RootedObject' (aka 'Rooted<JSObject *>') to 'mozilla::dom::Promise'
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseNativeHandler.cpp:19:10: error: use of undeclared identifier 'PromiseNativeHandlerBinding'; did you mean 'PromiseNativeHandler'?
/builds/slave/tb-c-cen-m64-00000000000000000/build/mozilla/dom/promise/PromiseNativeHandler.cpp:19:39: error: no member named 'Wrap' in 'mozilla::dom::PromiseNativeHandler'

Comment 12

[Frank-Rainer Grahl (:frg)]

You need to clobber. Had this with bug 911216 part 30 early this week. Bug has been backed out since then and in again yesterday.

Comment 13

[Jorg K (CEST = GMT+2)]

Clobber on Treeherder?? I wouldn't know how to do that, although I can see something here:
https://hg.mozilla.org/mozilla-central/rev/efcf012e6cfc

Comment 14

[Frank-Rainer Grahl (:frg)]

I think you just push an empty file called CLOBBER.

Comment 15

[Richard Marti (:Paenglab)]

I clobbered the OS X (and the Win to be sure) machines. I also restarted the OS X builds.

Comment 16

[Jorg K (CEST = GMT+2)]

Richard, can you please explain what you did exactly to clobber.

Restarted the OS X went via the Treeherder menu, "Add New Jobs" I suppose.

Comment 17

[Jorg K (CEST = GMT+2)]

Richard told me privately, that "clobber" is also available on the Treeherder menu.

Re. comment #4 and comment #7: Reading bug 1277106 comment #5 (quote):
Note that a few NS_LITERAL_STRING() callers must be changed when their string literal parameter actually contains escaped Unicode characters like "\x2026".

I've just looked again:
find . -name *.cpp -exec grep '\"\\x[0-9][0-9][0-9]' {} \; -print

The only thing that turned up was:
      AddText(L"\x2019"); // Unicode right single quotation mark
./import/outlook/src/rtfMailDecoder.cpp

That L could have been replaced by a u, but since this is compiled for Windows only (Outlook import), it doesn't matter.

Comment 18

[Jorg K (CEST = GMT+2)]

Despite looking red on Treeherder, Windows did compile:
http://archive.mozilla.org/pub/thunderbird/tinderbox-builds/comm-central-win32/1469165954/comm-central-win32-bm71-build1-build0.txt.gz
Finished compile (results: 0, elapsed: 59 mins, 4 secs) (at 2016-07-22 00:43:56.597159)
Looks like some packaging failed:
Error: c:\builds\moz2_slave\tb-c-cen-w32-00000000000000000\build\objdir-tb\mail\installer\package-manifest:55: Missing file(s): bin/icudt56l.dat
[snip]
mozmake.exe[3]: *** [stage-package] Error 1
mozmake.exe[2]: *** [make-package] Error 2
mozmake.exe[1]: *** [default] Error 2

Looks like bug 1262731 (SM bug 1288533)

Comment 19

[Jorg K (CEST = GMT+2)]

Raised bug 1288651.

Comment 20

[Frank-Rainer Grahl (:frg)]

Local suite build works fine at first glance. Just briefly tested mail / news. Thanks.

Comment 21

[Jorg K (CEST = GMT+2)]

Yes, I started up TB with the change yesterday and it seemed to work.

Since Mac compiled not, I guess we're done here.

Comment 22

[Jorg K (CEST = GMT+2)]

Opps: Since Mac compiled NOW, I guess we're done here.