Clangd started showing clang-tidy warnings recently and I’ve noticed there is too much of by clang-tidy inside the clang codebase, and most of it is coming from ‘readability-identifier-naming’ check (at least in the Sema and Parser code).
produces produces 52 warnings, 51 of which are naming violations from ‘readability-identifier-naming’. ‘Sema.h’ has 1830 clang-tidy warnings with ‘readability-identifier-naming’ and 228 without it.
IIUC, the consensus is that renaming everything to align with the style guide is just not worth it (would introduce merge conflicts, mess up the history, etc). Does this render the naming check non-useful for the ‘clang/’ project? Should we remove it from ‘clang/.clang-tidy’?
Are there other alternatives that could bring down the noise in clang-tidy output and actually make it useful (e.g. we could put a file-wide NOLINT comments into those files)?
We disabled the two most noisy checks in r352862 and clang-tidy now produces only 3 warnings on Sema.h.
Let us know if you have concerns and feel we should revert this.
I think there could be two modes in which clang-tidy is being run.
For new code seeing these violations would be great, but they could be run as linter in
arc, for existing code this is of course
In my opinion we should lint new code better and have clang-tidy run there at least with full configuration enabled.
Totally agree, if we had a nice UI (in Phabricator?) or a script to show only clang-tidy warnings touching the change diffs, disabling these checks would have been a much harder sell.
Even in that case, though, we’d probably want to have different configs for clang-tidy-over-diffs and clang-tidy to avoid cluttering the output when using clangd or standalone clang-tidy.
Yes, allowing different configuration files in clang-tidy would be an easy step forward (clang-tidy requires a ‘.clang-tidy’ file and provides no option to specify something else).
See this revision for some thoughts we have in the clang-tidy space on how to proceed and integrate with phabricator.