With new contributors coming to clang-tidy
project, It is getting harder to review many patches that usually have the same small flaws.
I propose creating a clang-tidy
config in /clang-tidy
directory that will have more checks enabled than LLVM root config. This could encourage developers to fix warning beforehand, and in the future we could have a clang-tidy CI job that would check code automatically.
For implementation of this RFC, a new empty .clang-tidy
config will be created with InheritParentConfig: true
. Later, create PRs with new check/checks enabled one by one and also include all fixes that the check needs to have. If the fixes are too big in one go, consider creating multiple PRās.
Hopefully, we could enable misc-const-correctness
some time in the future and make life of Eugene easier:)
4 Likes
Iām very much in favor of this
LLVM globally could enable more checks IMO, but thatās a different ask. Highly in favor of Clang-Tidy dogfooding with more checks enabled.
Later, create PRs with new check/checks enabled one by one and also include all fixes that the check needs to have.
I wonder if thereās a subset of checks that have no or very few errors that we could just enable in one go.
I assume that with āemptyā, you actually where thinking of āInheritParentConfig: trueā, right?
1 Like
I bet there is, another question I had in mind: should we even enable them to āwasteā cpu every CI run in the future if they have practically no benefit. We probably should, but not which are slow.
Yes, of course. Updated the description.
I bet there is, another question I had in mind: should we even enable them to āwasteā cpu every CI run in the future if they have practically no benefit.
Another incentive to optimize Clang-Tidy / slower checks
. More seriously, clangd
keeps a file with āfastā checks, maybe to cross-reference for this. The CI here is also additional tests/checks for the tool itself, so there is direct benefit. IMO the bar is high where costs outweigh benefits.
EDIT: Adding that the number/frequency of Clang-Tidy PRs is low enough that individual CI runs would need to be very long for this to be a significant cost.
Actually, Iāve looked at it today and wondered how accurate it is. Running various checks over /clang-tidy
code may also help update its index.
Totally in favor! But it should be checked in CI instead of trying on manual checks. As someone mentioned thereās not many patches to clang-tidy so itās probably not too heavy anyway.
I agree, but creating CI workflow is a broader problem that could be addressed separately. There was discussion on discord about updating current run-scripts, thus I postponed RFC for clang-tidy
in CI.
cc @TomJ