Setup clang-tidy

assigned to @jeromeh

changed the description

added CI ⚡ Discussion 💬 PriorityMedium labels

.clang-tidy : Use WebKit cpp style, and gogole checks ar ethe only one allowed. See #169 (comment 2833716).

This is nice @jeromeh, however no one here has any idea on how clang-tidy works(except you :') ), I would propose that we do it just like clang-format : integrate clang-tidy to the ci of aidge_core with this configuration so that we see the warnings it produces.

Could you also give a command to run clangd-tidy locally ?

Also make sure to use the latest version of clang-tidy (20 I think as we speak):)

Regarding the clang-tidy-base, I don't think that it is ready to be used like this, it needs more discussion about what should be enabled or not.

Could you also give a command to run clangd-tidy locally ?

clang-tidy file.cpp

Assuming there is a compile_commands.json file available in the top-level directory (and a .clang-tidy file, ofc). (I just tested clang-tidy src/scheduler/Scheduler.cpp and it worked for me)

In general though, the best way to use clang-tidy is to use it with CMake, by setting the option CMAKE_CXX_CLANG_TIDY to clang-tidy. (cf. https://cmake.org/cmake/help/latest/variable/CMAKE_LANG_CLANG_TIDY.html). This results in MUCH longer build times, but there are solutions to this.

changed the description

marked this issue as related to aidge#155

Could you tell me what is the difference between clang-tidy & clangd please ? thanks in advance !

clangd is the language server, it provides auto-completion, go-to definitions, find references and so on through the language server protocol. clang-tidy is a linter. It can be used manually on a file : clang-tidy file.cpp or integrated in the build system, but clangd can be configured to to use clang-tidy. When this is done, it automatically applies the clang-tidy checks specified in the project's .clang-tidy file.

Ok thanks for the precision :)

Do you have any ressource on how to configurate clang-tidy to work with clangd ?

It depends on your IDE, but in general, the option "--clang-tidy" should be given to the clangd command. (and you should but the above clang-tidy file, renamed to .clang-tidy, at the top-level directory of your project).

For instance in neovim :

["clangd"] =  function()
                    local lspconfig = require("lspconfig")
                    lspconfig.clangd.setup {
                        capabilities = capabilities,
                        on_attach = on_attach,
                        cmd = { "clangd", '--background-index', '--clang-tidy' }
                    }
                end,

And in helix : https://github.com/helix-editor/helix/discussions/11495

Very nice link, it also contains a link to all cland tidy config its really impressive how many of them there are

Here is my helix setup for clang-tidy :

[[language]]
name = "cpp"
language-servers =  ["clangd-20"]
auto-format = false
indent = { tab-width = 4, unit = "t" }
formatter = { command = "clang-format-20", args = ["--style=file"] }
comment-token = "//"

[language-server.clangd-20]
command = "clangd-20"
args=["--compile-commands-dir=./build_bundle_cpp", "--clang-tidy"]

We can use these to create the setup page for contributors

mentioned in issue aidge#185

marked this issue as related to aidge#185

Afer a discussion with @pineapple , we concluded to :

Use the WebKit coding style for naming things.
Use Google guidelines for cpp best practices.

This should impact class and private members names a lot.

Nice ! I updated wiki with this choice :)

I find the member prefix m_ quite heavy(I understand the prefix s_ for static tho). Could we simply use _ prefix for instance members ?

It is a very bad idea to prefix variables with _, because you risk colliding with names reserved for compiler and standard library implementation use

The current code already uses m prefix for member variables, using m_ instead is not that big a change and is already part of a tried naming style (WebKit here)

ha yeah I forgot about those... ok then I'm ok with that

mentioned in merge request !220 (merged)

@jeromeh I see you ve given clang-tidy 18 documentation in the issue, I currently use clang-format 20, do you think you could use clang-tidy 20 instead so we use the same version?