796119 - remove vestigal PR_* bits from xpcom headers

Status: RESOLVED FIXED

(Core :: XPCOM, defect)

Description

[Nathan Froyd [:froydnj]]

Removing various bits from xpcom headers is all that's necessary to make:

find $DIR -type f | xargs sed -i -e '/prtypes.h/d'

work just fine for many values of $DIR.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: part 1 - don't use PR_{BEGIN,END}_EXTERN_C

This patch should be relatively uncontroversial; it's not obvious to me that the PR_* macros really gain you anything, so just use the idiom directly.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: part 2 - don't use PR_BIT in pldhash.h

This patch is slightly more controversial.  Various xpcom headers use PR_BIT{,MASK}, but pldhash.h appears to be the worst offender.  This patch, then, addresses the lone case of pldhash.h and leaves the others alone.

If you'd like an NS_BIT{,MASK} macro in an appropriate location, I could do that instead.  But this is a good incremental step towards removing prtypes.h in a lot of places.  And I'm sure Ehsan is getting tired of running his prtypes sed script every week. :)

Alternatively, I'm happy to commit this patch and leave the bug open to address the remaining PR_BIT* usages at a later point.

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: part 2 - don't use PR_BIT in pldhash.h

Here, how about we attach the correct patch to look at.

Comment 4

[Nathan Froyd [:froydnj]]

Attachment: part 1 - don't use PR_{BEGIN,END}_EXTERN_C

Touches a few more places than the previous patch.  There are still some left in typelib/, but I'm guessing that we're keeping the NSPR types there for backwards compatibility?  I'm not too worried about those headers leaking into the wild.

Comment 5

[Nathan Froyd [:froydnj]]

Attachment: part 3 - remove extraneous prtypes.h #includes

Basically everywhere that doesn't use PRUnichar.h or typelib/.

Comment 6

[(no longer active)]

Comment on attachment 666722 [details] [diff] [review]
part 1 - don't use PR_{BEGIN,END}_EXTERN_C

Seems like we're duplicating effort!  Bug 795507 landed on inbound this morning and it removed PR_BEGIN_EXTERN_C and PR_END_EXTERN_C.  :-)

Comment 7

[(no longer active)]

Comment on attachment 666708 [details] [diff] [review]
part 2 - don't use PR_BIT in pldhash.h

(I can review these types of patches, bsmedberg's time is probably better spent on other things. :-)

Comment 8

[(no longer active)]

Comment on attachment 666726 [details] [diff] [review]
part 3 - remove extraneous prtypes.h #includes

Nice!  Please test builds on try, I had a few surprises in bug 793417.  And if there are no more prtypes.h headers in xpcom after this, please dupe that bug against this.  Thanks!

Comment 9

[Nathan Froyd [:froydnj]]

Added prtypes.h to nsString.h as per IRC conversation; the long and short of it is that nsString.h depended on PRUnichar, but didn't include prtypes.h directly.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e38c5518605
https://hg.mozilla.org/integration/mozilla-inbound/rev/acf91f25f228

Comment 10

[(no longer active)]

I had to back this out as part of a weird reftest breakage on 64-bit platforms:

https://hg.mozilla.org/integration/mozilla-inbound/rev/402e2cf6602d

Example failure log: https://tbpl.mozilla.org/php/getParsedLog.php?id=15760281&tree=Mozilla-Inbound

Comment 11

[Nathan Froyd [:froydnj]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/715eda9ee772
https://hg.mozilla.org/integration/mozilla-inbound/rev/5cf45899da84

Comment 12

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/715eda9ee772
https://hg.mozilla.org/mozilla-central/rev/5cf45899da84