Closed Bug 937157 Opened 7 years ago Closed 7 years ago

Remove some ancient or unused debugging #defines

Categories

(Core :: XPConnect, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: mccr8, Assigned: mccr8)

Details

(Whiteboard: [qa-])

Attachments

(2 files)

No description provided.
Yes please! I don't know of anyone who builds as "xpc_hacker" anymore.
Attached patch remove someSplinter Review
I removed some jband debug flags, and some other flags that aren't used, plus a few other misc things.  We appear to be vulnerable to LEADING_UPPERCASE_ACCESS_ERRORS.
(In reply to Bobby Holley (:bholley) from comment #1)
> Yes please! I don't know of anyone who builds as "xpc_hacker" anymore.

I just did some basic low-risk removal.  I can remove more of the DEBUG_xpc_hacker stuff after surveying people if you'd like.
Attachment #830228 - Flags: review?(bobbyholley+bmo)
Attachment #830228 - Flags: review?(bobbyholley+bmo) → review+
(In reply to Andrew McCreight [:mccr8] from comment #3)
> I just did some basic low-risk removal.  I can remove more of the
> DEBUG_xpc_hacker stuff after surveying people if you'd like.

I don't think a survey is necessary. I'm 90% certain that DEBUG_xpc_hacker builds wouldn't even compile anymore. Feel free to remove them if you feel so inclined. We can always add them back if igor comes out of the woodwork and claims that we broke his workflow. ;-)
In email and IRC, I spoke with dbaron, jst, timeless, and bz, and none of them have used any of this stuff in forever, so I'll work on some patches to remove more of it.  (Ms2ger asked mrbkap, and he isn't using any of this either.)
Attached patch remove some moreSplinter Review
Attachment #830919 - Flags: review?(bobbyholley+bmo)
Comment on attachment 830919 [details] [diff] [review]
remove some more

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

::: js/xpconnect/src/XPCWrappedNative.cpp
@@ -3013,5 @@
>  
>  /***************************************************************************/
>  
> -#ifdef XPC_CHECK_CLASSINFO_CLAIMS
> -static void DEBUG_CheckClassInfoClaims(XPCWrappedNative* wrapper)

This seems like it could still be useful to run #ifdef DEBUG and have it MOZ_ASSERT if it finds problems.
Attachment #830919 - Flags: review?(bobbyholley+bmo) → review+
Landed in 13 pieces as part of this push:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?changeset=5cb70c98cbb6

I left in XPC_CHECK_CLASSINFO_CLAIMS.  I filed bug 937157 for enabling this in debug builds.

L64 debug try run was green: https://tbpl.mozilla.org/?tree=Try&rev=8ab149fedc09
Looks like around 800 lines removed in total.
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.