Closed Bug 1018270 Opened 6 years ago Closed 6 years ago

Cherrypick -Wunused-private-field warning fix from upstream gtest

Categories

(Testing :: General, defect)

x86
macOS
defect
Not set

Tracking

(firefox31 wontfix, firefox32 fixed)

RESOLVED FIXED
mozilla32
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

clang on OS X reports the following gtest warning:

 testing/gtest/gtest/src/gtest-internal-inl.h:206:8 [-Wunused-private-field] private field 'pretty_' is not used

This warning has been fixed in upstream gtest, so we can cherrypick that trivial fix into our tree:

 https://code.google.com/p/googletest/source/detail?r=640

Also fix warning in Mozilla code:

 testing/gtest/mozilla/GTestRunner.cpp:101:16: error: unused variable 'rv' [-Wunused-variable]

Mark testing/gtest as FAIL_ON_WARNINGS. Green try build:

https://tbpl.mozilla.org/?tree=Try&rev=7cb3705993d6
Attachment #8431638 - Flags: review?(ted)
FWIW, this annotation makes warnings-as-errors builds fail in GCC 4.8 and higher (and 4.8 is the default in current Ubuntu, version 14.04), with errors like
{
testing/gtest/mozilla/SanityTest.cpp: In member function 'testing::internal::Function<void()>::Result {anonymous}::TestMock::MockedCall()':
testing/gtest/gmock/include/gmock/gmock-generated-function-mockers.h:351:9: error: typedef 'this_method_does_not_take_0_arguments' locally defined but not used [-Werror=unused-local-typedefs]
}

We should probably fix those Wunused-local-typedefs warnings (using one of the strategies from bug 851237) before labeling any gtest directory as FAIL_ON_WARNINGS, or else a lot of local builds are going to break.
Comment on attachment 8431638 [details] [diff] [review]
cherrypick-gtest-warning-fix.patch

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

r=me, but we should probably fix dholbert's issue before landing the annotation.
Attachment #8431638 - Flags: review?(ted) → review+
I landed warning fix without FAIL_ON_WARNINGS annotation for now:
https://hg.mozilla.org/integration/mozilla-inbound/rev/a456dc7f19c4
Rather than marking this as leave-open, might be better to address the unused-local-typedefs in its own bug, and then land the FAIL_ON_WARNINGS annotation when both are fixed.  (That way, if we don't get to the annotation for whatever reason, this bug isn't left in a still-open half-fixed state.)

(In that spirit, I spun off bug 1019382 for the unused-local-typedefs issue. But if you end up preferring to take care of it all on this bug, feel free to dupe that back to this bug.)
Blocks: 1019382
Keywords: leave-open
Summary: Cherrypick -Wunused-private-field warning fix from upstream gtest and mark testing/gtest as FAIL_ON_WARNINGS → Cherrypick -Wunused-private-field warning fix from upstream gtest
https://hg.mozilla.org/mozilla-central/rev/a456dc7f19c4
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
See Also: → 1295687
You need to log in before you can comment on or make changes to this bug.