Failed to compile with -Wmissing-declarations

993 views
Skip to first unread message

Nicolas Desprès

unread,
Dec 15, 2010, 12:24:34 PM12/15/10
to Google C++ Testing Framework
Hi everybody,

When using the macro:

INSTANTIATE_TEST_CASE_P(InstantiationName,
FooTest,
::testing::Values("meeny",
"miny", "moe"));

I get a warning if I compile with -Wmissing-declarations one of my
test written with google test 1.5.0 (not google test itself):

warning: no previous declaration for
'testing::internal::ParamGenerator<const char*>
gtest_InstantiationNameFooTest_EvalGenerator_()'

gcc(1) says this about this warning:

-Wmissing-declarations
Warn if a global function is defined without a previous
declaration. Do so even if the definition itself provides
a
prototype. Use this option to detect global functions that
are not
declared in header files. In C++, no warnings are issued
for
function templates, or for inline functions, or for
functions in
anonymous namespaces.

Here the patch that solve the problem:

--- include/gtest/gtest-param-test.h (revision 527)
+++ include/gtest/gtest-param-test.h (working copy)
@@ -1406,6 +1406,8 @@

#define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) \
::testing::internal::ParamGenerator<test_case_name::ParamType> \
+ gtest_##prefix##test_case_name##_EvalGenerator_(); \
+ ::testing::internal::ParamGenerator<test_case_name::ParamType> \
gtest_##prefix##test_case_name##_EvalGenerator_() { return
generator; } \
int gtest_##prefix##test_case_name##_dummy_ = \
::testing::UnitTest::GetInstance()-
>parameterized_test_registry(). \

Shall I open an issue for this?

PS: Note that when compiling google test with this flag generate a lot
of warning. I don't know if it worth fixing them.

Cheers,
Nicolas

Zhanyong Wan (λx.x x)

unread,
Dec 15, 2010, 1:54:34 PM12/15/10
to Nicolas Desprès, Google C++ Testing Framework
2010/12/15 Nicolas Desprès <nicolas...@gmail.com>:

> Hi everybody,
>
> When using the macro:
>
> INSTANTIATE_TEST_CASE_P(InstantiationName,
>                                               FooTest,
>                                               ::testing::Values("meeny",
> "miny", "moe"));
>
> I get a warning if I compile with -Wmissing-declarations one of my

I'm wondering why you turn on this warning. Note that it's not even
enabled by -Wall.

IMHO, this warning is just too noise for normal C++ code and not worth
it. See http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01050.html for
an interesting discussion on it.

Please see http://code.google.com/p/googletest/wiki/FAQ?ts=1292438732&updated=FAQ#I'm_getting_warnings_when_compiling_Google_Test.__Would_you,
which I just wrote. Since this warning is rarely used, and fixing it
makes Google Test's code more complex, I'm reluctant to fix it. Could
you disable the warning or use -isystem? Thanks,

--
Zhanyong

Nicolas Desprès

unread,
Dec 16, 2010, 5:32:19 AM12/16/10
to Zhanyong Wan (λx.x x), Google C++ Testing Framework
2010/12/15 Zhanyong Wan (λx.x x) <w...@google.com>:

> 2010/12/15 Nicolas Desprès <nicolas...@gmail.com>:
>> Hi everybody,
>>
>> When using the macro:
>>
>> INSTANTIATE_TEST_CASE_P(InstantiationName,
>>                                               FooTest,
>>                                               ::testing::Values("meeny",
>> "miny", "moe"));
>>
>> I get a warning if I compile with -Wmissing-declarations one of my
>
> I'm wondering why you turn on this warning.  Note that it's not even
> enabled by -Wall.
>
> IMHO, this warning is just too noise for normal C++ code and not worth
> it.  See http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01050.html for
> an interesting discussion on it.
>

Thanks for the link. Although most of declarations in C++ are in
scope, C++ remains a super set of C, so if it makes sense for C so
does it for C++. I agree with Ian Lance Taylor.

In some rare cases (and I think Google Test is a rare case), people
write tricky code using a lot of macros and templates leading to be
border line with usual programming patterns. Warnings not included in
-Wall (or even -Wextra?) are convenient for more "conventional" code.

Thanks for the FAQ extension and for -isystem tips. I did not know it.
Actually, it solves part of the problem. Google Test provides macros
that expand code in the source test file. Thus, this code does not
have the special treatment that -isystem gives. FYI, there are issues
with -Wswitch-default and -Wswitch-enum too and probably even more.
But it is an endless list, I think.

I understand perfectly, that you cannot possibly handle all the
combinations of compilers x warning-flags without making the code even
more complex than it is already.

I will disable those flags for the test.

Thanks for your time.

--
Nicolas Desprès

Zhanyong Wan (λx.x x)

unread,
Dec 28, 2010, 1:41:20 PM12/28/10
to Nicolas Desprès, Google C++ Testing Framework
2010/12/16 Nicolas Desprès <nicolas...@gmail.com>:

> 2010/12/15 Zhanyong Wan (λx.x x) <w...@google.com>:
>> 2010/12/15 Nicolas Desprès <nicolas...@gmail.com>:
>>> Hi everybody,
>>>
>>> When using the macro:
>>>
>>> INSTANTIATE_TEST_CASE_P(InstantiationName,
>>>                                               FooTest,
>>>                                               ::testing::Values("meeny",
>>> "miny", "moe"));
>>>
>>> I get a warning if I compile with -Wmissing-declarations one of my
>>
>> I'm wondering why you turn on this warning.  Note that it's not even
>> enabled by -Wall.
>>
>> IMHO, this warning is just too noise for normal C++ code and not worth
>> it.  See http://gcc.gnu.org/ml/gcc-patches/2006-12/msg01050.html for
>> an interesting discussion on it.
>>
>
> Thanks for the link. Although most of declarations in C++ are in
> scope, C++ remains a super set of C, so if it makes sense for C so
> does it for C++. I agree with Ian Lance Taylor.

FYI

I'm not saying that the warning doesn't make sense for C++. I'm
saying that it's way too noisy for C++ that it's not worth it. It may
uncover some bugs, but it also generates a lot of work to bring the
code into compliance, which means lost productivity. You have to
weigh the benefit against the cost. My hunch is that the cost is
greater than the benefit in most C++ code bases.

There is a reason why the company Ian Lance Taylor works for doesn't
use this flag. ;-)

--
Zhanyong

Malcolm Parsons

unread,
Mar 9, 2012, 4:46:00 AM3/9/12
to googletes...@googlegroups.com
On Wednesday, 15 December 2010 17:24:34 UTC, Nicolas Desprès wrote:
When using the macro:

INSTANTIATE_TEST_CASE_P(InstantiationName,
                                               FooTest,
                                               ::testing::Values("meeny",
"miny", "moe"));

I get a warning if I compile with -Wmissing-declarations one of my
test written with google test 1.5.0 (not google test itself):

warning: no previous declaration for
'testing::internal::ParamGenerator<const char*>
gtest_InstantiationNameFooTest_EvalGenerator_()'

Here the patch that solve the problem:

--- include/gtest/gtest-param-test.h        (revision 527)
+++ include/gtest/gtest-param-test.h        (working copy)
@@ -1406,6 +1406,8 @@

 #define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) \
   ::testing::internal::ParamGenerator<test_case_name::ParamType> \
+    gtest_##prefix##test_case_name##_EvalGenerator_(); \
+  ::testing::internal::ParamGenerator<test_case_name::ParamType> \
       gtest_##prefix##test_case_name##_EvalGenerator_() { return
generator; } \
   int gtest_##prefix##test_case_name##_dummy_ = \
       ::testing::UnitTest::GetInstance()-
>parameterized_test_registry(). \

Here's a better patch:

--- include/gtest/gtest-param-test.h
+++ include/gtest/gtest-param-test.h
@@ -1404,7 +1404,7 @@ internal::CartesianProductHolder10<Generator1, Generator2, Generator3,
   void GTEST_TEST_CLASS_NAME_(test_case_name, test_name)::TestBody()
 
 # define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator) \
-  ::testing::internal::ParamGenerator<test_case_name::ParamType> \
+  static ::testing::internal::ParamGenerator<test_case_name::ParamType> \
       gtest_##prefix##test_case_name##_EvalGenerator_() { return generator; } \
   int gtest_##prefix##test_case_name##_dummy_ = \
       ::testing::UnitTest::GetInstance()->parameterized_test_registry(). \
 
Please apply.

Thanks,
Reply all
Reply to author
Forward
0 new messages