Closed Bug 1153579 Opened 5 years ago Closed 5 years ago

widget/cocoa/nsNativeThemeCocoa.mm:857:33 [-Wpointer-bool-conversion] address of array 'settings.minimumSizes' will always evaluate to 'true'

Categories

(Core :: Widget: Cocoa, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

Remove unnecessary null check of settings.minimumSizes array because, as clang warns, arrays are never null. Also add some bounds-checking asserts as a sanity check because sizeIndex is calculated through a couple different code paths and used to index into two different arrays, minimumSizes and naturalSizes.
Attachment #8591261 - Flags: review?(mstange)
Attachment #8591261 - Flags: review?(mstange) → review+
Thanks! This was the only instance of -Wpointer-bool-conversion in mozilla-central. The -Wpointer-bool-conversion warning was added in Xcode's latest clang update.

https://hg.mozilla.org/integration/mozilla-inbound/rev/e82a14c4de34
https://hg.mozilla.org/mozilla-central/rev/e82a14c4de34
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
How embarrassing: my fix for the -Wpointer-bool-conversion warning introduced two -Wsign-compare warnings. :) I didn't catch this because the widget/cocoa directory is not FAIL_ON_WARNINGS yet.

sizeIndex and smallerControlSizeIndex are signed ints but my asserts compared them to size_t mozilla::ArrayLength(). This patch makes sizeIndex and smallerControlSizeIndex unsigned ints.

widget/cocoa/nsNativeThemeCocoa.mm:838:26 [-Wsign-compare] comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long')
widget/cocoa/nsNativeThemeCocoa.mm:858:24 [-Wsign-compare] comparison of integers of different signs: 'int' and 'size_t' (aka 'unsigned long')
Attachment #8592103 - Flags: review?(mstange)
Comment on attachment 8592103 [details] [diff] [review]
Wsign-compare_nsNativeThemeCocoa.patch

Since this variable is used to index into an array, size_t would be a better type to use. Can you also change the return type of EnumSizeForCocoaSize and change the other callers?
Changed EnumSizeForCocoaSize() and its callers to use size_t.
Attachment #8592103 - Attachment is obsolete: true
Attachment #8592103 - Flags: review?(mstange)
Attachment #8594047 - Flags: review?(mstange)
Comment on attachment 8594047 [details] [diff] [review]
Wsign-compare_nsNativeThemeCocoa-v2.patch

This patch is empty.
Attachment #8594047 - Flags: review?(mstange)
Attachment #8594047 - Attachment is obsolete: true
Comment on attachment 8594125 [details] [diff] [review]
Wsign-compare_nsNativeThemeCocoa-v2.patch

oops, sorry. Let's try this again.
Attachment #8594125 - Flags: review?(mstange)
Comment on attachment 8594125 [details] [diff] [review]
Wsign-compare_nsNativeThemeCocoa-v2.patch

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

I like!
Attachment #8594125 - Flags: review?(mstange) → review+
You need to log in before you can comment on or make changes to this bug.