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

Clang Frontend clang-tidy
1

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:)

7 Likes
2

Sounds good to me.

3

Iā€™m very much in favor of this

4

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.

5

I assume that with ā€˜emptyā€™, you actually where thinking of ā€œInheritParentConfig: trueā€, right?

1 Like
6

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.

7

Yes, of course. Updated the description.

8

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.

9

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.

10

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.

11

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

12

To keep in sync, the following PR was merged with new .clang-tidy config:

Thanks everyone for participating!

1 Like
13

Happy to announce that we now do have experimental clang-tidy enabled in CI with recent merge of

All freshly rebase PRs should go through clang-tidy CI workflow.

2 Likes
14

Great work!