Closed Bug 522779 Opened 10 years ago Closed 10 years ago

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

Categories

(Core :: Layout, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

(Whiteboard: [build_warning])

Attachments

(1 file, 1 obsolete file)

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.
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).
Blocks: 474369
Comment on attachment 406779 [details] [diff] [review]
fix: use nsTArray::Contains

r=dbaron
Attachment #406779 - Flags: review?(dbaron) → review+
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.
Attachment #406779 - Attachment is obsolete: true
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?
Attachment #406784 - Flags: review?(dbaron)
Comment on attachment 406784 [details] [diff] [review]
fix: use nsTArray::Contains

r=dbaron
Attachment #406784 - Flags: review?(dbaron) → review+
You need to log in before you can comment on or make changes to this bug.