Warnings reported by clang-tidy due to use of var_arg

[avitase]

Description
Using REQUIRE triggers clang-tidy to warn about the usage of var_arg. Using NOLINT at each and every call to REQUIRE resolves this issue but is very tedious... Maybe there is way to avoid the usage of var_arg (as suggested by core guidelines F.55) or maybe add NOLINT to the definition of the macro?

[horenmar]

Which version are you using?

[avitase]

catch2/2.11.0 via conan

[horenmar]

That's quite a bit behind. Have you tried updating?

[avitase]

sorry, my mistake. I mixed up the versions. The "error" occurs since catch2/2.11.3 (i.e. including catch2/2.12.1). Version catch2/2.11.1 is the last that is not "affected". (2.11.2 is not available via conan.)

[horenmar]

Interesting, that warning should already be suppressed since v2.12.0. Can you check why the NOLINT is not obeyed in your configuration?

[avitase]

I am not quite sure how. But maybe this error log helps? This is from version catch2/2.12.0 but it is the very same for catch2/2.12.1 (including the line numbers). You are right, obviously there is the /* NOLINT(cppcoreguidelines-pro-type-vararg) */ comment and I don't know why it is skipped. I cannot test it right now, but just two things that pop up to my mind: I always use NOLINT with // instead of a /* ... */ or maybe this issue is related to my old clang-tidy version 7.0.1 ...

[...] tests.cpp:10:3: error: do not call c-style vararg functions [hicpp-vararg,-warnings-as-errors]
  REQUIRE(Factorial(1) == 1);
  ^
[...] catch2/catch.hpp:17394:24: note: expanded from macro 'REQUIRE'
#define REQUIRE( ... ) INTERNAL_CATCH_TEST( "REQUIRE", Catch::ResultDisposition::Normal, __VA_ARGS__  )
                       ^
[...] catch2/catch.hpp:2698:9: note: expanded from macro 'INTERNAL_CATCH_TEST'
        CATCH_INTERNAL_IGNORE_BUT_WARN(__VA_ARGS__); \
        ^
[...] catch2/catch.hpp:166:55: note: expanded from macro 'CATCH_INTERNAL_IGNORE_BUT_WARN'
#    define CATCH_INTERNAL_IGNORE_BUT_WARN(...) (void)__builtin_constant_p(__VA_ARGS__) /* NOLINT(cppcoreguidelines-pro-type-vararg) */
                                                      ^

[horenmar]

@moha-gh Hey, can you confirm what clang-tidy version are you running?

[moha-gh]

@horenmar: We are using v9. I think the issue here is that for @avitase hicpp-vararg triggers, while the NOLINT is limited to cppcoreguidelines-pro-type-vararg. However, the hicpp warning is an alias for the cppcoreguidelines one. clang-tidy doesn't seem to take this into account when processing the NOLINT - even on v9: If I also enable hicpp-vararg in our config, I get the same warning locally. So I guess the solution would be to either extend the existing NOLINT

/* NOLINT(cppcoreguidelines-pro-type-vararg, hicpp-vararg) */

(works for me and my preference) or make it suppress every finding:

/* NOLINT */

[horenmar]

Ok, I will suppress the warning in both variants, and open up an issue with clang-tidy upstream.

[avitase]

thanks for your help

[horenmar]

Okay, this is now fixed in master.

I also opened two bugs in clang-tidy's bugzilla,

• one for silencing via aliases
• one for __builtin_constant_p