[RFC] Create hardened .clang-tidy config for /clang-tidy directory

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

Sounds good to me.

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 :wink:. 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