919865 - Replace #ifdef DEBUG with DebugOnly<> in a few spots in layout/generic

Status: RESOLVED FIXED

(Core :: Layout, defect)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1

Comment 2

[Mats Palmgren (inactive)]

Comment on attachment 808983 [details] [diff] [review]
fix v1

In the first hunk, the "aNextInFlow->GetPrevInFlow()" adds a virtual
call in Opt builds.  Perhaps it's possible to do the assignment inside
the assertion?

It looks like most of the other ones could fit on the same line?

Comment 3

[Daniel Holbert [:dholbert]]

(In reply to Mats Palmgren (:mats) from comment #2)
> In the first hunk, the "aNextInFlow->GetPrevInFlow()" adds a virtual
> call in Opt builds.  Perhaps it's possible to do the assignment inside
> the assertion?

Oops! thanks for catching that.  I just won't bother with that hunk -- the variable is used later in the function, too, and moving the assignment into the assertion feels hackier than what we have right now, so I'll just leave that as-is.

> It looks like most of the other ones could fit on the same line?

True. I'll fix those and re-post.

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: [ignore]

This version has that first hunk removed, and the remaining hunks converted to put things on one line.

(In the final hunk, I reduced the odd 4-space indentation in the contextual code, since most of the rest of that file uses our normal 2-space indentation, and since I needed to reduce it anyway to get it into 80 characters.)

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 809557 [details] [diff] [review]
[ignore]

sorry, forgot to qref; let's try that again

Comment 6

[Daniel Holbert [:dholbert]]

Attachment: fix v2

ok, here's the right version.

Comment 7

[Mats Palmgren (inactive)]

Comment on attachment 809558 [details] [diff] [review]
fix v2

> moving the assignment into the assertion feels hackier than what we have
> right now, so I'll just leave that as-is.

Yeah, that's a good call.  r=mats

Comment 8

[Daniel Holbert [:dholbert]]

needinfo=me to remember to land this

Comment 9

[Daniel Holbert [:dholbert]]

Sanity-check try run: https://tbpl.mozilla.org/?tree=Try&rev=56ee8a42a017

Comment 10

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d43432aad57d

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d43432aad57d