522779 - Build warning: "nsCSSFrameConstructor.cpp:6976: warning: comparison between signed and unsigned integer expressions", from comparing nsTArray::IndexOf to kNotFound

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Daniel Holbert [:dholbert]]

Build warning:
/mozilla/layout/base/nsCSSFrameConstructor.cpp: In function ‘void DoDeletingFrameSubtree(nsFrameManager*, nsTArray<nsIFrame*>&, nsIFrame*, nsIFrame*)’:
/mozilla/layout/base/nsCSSFrameConstructor.cpp:6976: warning: comparison between signed and unsigned integer expressions

The line it refers to is:
> 6976       NS_ASSERTION(aDestroyQueue.IndexOf(outOfFlowFrame) == kNotFound,
http://mxr.mozilla.org/mozilla-central/source/layout/base/nsCSSFrameConstructor.cpp#6976

This comparison is bad, because nsTArray::IndexOf returns unsigned, whereas kNotFound is -1.

kNotFound is the sentinel value for strings -- it's in nsAString.h.
nsTArray, on the other hand, should use "NoIndex" -- or better, its "Contains()" method.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix: use nsTArray::Contains

Comment 2

[Daniel Holbert [:dholbert]]

Looks like this bug was caused by bug 474369, which switched aDestroyQueue from being a nsVoidArray (whose IndexOf returns signed values) to nsTArray (whose IndexOf returns unsigned values).

Comment 3

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

Comment on attachment 406779 [details] [diff] [review]
fix: use nsTArray::Contains

r=dbaron

Comment 4

[Daniel Holbert [:dholbert]]

Attachment: fix: use nsTArray::Contains

I did a quick MXR for usages of IndexOf in layout, to see if I could catch any more instances of this bug.

I didn't find any, but I did find two instances of comparing "IndexOf(foo)" to SomethingComplicated::NoIndex, which can be replaced with a simple "Contains" call.  This patch cleans those up, too.

Comment 5

[Daniel Holbert [:dholbert]]

Comment on attachment 406784 [details] [diff] [review]
fix: use nsTArray::Contains

dbaron -- thanks for the fast review on the first patch!

Could you take a quick glance over the two related changes in the new version?

Comment 6

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

Comment on attachment 406784 [details] [diff] [review]
fix: use nsTArray::Contains

r=dbaron

Comment 7

[Daniel Holbert [:dholbert]]

pushed attachment 406784 [details] [diff] [review]: http://hg.mozilla.org/mozilla-central/rev/8f9e84a49399