Let's switch from PRBool to bool.

These are the commands I use to generate the patch that converts most of mozilla from PRBool to bool.

Making a guess for a reviewer here.

These are the commands I use to generate a patch and then modify the patch to s/PR_TRUE/true/ s/PR_FALSE/false/ on modified lines in the patch. Actual patch not attached because it's about 8.5mb and very quickly bitrots.

Comment on attachment 556903 [details] [diff] [review] 4. Make pyidl convert boolean to bool Yay for neat clean code.

Certain calls, SetBool especially, confuse the compiler if PR_TRUE/FALSE is used instead of true/false since the compiler can't decide which function we mean. This patch fixes those cases by doing s/PR_FALSE/false/ as necessary.

Necessary since pldhash.h now requires C++ since it uses bool.

Not sure what the real type should be here, but bool isn't it, apparently.

And that's all the patches I have for now. Basically, the strategy is to do a massive search and replace on the whole tree and undo the parts that aren't appropriate. That's patches 6-12 here. All patches need to be applied for this to work. Patches which can land separately have their own bugs. Not sure what people want to do in terms of closing trees/scheduling for this.

Comment on attachment 556907 [details] [diff] [review] 5. Switch quickstubs to bool This looks fine. r=me

Comment on attachment 556900 [details] [diff] [review] 3. Convert prbools in IPDLs to bools Would you please also nuke these guys http://mxr.mozilla.org/mozilla-central/source/ipc/ipdl/ipdl/builtin.py#64 so we don't need to worry about this again? (Or at least, worry about it less.)

PRBool/PRPackedBool removed from builtins, some s/PRBool/bool/ redone with better indenting.

This command does a better job at maintaining indentation.

And this one does an even better job of maintaining indentation. It's not perfect, but I think it covers most cases sufficiently that we can live with the cases that it misses.

Comment on attachment 557165 [details] [diff] [review] 3. Convert prbools in IPDLs to bools, v2 Thanks.

Just discovered that the sed command for replacing PR_TRUE/PR_FALSE should actually be run about three times to cover cases where there is more than one PR_TRUE/PR_FALSE on the same line. There's probably a smarter sed command to do this..

s///g ?

(In reply to Masatoshi Kimura [:emk] from comment #24) > s///g ? Nope. The current regex is too greedy for s///g to make a difference.

Something like: -e ':loop s/foo/bar/g t loop' \ (http://www.thegeekstuff.com/2009/12/unix-sed-tutorial-6-examples-for-sed-branching-operation/)

Comment on attachment 557249 [details] 6. Commands to generate a patch that s/PRBool/bool/, v3 Blech, I think I'd prefer emacs-lisp to actually do reindentation, but I guess that this worse-is-better approach is reasonable.

Comment on attachment 556898 [details] [diff] [review] 2. Remove PRBool testing in xpconnect I think this is fine, but bholley may have patches which conflict, we should ask him.

Comment on attachment 556916 [details] [diff] [review] 8. Fixes for calls that are now ambiguous Why are all the changes to Preferences::GetBool necessary? Doesn't the other patch automatically replace PR_FALSE with false? I'm not opposed to this, but I don't understand it.

Comment on attachment 556917 [details] [diff] [review] 9. Misc fixes for compiler errors This is safe because we blow away the startup cache every time the buildid or version changes, right?

Comment on attachment 556920 [details] [diff] [review] 11. Undo some s/PRBool/bool/ that breaks C code What C code do we have left? Could we just stop supporting C code here?

Comment on attachment 556922 [details] [diff] [review] 12. Undo a s/PRBool/bool/ that was crashing plugins Make this a PRUint32 please and then use !! to set *wantsAllStreams? This will also be necessary in AnswerNPP_GetValue_NPPVpluginNeedsXEmbed.

Comment on attachment 556920 [details] [diff] [review] 11. Undo some s/PRBool/bool/ that breaks C code Tenatively r-, please re-request if necessary.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #29) > Comment on attachment 556916 [details] [diff] [review] > 8. Fixes for calls that are now ambiguous > > Why are all the changes to Preferences::GetBool necessary? Doesn't the other > patch automatically replace PR_FALSE with false? I'm not opposed to this, > but I don't understand it. The script that replaces PR_FALSE with false only does it on the lines that we've done a s/PRBool/bool/ replacement on, in order to reduce the number of changes. The reason this is needed is because there are two Preferences::GetBool: static PRBool GetBool(const char* aPref, PRBool aDefault = PR_FALSE) static nsresult GetBool(const char* aPref, PRBool* aResult) The compiler doesn't know which one we want if we don't use false.

(In reply to Benjamin Smedberg [:bsmedberg] from comment #30) > Comment on attachment 556917 [details] [diff] [review] > 9. Misc fixes for compiler errors > > This is safe because we blow away the startup cache every time the buildid > or version changes, right? Yeah the startup cache should be blown away on updates. Though, it might even end up the same since we haven't changed the size of the type..

Oh, and besides GetBool, there's a similar issue with the Item function.

Comment on attachment 556898 [details] [diff] [review] 2. Remove PRBool testing in xpconnect These files are moving from tests/components to tests/components/native, but otherwise they're fine. It's a trivial merge, so whichever one of us lands second can take care of it.

Comment on attachment 556916 [details] [diff] [review] 8. Fixes for calls that are now ambiguous Do you have plans to replace all the PR_TRUE, or is that just considered too invasive for now?

(In reply to Benjamin Smedberg [:bsmedberg] from comment #38) > Comment on attachment 556916 [details] [diff] [review] > 8. Fixes for calls that are now ambiguous > > Do you have plans to replace all the PR_TRUE, or is that just considered too > invasive for now? I was considering doing a whole tree s/PR_TRUE/true/ s/PR_FALSE/false/ after a few months to allow it to naturally happen as people land their patches. I'm assuming that most people would kill PR_TRUE/PR_FALSE in the code they're touching if given a choice. But, we can do it sooner if that's better.

I didn't add a !! because the bool type does that for us.

Apparently none of the C code use the things that are C++ only now, so we can just take these things away in that case.

Try run for c7a807408637 is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=c7a807408637 Results (out of 231 total builds): exception: 1 success: 218 warnings: 10 failure: 2 Builds available at http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/mwu@mozilla.com-c7a807408637

Blog post at https://blog.mozilla.com/mwu/2011/09/29/mozilla-central-now-with-88-7-less-prbool/ https://people.mozilla.com/~mwu/convert.sh

Just an observation. There seems to be an issue with the way this is being rolled out. It seems to me that once this is completed for an area of the code, then PRBool PR_TRUE and PR_FALSE need to be redefined in a way that causes a compiler error. The issue stems from pending already R+'d patches that add more uses of PRBool, PR_TRUE and PR_FALSE.

I think we need to remove the definitions for PRBool PR_TRUE and PR_FALSE from any global include files and make sure these are only defined in modules where their use is still p=permitted. The issue is there are many unlanded r+'d patches that add new instances of this. Should I file a new bug on this?

Given the enormity of this change to the documentation, here's how we're handling it (for the record): 1. We've noted this change on Firefox 10 for developers, as well as on the article about updating add-ons for Firefox 10. 2. We are not actively updating any other documentation at this time. 3. Once we're on the new Kuma wiki for MDN, and it has its global find and replace feature implemented, we'll begin work on actually actively updating documentation for this change everywhere else. So, for my purposes, this is done for Firefox 10, but leaving the doc needed flag on it to remember to do (3) later.