Closed Bug 1019382 Opened 5 years ago Closed 5 years ago

gmock-generated-function-mockers.h triggers gcc 4.8 build warnings like "typedef 'this_method_does_not_take_0_arguments' locally defined but not used [-Wunused-local-typedefs]"

Categories

(Testing :: General, defect)

x86_64
Linux
defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
mozilla37

People

(Reporter: dholbert, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

STR:
Build mozilla-central with GCC 4.8. (Default gcc version on Ubuntu 14.04)
(Optional: add FAIL_ON_WARNINGS to testing/gtest/moz.build to make warnings easier to notice)


ACTUAL RESULTS:
Build warnings like the following (with -Werror in the output here because I had FAIL_ON_WARNINGS):
{
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]
}

See also bug 1018270 for another warning-fix in this same directory.
This warning is for typedefs being shoehorned into static assertions.

We have at least 2 options for fixing this:

 (A) We could label the typedefs as intentionally-unused, like we did in https://hg.mozilla.org/mozilla-central/rev/9455756963a4 , back when we had MOZ_STATIC_ASSERT and when it caused this same problem.

 (B) We could switch this file to using C++11's "static_assert" (which the rest of our codebase has moved to, in bug 895322).
Depends on: 1018270
(We get these warnings in other gtest directories too, of course, like gfx/tests/gtest )
I started getting this on m-c and fx-team today, like:

 7:15.26 In file included from ../../../dist/include/gtest/internal/gtest-internal.h:40:0,
 7:15.26                  from ../../../dist/include/gtest/gtest.h:57,
 7:15.26                  from /home/tromey/firefox-git/gecko-dev/gfx/tests/gtest/TestAsyncPanZoomController.cpp:6,
 7:15.26                  from /home/tromey/firefox-git/gecko-dev/obj-x86_64-unknown-linux-gnu/gfx/tests/gtest/Unified_cpp_gfx_tests_gtest0.cpp:38:
 7:15.26 ../../../dist/include/gmock/gmock-actions.h: In member function ‘testing::internal::ReturnAction<R>::operator testing::Action<Func>() const’:
 7:15.26 ../../../dist/include/gmock/gmock-actions.h:471:9: error: typedef ‘use_ReturnRef_instead_of_Return_to_return_a_reference’ locally defined but not used [-Werror=unused-local-typedefs]


I have a fix following option A in comment #1.  I'll attach it shortly.
Comment on attachment 8533761 [details] [diff] [review]
use GTEST_ATTRIBUTE_UNUSED_ to avoid gcc 4.8 compiler warning

This patch fixes the subset of errors I saw while trying to build
m-c on Fedora 20 today, using --enable-warnings-as-errors.
I am not sure whether it is sufficient to fix this bug (I don't
seem to see the reported issues as-is) but it seems necessary.
Attachment #8533761 - Flags: review?(ted)
Comment on attachment 8533761 [details] [diff] [review]
use GTEST_ATTRIBUTE_UNUSED_ to avoid gcc 4.8 compiler warning

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

These are fine, but Google Test is also an upstream project, so if it's not already fixed there you should submit a patch upstream:
https://code.google.com/p/googletest/
Attachment #8533761 - Flags: review?(ted) → review+
(In reply to Ted Mielczarek [:ted.mielczarek] from comment #7)

> These are fine, but Google Test is also an upstream project, so if it's not
> already fixed there you should submit a patch upstream:
> https://code.google.com/p/googletest/

Thanks.

The gtest patch appears to already be upstream:
https://code.google.com/p/googletest/source/browse/trunk/include/gtest/internal/gtest-port.h#1054

I sent a note to the gmock forum with the original error message from gmock and
a pointer to both this bug and directly to the patch.  (I looked at filing a bug
or sending a patch, but the process for a patch requires a bug, and the bug tracker
says to send email instead.)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/ed809a292746
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
You need to log in before you can comment on or make changes to this bug.