Last Comment Bug 776630 - Switch comm-central from using nsnull to nullptr.
: Switch comm-central from using nsnull to nullptr.
Status: RESOLVED FIXED
:
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: x86 All
: -- normal (vote)
: Thunderbird 17.0
Assigned To: Mike Conley (:mconley)
:
:
Mentors:
Depends on:
Blocks: 626472
  Show dependency treegraph
 
Reported: 2012-07-23 11:54 PDT by Mike Conley (:mconley)
Modified: 2012-08-02 07:29 PDT (History)
11 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
comm-central: switch nsnull to nullptr (1.77 MB, patch)
2012-07-23 11:54 PDT, Mike Conley (:mconley)
no flags Details | Diff | Splinter Review
Switch nsnull to nullptr (1.77 MB, patch)
2012-07-30 07:41 PDT, Mike Conley (:mconley)
philipp: review+
Details | Diff | Splinter Review
Unbitrotted patch (checked in) (1.77 MB, patch)
2012-08-02 07:29 PDT, Mike Conley (:mconley)
no flags Details | Diff | Splinter Review

Description Mike Conley (:mconley) 2012-07-23 11:54:23 PDT
Created attachment 645008 [details] [diff] [review]
comm-central: switch nsnull to nullptr

Bug 626472 is dropping nsnull in favour of nullptr.

I talked to Ehsan, and I think the plan is to land the definition of nullptr soon, and once it's available in mozilla-central, push our patch to try, and test building it using both clang and gcc.

I'll probably get our try server to apply the removal of nsnull against our mozilla-central subdirectory, for added assurance that we still build alright.
Comment 1 Mike Conley (:mconley) 2012-07-24 06:40:38 PDT
Part 1 from Bug 626472 has landed in mozilla-central.

As per our plan, I'm going to push the Attachment 645008 [details] [diff] to try to see if we get green builds.
Comment 2 :Ehsan Akhgari 2012-07-24 09:49:40 PDT
FWIW, nullptr is now on mozilla-central, so you guys can proceed with this whenever you want.
Comment 3 Mike Conley (:mconley) 2012-07-24 11:01:31 PDT
Ok, here are our try results:

https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=e3b12e72613a

Mostly green, but not perfect. The build failures on Windows seem to be unrelated. The bloattest crashes on Linux, and the crashes caused by Mozmill are more disturbing.

Cc'ing Irving, who's awesome at diagnosing crashes and stuff.

I'll start by poking at the Mozmill crashes.
Comment 4 Mike Conley (:mconley) 2012-07-24 11:29:59 PDT
So the crashes on mailbloat seem to also be happening on trunk (sigh, they just showed up this morning), so that appears to be unrelated.
Comment 5 :Ehsan Akhgari 2012-07-24 14:13:03 PDT
FWIW it will be really surprising if this results in crashes etc once you get green builds...
Comment 6 Mike Conley (:mconley) 2012-07-25 13:16:22 PDT
Yeah, it turns out that other stuff landed on mozilla-central that broke us in new and wonderful ways.

I think we're getting close to sorting it all out. Anyhow, those reds on the try build can be safely ignored, and I think we're going to survive the landing.
Comment 7 Mike Conley (:mconley) 2012-07-30 07:40:01 PDT
This spills outside of mailnews, and affects everything under comm-central.
Comment 8 Mike Conley (:mconley) 2012-07-30 07:41:10 PDT
Created attachment 647163 [details] [diff] [review]
Switch nsnull to nullptr

Here's an up to date conversion patch
Comment 9 Philipp Kewisch [:Fallen] 2012-07-30 08:39:14 PDT
Comment on attachment 647163 [details] [diff] [review]
Switch nsnull to nullptr

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

r=philipp for calendar parts
Comment 10 Mike Conley (:mconley) 2012-07-30 09:38:47 PDT
Mark:

My try build looks good (well, good enough, considering the current bustedness of the tree): https://tbpl.mozilla.org/?tree=Thunderbird-Try&rev=c2c1e3d40065

Fallen has confirmed that the Calendar bits look sane. I've leafed through the diff, and I don't see anything crazy. What's the protocol for a patch this big?

Should we get a swarm of people to ensure that this thing is OK, or should we just land it?

-Mike
Comment 11 Aryeh Gregor (:ayg) (next working March 28-April 26) 2012-08-02 05:39:10 PDT
On m-c, we just did sed -i 's/\bnsnull\b/nullptr/g' and ehsan gave me r+ for whatever that produced without manual review.  It's really unlikely this could cause problems, since nsnull is currently defined to be nullptr in m-c!
Comment 12 Mike Conley (:mconley) 2012-08-02 07:29:14 PDT
Created attachment 648339 [details] [diff] [review]
Unbitrotted patch (checked in)

Re-ran sed script to un-bitrot, and committed to comm-central as

https://hg.mozilla.org/comm-central/rev/0a10c274a1fe

Note You need to log in before you can comment on or make changes to this bug.