Meson: Support for clang-tidy (or other linters)

[jhasse]

CMake has support for additional linting by clang-tidy: http://www.mariobadr.com/using-clang-tidy-with-cmake-36.html

I was wondering if something like this could be done for Meson, so that header dependencies etc. still work.

[FSMaxB]

Meson already produces the compile_commands.json that is required for clang-tidy but sadly there are too many command line options in there.

I don't know why exactly, but clang-tidy chokes on -pipe, various -W diagnostic flags and more.

[FSMaxB]

Actually if you remove all -pipe from the compile_commands.json then you can run clang-tidy as long as you pass -clang-diagnostic-unused-command-line-argument to the list of checks.

[agauniyal]

@FSMaxB how did you make it work? Something like -

run_target('clang-tidy', command : ['clang-tidy', '-checks="*" -p=' +
            join_paths(meson.build_root())])

doesn't works for me 😕

[FSMaxB]

@agauniyal https://github.com/1984not-GmbH/molch/blob/f06980fa7e65bcc6ac06b72caec39e8771ed2ffc/ci/clang-tidy.sh I'm not proud of it, but it works for me.

[ghost]

I‘d second that request and add iwyu as a request.

It would be really good to have easy support for automatic fixes being applied as well. Setting up the python script is a real pain in my experience.

🙏🏼 thanks

[NickeZ]

Using clang-tidy-6.0 the following works for me:

src_files = files('asourcefile.cpp')
clangtidy = find_program('clang-tidy', required: false)
if clangtidy.found()
    run_target(
        'tidy',
        command: [
            clangtidy,
            '-checks=*',
            '-p', meson.build_root()
        ] + src_files)
endif

[kaimast]

@NickeZ thanks! This kinda works for me too.

I'm just curious if clang throws these errors for you too; error: unknown argument: '-pipe' [clang-diagnostic-error]

I don't think I specify this option anywhere, but maybe meson does? This seems to still have the original problem from above.

[NickeZ]

@kaimast Actually I don't get that issue. I'm using llvm 6.0, which version are you using? (But even with 3.9 it works for me)

[ghost]

If you have to manually provide the src_files this somewhat defeats the benefit of a build system.

[kaimast]

It's kind of necessary though. For example, I use google test and the macros generate code that is not very modern, so I need to ignore those tests when linting.

Usually you already have a variable that holds all your sourcefiles anyways, so supporting this in meson is still pretty useful.

[kaimast]

@NickeZ strange. I'm using 6.0 too. Are you using "ninja tidy" or some other command to run the linter?

[ghost]

So your Tests supposedly are in a dedicated folder or follow a certain naming based on which they should be excluded/ or the other way around where source directories would be included. (But not on a file by file basis otherwise it’s pretty much just a bash script)

[kaimast]

Any progress on this? Or any way I could help? I still have the same problem on 0.48.2

[jpakkane]

The problem with this is not the actual target but the fact that you almost always want to specify a specific command line argument (specifically what tests you want to run and which header directories to use). This is in contrast to scan-build, which consists of only one and the same command that you always run. Enabling all checks just drowns you in warnings on almost all code bases. Doing this right would take possibly two new options just for this. Unfortunately we already have too many options and cramming even more in makes the whole thing unwieldy.

If you just want to replicate what CMake (as linked above) does, then NickeZ's snippet is pretty much the same thing.

[jhasse]

clang-tidy reads the file .clang-tidy for options, so there doesn't have to be a way to pass them.

[jpakkane]

You always need to pass the files you want to process, though. Sadly clang-tidy does not seem to have a flag for "process all files in compile_commands.json". Even if it did, passing all files becomes very slow even with medium sized projects. Then you'd want to use run-clang-tidy instead to get the parallelism.

[jhasse]

Well, for a Meson project I would expect that all C/C++ files are passed that are also being compiled.

[FSMaxB]

@jhasse: All C/C++-Files that are not part of a subproject more likely.

[phillipjohnston]

@NickeZ's solution works well, except for the fact that meson is inserting the -pipe flag, which does in fact prevent an error. Is there a way to disable that flag?

error: unknown argument: '-pipe' [clang-diagnostic-error]

[kaimast]

Yeah the -pipe is indeed the major issue for me too.

I think we need a proper Meson module that invokes the linter and cleans up the compile commands beforehand.
That is essentially what I am doing now except I use a bash script. See below what I do

Meson part

clangtidy = find_program('clang-tidy', required: false)

if clangtidy.found()
    run_target(
        'tidy',
        command: [
            'scripts/clang-tidy.sh',
            clangtidy.path(),
            meson.source_root(),
            meson.build_root()
        ] + <CPP FILES>,
    depends: <PROJECT NAME>)
endif

Bash script

#! /bin/bash

# Pick any flags you like here
CHECKS='-hicpp-*,-readability-implicit-bool-conversion,-cppcoreguidelines-*,-clang-diagnostic*,-llvm-*,-bugprone-*,-modernize-*,-misc-*'

BIN=$1 && shift
PROJECT_ROOT=$1 && shift
MESON_ROOT=$1 && shift

# Execute in a different directory to ensure we don't mess with the meson config
TIDY_DIR=${PROJECT_ROOT}/build-tidy

mkdir -p ${TIDY_DIR}
cp  ${MESON_ROOT}/compile_commands.json ${TIDY_DIR}

# Replace meson commands clang does not understand
sed -i 's/-pipe//g' ${TIDY_DIR}/compile_commands.json

echo "Running clang checks: ${CHECKS}"
$BIN -checks=${CHECKS} -warnings-as-errors=* -p ${TIDY_DIR} $@

Ideally the meson code should look something like this imho

clang_tidy = import('clang_tidy')

if clang_tidy.found():
       clang_tidy.run_on(<PROJECT NAME>)
endif

[toanju]

Adding some observations. https://bugs.llvm.org/show_bug.cgi?id=37315 seems to be related to the -pipe issues since actually removing -Xclang works as well. Furthermore setting the option b_colorout=never which still sets -Xclang works as well. Is -Xclang really necessary here?

[mhhollomon]

@kaimast - thank you for the inspiration. I wound up doing things a bit differently. I wanted everything in the build directory, I needed to be able to pass other flags to clang-tidy, and I wanted something fairly generic. So, the bash script I came up with is:

#!/usr/bin/bash

BIN=$1 && shift
BUILD_ROOT=$1 && shift

# Execute in a different directory to ensure we don't mess with the meson config
TIDY_DIR="${BUILD_ROOT}/:clang-tidy"

mkdir -p ${TIDY_DIR}
cp  ${BUILD_ROOT}/compile_commands.json ${TIDY_DIR}

# Replace meson commands clang does not understand
sed -i 's/-pipe//g' ${TIDY_DIR}/compile_commands.json

$BIN -p ${TIDY_DIR} $*

[martin-ejdestig]

There is also https://bugs.llvm.org/show_bug.cgi?id=37281 to consider.

Also, none of the run_target() examples above parallelizes. Would be nice if Meson took care of that.

[martin-ejdestig]

FYI... forgot that I filed a clang-tidy bug for -the pipe and clang-diagnostic error but found it now again when searching for the -pipe problem in bugzilla. Can be found here:

https://bugs.llvm.org/show_bug.cgi?id=37315

[XVilka]

Seems there is exactly zero reaction from Clang developers on this bug. Maybe it makes sense to write a message to their mailing list instead? I think they ignore most of the bugzilla bugs (judging from my own experience).

[martin-ejdestig]

The problem with this is not the actual target but the fact that you almost always want to specify a specific command line argument (specifically what tests you want to run and which header directories to use). This is in contrast to scan-build, which consists of only one and the same command that you always run. Enabling all checks just drowns you in warnings on almost all code bases. Doing this right would take possibly two new options just for this. Unfortunately we already have too many options and cramming even more in makes the whole thing unwieldy

@jpakkane What two options are you thinking about?

I think everything should be possible to configure in .clang-tidy. What checks to enable definitely is. The header filter is a bit problematic due to https://bugs.llvm.org/show_bug.cgi?id=37281, but could say "set header filter in .clang-tidy like this for it to work with Meson" and/or extract and modify it if it looks like it has paths relative to the source root (e.g. ^src/ -> ^../src/).

If a clang-tidy target should be available or not could be handled just like you do in 1fca654055d3502d2db9c5aad66a522beaa1df19 . Only add it if there is a .clang-tidy in the root.

The only thing I can come to think of that I would want configurable in Meson itself is some way to say "exclude these sources". Not running clang-tidy on source in the build directory and subprojects by default would make it so that is not even needed in most cases. A regex option that defaults to match source in the subprojects and build folder? (In CMake they work around this by placing a mostly empty dummy .clang-tidy in folder of source that should not be checked. See e.g. https://gitlab.kitware.com/cmake/cmake/commit/b13bc8659f87567b1b091806d42f5023b2a6b48b . Ugh!)

[jpakkane]

If a clang-tidy target should be available or not could be handled just like you do in 1fca654 . Only add it if there is a .clang-tidy in the root.

I did not know clang-tidy had this functionality (it's not particularly prominent in the docs). Adding a clang-tidy target that behaves just like the clang-format target (i.e. run it on all sources using the given .clang-tidy file) is something we can do fairly easily.

[stderr-enst]

Just a +1 on having a clang-tidy target as well - would be really neat if it worked like clang-format and you can just throw it at your whole project.
With the script/run_target approach above you kind of want a global source-file list of your project, which would be against meson principles I guess. :)

[nhooyr]

Can confirm I'm still getting the -pipe argument undefined error.

Is it possible we could just remove -Xclang from the default args like @toanju mentioned?

[jhasse]

Shouldn't the new clang-tidy target only include source files I use in my build? This way it checks way too much stuff. It even ignores .gitignore and therefore checks cache files from my language server.

It also doesn't use Ninja jobs but implements its own parallelism.

Ctrl+C doesn't stop the job from running.

This is not what I had in mind and doesn't even match CMake's support. Please re-open.

[jpakkane]

Does #6083 fix things for you?

[jhasse]

Not really, as I could have ran run-clang-tidy.py myself before (also it isn't in the PATH on most distros, if I'm not mistaken, so there's still some tinkering needed). It also uses its own parallelism and doesn't track header-dependencies.

[mattyclarkson]

By default it shouldn't run on subprojects. I get huge amounts of false positives. As @jhasse said, respecting .gitignore would be sensible.

[severen]

I can confirm that on Arch run-clang-tidy.py is not in $PATH (instead it is in /usr/share/clang). I agree with @jhasse that this issue should be reopened. I don't think the current support for clang-tidy is sufficient or what the average person would expect.

[XVilka]

Ignoring some files like for clang-format target would be awesome too (apart from .gitignore), e.g. for 3rd-party submodules, copied code, etc. See also https://github.com/mesonbuild/meson/issues/7271