Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Suppress clang-tidy warning about vararg usage in assertion macros #1901

Merged

Conversation

moha-gh
Copy link
Contributor

@moha-gh moha-gh commented Apr 2, 2020

Description

CATCH_INTERNAL_IGNORE_BUT_WARN() introduced with b7b346c (*) triggers clang-tidy warning cppcoreguidelines-pro-type-vararg for every usage of assertion macros like CHECK() and REQUIRE(). Silence it via NOLINT in the #if defined(__clang__) block only, as clang-tidy honors those.

(*) Thanks for that one BTW, uncovered a few warnings in our test code.

GitHub Issues

n/a (decided to try to file PR instead of creating a ticket)

CATCH_INTERNAL_IGNORE_BUT_WARN() introduced with b7b346c triggers
clang-tidy warning 'cppcoreguidelines-pro-type-vararg' for every usage
of assertion macros like CHECK() and REQUIRE(). Silence it via NOLINT
in the '#if defined(__clang__)' block only, as clang-tidy honors those.
@codecov
Copy link

codecov bot commented Apr 2, 2020

Codecov Report

Merging #1901 into master will not change coverage by %.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1901   +/-   ##
=======================================
  Coverage   86.49%   86.49%           
=======================================
  Files         131      131           
  Lines        3900     3900           
=======================================
  Hits         3373     3373           
  Misses        527      527           

@horenmar
Copy link
Member

horenmar commented Apr 2, 2020

Okay, but I have no idea where function-level varargs are used. 🤔

@horenmar horenmar merged commit d399a30 into catchorg:master Apr 2, 2020
@moha-gh moha-gh deleted the mh/suppress-clang-tidy-va-args-warning branch April 3, 2020 06:09
@moha-gh
Copy link
Contributor Author

moha-gh commented Apr 3, 2020

I checked again. It actually complains about a call to a "c-style vararg function" and points to the invocation of __builtin_constant_p(), which from a very cursory "I have no idea what I'm doing here" glance at the LLVM sources indeed seems to have a ... in its signature.

warning: do not call c-style vararg functions [cppcoreguidelines-pro-type-vararg]
[...]
#    define CATCH_INTERNAL_IGNORE_BUT_WARN(...) (void)__builtin_constant_p(__VA_ARGS__)
                                                      ^

@horenmar
Copy link
Member

horenmar commented Apr 4, 2020

Fair.

horenmar added a commit that referenced this pull request Apr 21, 2020
--- Improvements ---
* Running tests in random order (`--order rand`) has been reworked significantly (#1908)
  * Given same seed, all platforms now produce the same order
  * Given same seed, the relative order of tests does not change if you select only a subset of them
* Vector matchers support custom allocators (#1909)
* `|` and `&` (bitwise or and bitwise and) are now supported in `CHECK` and `REQUIRE`
  * The resulting type must be convertible to `bool`

--- Fixes ---
* Fixed computation of benchmarking column widths in ConsoleReporter (#1885, #1886)
* Suppressed clang-tidy's `cppcoreguidelines-pro-type-vararg` in assertions (#1901)
  * It was a false positive trigered by the new warning support workaround
* Fixed bug in test specification parser handling of OR'd patterns using escaping (#1905)

--- Miscellaneous ---
* Worked around IBM XL's codegen bug (#1907)
  * It would emit code for _destructors_ of temporaries in an unevaluated context
* Improved detection of stdlib's support for `std::uncaught_exceptions` (#1911)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants