-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Support for clang-tidy (or other linters) #2383
Comments
Meson already produces the I don't know why exactly, but |
Actually if you remove all |
@FSMaxB how did you make it work? Something like -
doesn't works for me 😕 |
@agauniyal https://github.com/1984not-GmbH/molch/blob/f06980fa7e65bcc6ac06b72caec39e8771ed2ffc/ci/clang-tidy.sh I'm not proud of it, but it works for me. |
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 |
Using 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 |
@NickeZ thanks! This kinda works for me too. I'm just curious if clang throws these errors for you too;
|
@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) |
If you have to manually provide the src_files this somewhat defeats the benefit of a build system. |
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. |
@NickeZ strange. I'm using 6.0 too. Are you using "ninja tidy" or some other command to run the linter? |
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) |
Any progress on this? Or any way I could help? I still have the same problem on 0.48.2 |
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. |
clang-tidy reads the file .clang-tidy for options, so there doesn't have to be a way to pass them. |
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 |
Well, for a Meson project I would expect that all C/C++ files are passed that are also being compiled. |
@jhasse: All C/C++-Files that are not part of a subproject more likely. |
@NickeZ's solution works well, except for the fact that meson is inserting the
|
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. 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 |
Adding some observations. https://bugs.llvm.org/show_bug.cgi?id=37315 seems to be related to the |
@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} $* |
There is also https://bugs.llvm.org/show_bug.cgi?id=37281 to consider. Also, none of the |
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: |
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). |
@jpakkane What two options are you thinking about? I think everything should be possible to configure in If a 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!) |
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 |
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. |
Can confirm I'm still getting the Is it possible we could just remove |
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 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. |
Does #6083 fix things for you? |
Not really, as I could have ran |
By default it shouldn't run on |
I can confirm that on Arch |
Ignoring some files like for |
Does anyone have a workaround for the way meson has chosen to run clang-tidy on every source file individually (including headers!) ? |
How can I now run clang-tidy with meson? Is there a documentation about it? |
meson setup builddir
ninja -C builddir clang-tidy You can also use |
Thank you. To include clang-tidy in meson.build: What is the current syntax? Still the following as in some comments? 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 |
Meson will automatically generate a clang-tidy target for you if you have a .clang-tidy configuration at the root of your project and a you don't define your own target |
Thanks for the answer! Now it works. Edit: Ninja seems to execute |
The It exists to abstract away the differences between different backends, i.e. you can use -- and document in README.md -- the same command to build with ninja, or to build with Visual Studio or XCode (assuming you passed the
It is possible that meson will change the command name, or argument passing conventions. An We cannot make any promises regarding whether or not we will, sometime in the future, decide on a need to refactor the internals. |
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.
The text was updated successfully, but these errors were encountered: