Closed Bug 1245610 Opened 8 years ago Closed 8 years ago

Enable -Wsign-compare for gtests

Categories

(NSS :: Test, defect)

defect
Not set
normal

Tracking

(firefox47 affected)

RESOLVED FIXED
Tracking Status
firefox47 --- affected

People

(Reporter: mt, Assigned: mt)

Details

Attachments

(1 file)

On some platforms gtests generate errors for sign comparison that aren't covered by -Wsign-compare.  We don't enable that warning because it's too noisy in most of the tree.  However, in the gtests it causes surprising build failures on some platforms, so we should probably turn it on for gtests.
Attached patch bug1245610.patchSplinter Review
This adds -Wsign-compare if it isn't already set.  I test for -Werror rather than trying to determine which platform we are on and all that business for the sake of simplicity.  On Windows, just remove the level downgrade on C4018.
Assignee: nobody → martin.thomson
Status: NEW → ASSIGNED
Attachment #8715586 - Flags: review?(ekr)
Comment on attachment 8715586 [details] [diff] [review]
bug1245610.patch

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

LGTM
Attachment #8715586 - Flags: review?(ekr) → review+
https://hg.mozilla.org/projects/nss/rev/fefbddf8427d
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → 3.23
You need to log in before you can comment on or make changes to this bug.