4

Note December 2022: it seems that this problem is solved in clang-tidy 14. Just by experimentation I can reproduce the problem with clang-tidy 11, 12 and 13 but not with 14.

I have a Cmake project that uses Boost.UnitTest for testing. When I do static analysis with clang-tidy it reports some warnings from the Boost.UnitTest headers. I would like to filter those.

As an example (disregard detaisl)

/usr/include/boost/test/tools/old/interface.hpp:84:45: note: expanded from macro 'BOOST_REQUIRE'
#define BOOST_REQUIRE( P )                  BOOST_TEST_TOOL_IMPL( 2, \
                                            ^
/usr/include/boost/test/tools/old/interface.hpp:65:5: note: expanded from macro 'BOOST_TEST_TOOL_IMPL'
    BOOST_TEST_PASSPOINT();                                                     \
    ^
/usr/include/boost/test/unit_test_log.hpp:261:5: note: expanded from macro 'BOOST_TEST_PASSPOINT'
    ::boost::unit_test::unit_test_log.set_checkpoint(           \
    ^
/usr/include/boost/test/unit_test_log.hpp:209:82: note: default parameter was declared here
    void                set_checkpoint( const_string file, std::size_t line_num, const_string msg = const_string() );
                                                                                 ^
/home/user/prj/alf/boost/multi/test/zero_dimensionality.cpp:23:3: error: calling a function that uses a default argument is disallowed [fuchsia-default-arguments-calls,-warnings-as-errors]
                BOOST_REQUIRE( num_elements(m1) == 3 );

So far, I add the dependency on Boost.UnitTest with these lines

    target_link_libraries(${TEST_EXE} PRIVATE Boost::unit_test_framework Boost::serialization)

I tried with this, to make Boost.UnitTest a "system" library but I still get the same warnings

    target_include_directories(${TEST_EXE} SYSTEM PRIVATE ${Boost_INCLUDE_DIRS})
    target_link_libraries(${TEST_EXE} PRIVATE ${Boost_UNIT_TEST_FRAMEWORK_LIBRARY})
    target_link_libraries(${TEST_EXE} PRIVATE ${Boost_SERIALIZATION_LIBRARY})

but I still get the same results.

How can I prevent clang-tidy to check or report errors in the Boost headers?

(I accept answers changing the configuration of clang-tidy itself (I used a .clang-tidy configuration file); although it seems more elegant to change CMakeLists.txt instead.)

ADDED NOTE:

Following one of the recommendations in the comments I disabled these warnings that were "incompatible" with Boost.Test. I would still prefer to leave them on and make clang-tidy filter somehow the headers of Boost.Test:

#  -altera-unroll-loops,                                  // BOOST_REQUIRE macro requires this
#  -cert-err58-cpp,                                       // BOOST_AUTO_TEST_CASE macro requires this
#  -cppcoreguidelines-avoid-non-const-global-variables,   // BOOST_AUTO_TEST_CASE macros require this
#  -cppcoreguidelines-macro-usage,                        // BOOST_TEST_MODULE macro requires this
#  -cppcoreguidelines-pro-type-vararg,                    // BOOST_REQUIRE macros require this
#  -fuchsia-default-arguments-declarations                // BOOST_AUTO_TEST_CASE_TEMPLATE
#  -fuchsia-default-arguments-calls,                      // BOOST_REQUIRE macros require this
#  -fuchsia-statically-constructed-objects,               // BOOST_AUTO_TEST_CASE creates these
#  -hicpp-vararg,                                         // all BOOST_TEST_REQUIRE macros require this
alfC
  • 14,261
  • 4
  • 67
  • 118
  • 1
    Most likely you can't do anything else but disabling some specific checks for tests. I had the same issue with GTest and disabled some checks for UT subprojects. The problem isn't in boost headers but in macros which as far as tidy is concerned are in your code and not in boost headers. – ixSci Apr 10 '22 at 10:59
  • 2
    Including as system headers normally does the job. (I use that). Please include HOW you run clang-tidy + a minimal self-contained CMakeLists.txt (containing just a single target .cpp) – sehe Apr 10 '22 at 19:15
  • @sehe i do `CXX=clang++ cmake .. -DCMAKE_CXX_CLANG_TIDY="clang-tidy"` (plus a configuration file for clang-tidy) – alfC Apr 10 '22 at 21:50
  • If you can add that plus a self-contained CMakeLists.txt I'll repro it and see whether I can fix it – sehe Apr 10 '22 at 21:51
  • @ixSci yes. i understand that for macros the warnings are technically in my code. (although clang-tidy could be intelligent about it). however, some warnings are in the Boost header file explicitly, like the one in the example. i would like to know how to suppress both smartly even if i have to use separate solutions. warnings in dependent headers are one thing but in this case it is really annoying because it is simply coming from the way i test my library. – alfC Apr 10 '22 at 21:54
  • @sehe thank you. https://gitlab.com/correaa/boost-multi. given your background i think you will be interested in this library anyway :). – alfC Apr 10 '22 at 21:57
  • @sehe note that currently i am suppressing the warnings in a blanket form in the .clang-tidy file. – alfC Apr 10 '22 at 22:04
  • @alfC looks like a nice library indeed. For a moment I was thinking: "whoa what kind of steroids are these when there's a build dependency on fftw3" - but I guess that's just a test/example? I do notice there are a *lot* of places where Boost include directories are added to targets. On my system I cannot compile enough of things to actually see a wayward clang-tidy diagnostic, but for comparison, here's my patch on 0599128c to mark them ALL as SYSTEM: http://stackoverflow-sehe.s3.amazonaws.com/4f985326-9f74-4295-bc9f-646bb2ada5bd/0001-make-Boost-includes-SYSTEM.patch – sehe Apr 10 '22 at 23:06
  • 1
    @sehe that is right. the fftw is for an “adaptor” and for the test example. you don’t need fftw or blas for using the core of the library. Although anecdotally fftw probably provides the fastest in-place and out-of-place *transpositions* for complex arrays. Which i though about incorporating as a backend but failed because it is very specific to complex arrays and in CPU memory, not to mention that fftw allocates scratch space and “plans”. – alfC Apr 10 '22 at 23:45
  • If the warnings are really just about headers and not your code (generated from macros), do you have `HeaderFilterRegex` set in your clang-tidy? – ixSci Apr 11 '22 at 04:56
  • @ixSci, I have this, which I copied from somewhere `HeaderFilterRegex: '.'`. Can I use it filter warnings from particlar headers?, e.g. `/usr/include/boost/test/unit_test_log.hpp` (like in the example above). – alfC Apr 11 '22 at 05:14
  • You should set it to the pattern matching headers you want. For example, if you have your sources in `src` folder then you could set it to `HeaderFilterRegex: "src/"` meaning it should produce warnings only from the headers matching this pattern. – ixSci Apr 11 '22 at 05:29
  • Would these be absolute paths? I tried adding `HeaderFilterRegex: "include/multi/"` and `HeaderFilterRegex: "multi/"` and I get the same warnings (as in the example above). https://gitlab.com/correaa/boost-multi/-/blob/master/.clang-tidy#L4 – alfC Apr 11 '22 at 06:12
  • 1
    I can't be sure as this property isn't documented anywhere but as far as I can tell what you put there should be enough. Regex match is a substring matching. But as I said before, I don't believe that your problem is in boost headers but in your code. Look at the error you have: _"calling a function <...>"_ and it points to your source file. The function gets called in your code, not boost. – ixSci Apr 11 '22 at 06:35
  • @ixSci , given the lack of a better solution, I ended up commenting several warnings, see the note added in my question. – alfC May 11 '22 at 20:22

3 Answers3

1

Okay, I't not entirely clear (to me) from this:

I do CXX=clang++ cmake .. -DCMAKE_CXX_CLANG_TIDY="clang-tidy" (plus a configuration file for clang-tidy)

how or when clang-tidy is actually invoked. Apparently that uses some CMake magic I'm not familiar with. I do remember once seeing such a thing, and there were reasons why I didn't end up using it.

Instead, I use CMAKE_EXPORT_COMPILE_COMMANDS=On (very handy for all tooling based on libclang and then some, like IDEs and LSP servers). Having the compile_commands.json makes it so you can invoke the run-clang-tidy tool with -p pointing to it.

In my experience it does filter system includes as it ought to (and it gives a count of diagnostics suppressed in verbose modes).

sehe
  • 374,641
  • 47
  • 450
  • 633
  • I had a brainwave. In my setup, BOOST_ROOT is **not** under /usr/include. Perhaps that is a factor? – sehe Apr 10 '22 at 23:42
  • i understand. yes, it is cmake magic, it is well documented though. perhaps i am not properly declaring boost (test) as a proper system library. i don’t see “isystem” reflected in the command line when i do `make VERBOSE=1`. – alfC Apr 10 '22 at 23:47
  • I always inspect using `compile_commands` (or `make -Bsn`). It *does* reflect `-isystem` for me, but then again, it might be a "tie" because `/usr/include` for Boost_INCLUDE_DIRECTORIES may alias with the actual system includes (which may or may not be correctly marked as SYSTEM?) – sehe Apr 11 '22 at 00:00
  • Anyways, using this docker file I cannot see a wayward diagnostic even in a default Ubuntu 21.10 box: http://stackoverflow-sehe.s3.amazonaws.com/f2fd061a-50b3-425e-a969-fd5664c8043d/Dockerfile (with [my patch](http://stackoverflow-sehe.s3.amazonaws.com/4f985326-9f74-4295-bc9f-646bb2ada5bd/0001-make-Boost-includes-SYSTEM.patch) applied - haven't checked without) – sehe Apr 11 '22 at 00:01
  • Oh I forget to mention I run that with a bind-mount from the `multi` checkout worktree: `docker run --rm -v "$PWD:/root" -it multi-sehe:latest` – sehe Apr 11 '22 at 00:05
  • ahrr, boost and Cmake, always a pain. – alfC Apr 11 '22 at 00:15
  • TBH I don't see how boost makes it worse than just CMake. It's pretty much literally just the same as any other CMake stuff? The thing I feel the most dubious about here is putting CMake in charge of the clang-tidy calls. I suppose it should amount to the same, but still I'm not convinced I'd even want the tidy to always go off (build times). It might grow on me though :) – sehe Apr 11 '22 at 00:21
  • Well. Boost seems to be special somehow to Cmake in my experience. For example you have to tell it *not* to use the "system" boost sometimes. I use the `-DCMAKE_CXX_CLANG_TIDY="clang-tidy"` option only for the CI, and before pushing to the repository. – alfC Apr 11 '22 at 01:28
  • That's exactly the same as with any other library (e.g. if you used your own libfftw version it would be *exactly* the same). Like I said, I might give it try using the CMAKE integration one day. Perhaps it will grow on me (for now I'm content with `run_clang_tidy` in the CI scripts) – sehe Apr 11 '22 at 01:38
  • 1
    ok, I have to learn more Cmake. I will investigate using the `export commands` feature and also investigate why the "isystem" doesn't appear in the first place which is probably the ultimate cause. The `-DCMAKE_CXX_CLANG_TIDY="clang-tidy"` feature is very handy because it exploits the fact that `clang-tidy` understands the `clang` compile options . There an equivalent one for `cppcheck` (check my CI). Feel free to use or ask about my library if you find it useful. – alfC Apr 11 '22 at 01:44
  • One question: did I run into it before on the Boost mailing list? Maybe it was proposed for adoption by Boost, or did I see a standards proposal that (very similar to) this library? – sehe Apr 11 '22 at 01:57
  • 1
    Yes, I think I send and email to the Boost List and also to the Slack group asking for interest but nobody responded. Maybe I will try again, or simply remove the whishful "boost-". The standard proposal you are talking about is called `mdspan` which is a much limited subset of this (to begin it doesn't provide owning containers). – alfC Apr 11 '22 at 02:05
  • Boost isn't marked as system because you have to make it so explicitly in your CMake file (as a library author) and Boost authors don't do it for some reason or, maybe, they simply don't know they should. @alfC – ixSci Apr 11 '22 at 07:25
  • Whut? If that's how it works, that's definitely a CMake mess-up, not a Boost, since there is no diagnostic or even documentation warning of this when you do `include_directories(SYSTEM ${Boost_INCLUDE_DIRS);`. Also, I see zero reason not to honour this directive anyways (although there could be some merit to suggesting/defaulting SYSTEM from inside a find-package thing). To be honest, I'm skeptical that this can be how it works, since findboost is documented to result in these variables only. They logically cannot contain a "system" indicator? – sehe Apr 11 '22 at 10:26
  • 1
    It's is a boost's job to do it properly, CMake provides means, boost should use them. I created an [issue](https://github.com/boostorg/boost/issues/638). Also using variables from the find modules/config (like `Boost_INCLUDE_DIRS`) is an old way and discouraged. One should use targets instead. – ixSci Apr 11 '22 at 17:33
  • I changed all Boost includes to `SYSTEM` as sehe suggested and I get this typical command: `/usr/bin/cmake -E __run_co_compile --tidy="clang-tidy;--extra-arg-before=--driver-mode=g++" --source=/home/correaa/prj/alf/boost/multi/test/zero_dimensionality.cpp -- /usr/bin/clang++ -DBOOST_ALL_NO_LIB -DBOOST_SERIALIZATION_DYN_LINK -DBOOST_UNIT_TEST_FRAMEWORK_DYN_LINK -I/home/correaa/prj/alf/boost/multi/include -g -std=c++17 -o CMakeFiles/zero_dimensionality.cpp.x.dir/zero_dimensionality.cpp.o -c /home/correaa/prj/alf/boost/multi/test/zero_dimensionality.cpp`, so, no `-isystem` in the command. – alfC Apr 12 '22 at 03:32
  • This is reflected in my repo now. I am quite lost, I don't even know who to blame, Cmake, Boost or my own basic incompetence with Cmake. – alfC Apr 12 '22 at 03:34
  • It seems I was doing nothing wrong. The problem solved by itself in clang-tidy 14. – alfC Nov 09 '22 at 21:42
1

If you use macros from a library which incur clang-tidy warnings, I don't think there's a good way to avoid it today. Even if you include the library as a system library (e.g., via -isystem) my understanding is the warnings will still trigger because the macro is used in your code, even if it is declared in the library.

Here's a proposal to fix it, unfortunately it seems stalled since 2021.

BeeOnRope
  • 60,350
  • 16
  • 207
  • 386
  • 1
    clang-tidy already seems to propagate the warning suppressions from the point of the macro definition, which is great. However this assumes that you have unrestricted access to the definition of the macro. This would be a nice step further. – alfC Nov 02 '22 at 20:33
  • Right, but this doesn't contradict my answer, right? (Not saying that you are saying that, just clarifying). I.e., for library code that you don't control, it seems you can't use the configuration tools clang-tidy gives you to do a targeted disabling of some warnings coming from a macro in that library that you use in your code, but that proposal would fix that. @alfC – BeeOnRope Nov 02 '22 at 21:53
  • No contradiction hetr, just wanted to point out that things moved in the right direction for macros. – alfC Nov 02 '22 at 22:55
  • I wonder if one can exploit the featured I mentioned to redefine the macro to carry the suppression. I am not an expert on macros but it seems risky. As if this would work `#define A_BOOST_TEST_MACRO(VARGS...) A_BOOST_TEST_MACRO(ARGS...) // NOLINT(...)` – alfC Nov 02 '22 at 22:55
  • @alfC - no, that would be an infinitely recursive macro, I think. You could redefine it under a different name though. Or else you could redefine it in terms of the "next level" macro since something like `BOOST_FOO` is often a very short definition which immediately shells out to `BOOST_FOO_IMPL` or whatever: you could copy/paste that and add the NOLINT, though your depending on internals which may change in another version then. – BeeOnRope Nov 03 '22 at 00:31
  • It seems I was doing nothing wrong. The problem solved by itself in clang-tidy 14. Not sure what particular change in 14 solved it. – alfC Nov 09 '22 at 21:42
0

Solution: use clang-tidy 14 or newer.

alfC
  • 14,261
  • 4
  • 67
  • 118