577914 - mark DEBUG only variables as ifdef DEBUG in layout

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[timeless]

this is part of a crusade to get rid of compilation warnings

Comment 1

[timeless]

Attachment: patch

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This makes the code pretty unreadable; I don't think this is worth it to get rid of compiler warnings.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 456741 [details] [diff] [review]
patch

see previous comment

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

An alternative that might be a *little* more readable would be a general NS_IF_DEBUG() macro that makes its argument appear ifdef debug, and disappear otherwise, so you'd write:

  NS_IF_DEBUG(nsIContent* propagatedScrollFrom =) PropagateScrollToViewport();

It still makes our code a lot less approachable to outsiders, though, so I'm not especially happy about that, either.

Comment 5

[timeless]

Attachment: using #ifdef DEBUG

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 463501 [details] [diff] [review]
using #ifdef DEBUG

This should land after we branch for Gecko 2.0 / Firefox 4.

Comment 7

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/03c135d7887a

Comment 8

[Jonathan Kew [:jfkthame]]

Did we really want to do this (and the other similar patches for other modules that just landed as well) at this point, in view of the work-in-progress in bug 577899?

Comment 9

[(no longer active)]

(In reply to comment #8)
> Did we really want to do this (and the other similar patches for other modules
> that just landed as well) at this point, in view of the work-in-progress in bug
> 577899?

These patches were reviewed, that's why I landed them.  I personally didn't know about bug 577899 before this, but I'm not sure what we want to do now (back all of those patches out, switch them over to NS_DEBUG_ONLY, etc).