Googletest: INSTANTIATE_TEST_CASE_P generates warnings with clang++ -Wpedantic

[llugin]

Reproduction:

foo.cc

#include "gtest/gtest.h"

class Foo : public ::testing::TestWithParam<int> {
public:
  virtual void SetUp() { return; }
  virtual void TearDown() { return; }
};

TEST_P(Foo, FOO_TEST) { ASSERT_TRUE(GetParam()); }

INSTANTIATE_TEST_CASE_P(InstantiationName, Foo, ::testing::Values(1, 2, 3));

int main(int argc, char **argv) {
  ::testing::InitGoogleTest(&argc, argv);
  return RUN_ALL_TESTS();
} 

$ clang++ -Wall -Wpedantic -Werror -isystem ${GTEST_DIR}/include -pthread \
foo.cc ${GTEST_DIR}/build/libgtest.a -o foo

Output:

foo.cc:11:75: error: must specify at least one argument for '...' parameter of variadic macro [-Werror,-Wgnu-zero-variadic-macro-arguments]
INSTANTIATE_TEST_CASE_P(InstantiationName, Foo, ::testing::Values(1, 2, 3));
^
/my/dir/googletest/googletest//include/gtest/gtest-param-test.h:1421:10: note: macro 'INSTANTIATE_TEST_CASE_P' defined here
# define INSTANTIATE_TEST_CASE_P(prefix, test_case_name, generator, ...) \
^
1 error generated.

On the other hand, it compiles for g++ (checked on 7.2.0).

Tested on ubuntu 17.10, for clang versions:

• 4.0.1-6 (tags/RELEASE_401/final)
• 5.0.0-3 (tags/RELEASE_500/final)

Found on: 3c5e064ca089cbe3e89278e16c2c6cc3a97dba1f

[VSol-ll]

I have the same with GCC 7.1.

[improbable-nickkrempel]

Confirmed happening here too. Using the compiler option -Wno-gnu-zero-variadic-macro-arguments to suppress the warning as a workaround.

[patrick96]

Adding a comma after the last argument also solves this for me:

INSTANTIATE_TEST_CASE_P(InstantiationName, Foo, ::testing::Values(1, 2, 3),);
                                                                          ^

[gennadiycivil]

Thank you very much for this report. The best way to approach this would be to create a proper PR and submit it for consideration.

[marehr]

FYI, this warning was introduced in 03867b5389516a0f185af52672cf5472fa0c159c.

[gennadiycivil]

This issue has been opened for a while with some activity however it is not clear that there is anything that can/should be done in googletest as the existing workarounds seem to be sufficient.

[VSol-ll]

To my opinion there're two options:

• add a non-variadic macro INSTANTIATE_TEST_CASE_P0(prefix, test_case_name, generator)
• make generator a part of ellipses
  First option puts this on user to choose the right macro (just like adding comma, but without possible conflicts with other compilers).
  For second option there's a trick to extract head and tail from __VA_ARGS__

#define VA__GETFIRST(X, ...) X
#define VA_GETFIRST(...) VA__GETFIRST(__VA_ARGS__, 0) // add a fake argument to suppress this warning for VA__GETFIRST(only_one_arg)
#define VA_GETREST(X, ...) __VA_ARGS__ // This one should be used with a fake argument which can be suppressed by template. E.g. constructObjectIgnoringLastArg<VA_GETFIRST(__VA_ARGS__)>(VA_GETREST(__VA_ARGS__, 0))

Another way dealing with this is described here https://stackoverflow.com/questions/20818800/variadic-macro-and-trailing-comma

[mathbunnyru]

@VSol-ll so, the second solution doesn't require to change user-code, right?
Could you please create a PR?

Also, Ubuntu 18.04 for example uses newer gtest with this bug, so adapting to the new version of Ubuntu is problematic sometimes. This will make it easier.

[VSol-ll]

@mathbunnyru correct. Unfortunately I can't contribute from work.
Will try to find some time at home.

[mathbunnyru]

I think I wrote something and it works :) Please, review it - I'm new to this codebase.

[VSol-ll]

I believe it won't work for the case with a parameter after generator.
See https://en.cppreference.com/w/cpp/language/template_argument_deduction, "Non-deduced contexts", point 7):

template<class... Ts, class T> void f1(T n, Ts... args);
template<class... Ts, class T> void f2(Ts... args, T n);
f1(1, 2, 3, 4); // P1 = T, A1 = 1: deduced T = int
                // P2 = Ts..., A2 = 2, A3 = 3, A4 = 4: deduced Ts = [int, int, int]
f2(1, 2, 3, 4); // P1 = Ts...: Ts is non-deduced context

In addition you can look at this: https://godbolt.org/z/lhtUMp

But there's an obvious simplification. The thing is that GetParamNameGen() is overloaded (template) function with either one or zero arguments.

Two other points:

• style: functions in gtests are named in CamelCase
• your create_param_generator<>() is not intended for end user. I assume it should go to ::testing::internal

[mathbunnyru]

Fixed: style, function for 0 or 1 args.
Will fix later: namespace, CI.

[mathbunnyru]

@VSol-ll fix is in master. Please, check it to make sure everything is good.
After that I think issue could be closed.

[gennadiycivil]

We had to roll back the change. Stay tuned

[marehr]

We had to roll back the change. Stay tuned

@gennadiycivil So this was never fixed? Do you remember why this was reverted?

For me this issue is still open and since #2069 and #2271 reported the exact same issue, I think this issue should receive some love.

[staffantj]

Just a note to mention that this now also occurs in Visual Studio 2019 release 5.0, which was released earlier today. The "add a comma" work-around does not appear to work for MSVC.

[Andreas-Schniertshauer]

/wd4996 would solve it in Visual Studio 2019 16.5.0, but I don't like this.

[Andreas-Schniertshauer]

I decided to replace INSTANTIATE_TEST_CASE_P with INSTANTIATE_TEST_SUITE_P to solve the problem instead of using /wd4996.

[staffantj]

That does solve it for Visual Studio - thanks!

On Tue, Mar 24, 2020 at 5:35 AM Andreas-Schniertshauer <
[email protected]> wrote:

I decided to replace INSTANTIATE_TEST_CASE_P with INSTANTIATE_TEST_SUITE_P
to solve the problem instead of using /wd4996.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/google/googletest/issues/1419#issuecomment-603131017,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/ACCS5X47HLHLCKHT3QXEHSDRJB5FHANCNFSM4ENIJCQA
.