I'm going to make a bunch of changes to xpt_struct.h in bug 1438688, so I think it makes sense to go through and do some clean ups before that. This includes a few things: - Cleaning up whitespace: making the file 2-space indented, getting rid of the extra space that tries to line up member names, and moving the * next to the type instead of the field name. - Moving things out of the file that aren't used there to where they are used: an include of xpt_arena.h, various macros that are only used for loading a file. - Removing things that aren't used at all: various macros, and the minor_version field of XPTHeader. - There's a ton of macros related to bit flags that I have turned into constants and functions. Most access of these flags goes through the helper classes in xptinfo.h, so this doesn't require too many changes. I have deliberately not spent any time cleaning up xpt_arena.h, xpt_arena.cpp, xpt_xdr.h, xpt_xdr.cpp, and xpt_struct.cpp because if bug 1438688 works out those files can get deleted entirely. Hopefully that's not too weird. I decided to not change the member names to the mFoo form, because it doesn't bother me all that much and there are a lot of fields. Combined, this makes the file about 60 lines shorter.

(In reply to Andrew McCreight [:mccr8] from comment #0) > - Cleaning up whitespace: making the file 2-space indented, getting rid of > the extra space that tries to line up member names, and moving the * next to > the type instead of the field name. Suggestion: Why not just run `mach clang-format`?

(In reply to Gerald Squelart [:gerald] from comment #1) > Suggestion: Why not just run `mach clang-format`? I didn't know how to run clang format. The file's not that big so I just did it by hand.

> I didn't know how to run clang format. The file's not that big so I just did > it by hand. ./mach clang-format -p path/to/files

Comment on attachment 8954553 [details] Bug 1441677, part 1 - Fix spacing in xpt_struct.h. https://reviewboard.mozilla.org/r/223594/#review229702

Comment on attachment 8954554 [details] Bug 1441677, part 2 - Move things only used in xpt_struct.cpp there. https://reviewboard.mozilla.org/r/223596/#review229704

Comment on attachment 8954555 [details] Bug 1441677, part 3 - Use a function to get the tag of an XPT type. https://reviewboard.mozilla.org/r/223598/#review229708

Comment on attachment 8954556 [details] Bug 1441677, part 4 - Use functions to check XPTInterfaceDescriptor::flags. https://reviewboard.mozilla.org/r/223600/#review229716

Comment on attachment 8954557 [details] Bug 1441677, part 5 - Demacroize nsXPTMethodInfo masks. https://reviewboard.mozilla.org/r/223602/#review229718 ::: xpcom/reflect/xptinfo/xptinfo.h:184 (Diff revision 1) > // NO DATA - this a flyweight wrapper > public: > MOZ_IMPLICIT nsXPTMethodInfo(const XPTMethodDescriptor& desc) > {*(XPTMethodDescriptor*)this = desc;} > > - bool IsGetter() const {return 0 != (XPT_MD_IS_GETTER(flags) );} > + bool IsGetter() const {return 0 != (flags & kGetterMask);} Use `!!` instead of `0 !=` ?

Comment on attachment 8954558 [details] Bug 1441677, part 6 - Get rid of macros for nsXPTParamInfo flags. https://reviewboard.mozilla.org/r/223604/#review229720 ::: xpcom/reflect/xptinfo/xptinfo.h:126 (Diff revision 1) > public: > MOZ_IMPLICIT nsXPTParamInfo(const XPTParamDescriptor& desc) > {*(XPTParamDescriptor*)this = desc;} > > > - bool IsIn() const {return 0 != (XPT_PD_IS_IN(flags));} > + bool IsIn() const {return 0 != (flags & kInMask);} Again, use `!!` instead? ::: xpcom/reflect/xptinfo/xptinfo.h:148 (Diff revision 1) > // converts it to an 'in', and sets the XPT_PD_DIPPER flag on it. For this > // reason, dipper types are sometimes referred to as 'out parameters > // masquerading as in'. The burden of maintaining this illusion falls mostly > // on XPConnect, which creates the empty containers, and harvest the results > // after the call. > - bool IsDipper() const {return 0 != (XPT_PD_IS_DIPPER(flags));} > + bool IsDipper() const {return 0 != (flags & kDipperMask);} ditto

Nice cleanups.

Sure, I'll use !! everywhere. I wasn't sure what was the right thing to do, so I just left it alone. (In reply to Nicholas Nethercote [:njn] from comment #18) > Nice cleanups. Thanks. And thanks for the fast reviews.

Pushed by amccreight@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/fbe29e607536 part 1 - Fix spacing in xpt_struct.h. r=njn https://hg.mozilla.org/integration/autoland/rev/61913a1d556e part 2 - Move things only used in xpt_struct.cpp there. r=njn https://hg.mozilla.org/integration/autoland/rev/5abfcbcd30d1 part 3 - Use a function to get the tag of an XPT type. r=njn https://hg.mozilla.org/integration/autoland/rev/10ea4ebc9d54 part 4 - Use functions to check XPTInterfaceDescriptor::flags. r=njn https://hg.mozilla.org/integration/autoland/rev/c35703f90f60 part 5 - Demacroize nsXPTMethodInfo masks. r=njn https://hg.mozilla.org/integration/autoland/rev/ded8385fa298 part 6 - Get rid of macros for nsXPTParamInfo flags. r=njn

https://hg.mozilla.org/mozilla-central/rev/fbe29e607536 https://hg.mozilla.org/mozilla-central/rev/61913a1d556e https://hg.mozilla.org/mozilla-central/rev/5abfcbcd30d1 https://hg.mozilla.org/mozilla-central/rev/10ea4ebc9d54 https://hg.mozilla.org/mozilla-central/rev/c35703f90f60 https://hg.mozilla.org/mozilla-central/rev/ded8385fa298