Wsign-compare build warning for TestAsyncPanZoomController.cpp, from "EXPECT_EQ(1, aLayer->GetFrameMetricsCount());"

RESOLVED FIXED in Firefox 34, Firefox OS v2.1

Status

()

RESOLVED FIXED
4 years ago
2 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug)

Trunk
mozilla35
x86_64
Linux
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite -
qe-verify -

Firefox Tracking Flags

(firefox33 unaffected, firefox34 fixed, firefox35 fixed, b2g-v2.1 fixed, b2g-v2.2 fixed)

Details

Attachments

(1 attachment)

(Assignee)

Description

4 years ago
I get this build-warning-spew in clang in current mozilla-central:
{
 0:02.98 In file included from /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/gfx/tests/gtest/TestAsyncPanZoomController.cpp:6:
 0:02.98 Warning: -Wsign-compare in /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/obj/dist/include/gtest/gtest.h: comparison of integers of different signs: 'const int' and 'const unsigned int'
 0:02.98 ../../../dist/include/gtest/gtest.h:1316:16: warning: comparison of integers of different signs: 'const int' and 'const unsigned int' [-Wsign-compare]
 0:02.98   if (expected == actual) {
 0:02.98       ~~~~~~~~ ^  ~~~~~~
 0:02.98 ../../../dist/include/gtest/gtest.h:1352:12: note: in instantiation of function template specialization 'testing::internal::CmpHelperEQ<int, unsigned int>' requested here
 0:02.98     return CmpHelperEQ(expected_expression, actual_expression, expected,
 0:02.98            ^
 0:02.98 /scratch/work/builds/mozilla-central/mozilla-central.12-12-07.08-38/mozilla/gfx/tests/gtest/TestAsyncPanZoomController.cpp:1484:5: note: in instantiation of function template specialization 'testing::internal::EqHelper<false>::Compare<int, unsigned int>' requested here
 0:02.98     EXPECT_EQ(1, aLayer->GetFrameMetricsCount());
 0:02.98     ^
 0:02.98 ../../../dist/include/gtest/gtest.h:1848:67: note: expanded from macro 'EXPECT_EQ'
 0:02.98                       EqHelper<GTEST_IS_NULL_LITERAL_(expected)>::Compare, \
 0:02.98                                                                   ^
 0:02.98 ../../../dist/include/gtest/gtest_pred_impl.h:162:23: note: expanded from macro 'EXPECT_PRED_FORMAT2'
 0:02.98   GTEST_PRED_FORMAT2_(pred_format, v1, v2, GTEST_NONFATAL_FAILURE_)
 0:02.98                       ^
 0:02.98 ../../../dist/include/gtest/gtest_pred_impl.h:147:17: note: expanded from macro 'GTEST_PRED_FORMAT2_'
 0:02.98   GTEST_ASSERT_(pred_format(#v1, #v2, v1, v2),\
 0:02.98                 ^
 0:02.98 ../../../dist/include/gtest/gtest_pred_impl.h:77:52: note: expanded from macro 'GTEST_ASSERT_'
 0:02.98   if (const ::testing::AssertionResult gtest_ar = (expression)) \
 0:02.98                                                    ^
}

This is from this line, added for bug 1058886:
> EXPECT_EQ(1, aLayer->GetFrameMetricsCount());
http://hg.mozilla.org/mozilla-central/rev/07d1e106d250#l1.13

That needs to be "1u" to match the signedness of GetFrameMetricsCount.  That change fixes the warning-spew for me.
(Assignee)

Comment 1

4 years ago
Created attachment 8485306 [details] [diff] [review]
fix
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #8485306 - Flags: review?(bugmail.mozilla)
(Assignee)

Comment 2

4 years ago
(Note: if/when bug 1058886 is uplifted to Aurora, let's uplift this as well, since it's test-only, is a correctness fix, and makes builds quieter.)
Interestingly, the patch posted to bug 1058886 has the '1u', but the one that landed doesn't.
Comment on attachment 8485306 [details] [diff] [review]
fix

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

Stealing review.
Attachment #8485306 - Flags: review?(bugmail.mozilla) → review+
(Assignee)

Updated

4 years ago
Flags: in-testsuite-
Summary: -Wsign-compare build warning for TestAsyncPanZoomController.cpp, from "EXPECT_EQ(1, aLayer->GetFrameMetricsCount());" → Wsign-compare build warning for TestAsyncPanZoomController.cpp, from "EXPECT_EQ(1, aLayer->GetFrameMetricsCount());"
Thanks for fixing this! I remember adding the u because I ran into the error locally too but apparently I landed the older version of the patch rather than the new one. Whoops!
https://hg.mozilla.org/mozilla-central/rev/4914c3127025
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
https://hg.mozilla.org/releases/mozilla-aurora/rev/09e070cb6917
status-b2g-v2.1: --- → fixed
status-b2g-v2.2: --- → fixed
status-firefox33: --- → unaffected
status-firefox34: --- → fixed
status-firefox35: --- → fixed
Flags: qe-verify-
You need to log in before you can comment on or make changes to this bug.