Refactoring run-clang-tidy and friends for better CMake integration

Clang Frontend clang-tidy
1

There’s a comment in run-clang-tidy that suggests it should be integrated with clang-tidy-diff. This makes sense, as they appear to share some functionality. I couldn’t find an existing ticket for this refactoring. Before I raise one, I’d like to discuss how run-clang-tidy can better interact with CMake.

In my opinion, the current run-clang-tidy doesn’t play nicely with CMake. When running clang-tidy through CMake, CMake dumps a bunch of header files to CMAKE_<LANG>_CLANG_TIDY_EXPORT_FIXES_DIR. If I then run clang-apply-replacements on this directory, code that is compiled multiple times (e.g. header files) gets fixits applied multiple times. run-clang-tidy deals with this by mashing all the yaml files together into one massive file, but unfortunately run-clang-tidy always tries to run clang-tidy. Therefore, if I want to use CMake to run clang-tidy, there is no built-in way to apply fixes to header files. See the discussion on the CMake discourse forum here.

We could fix this at the CMake end, but then we’d end up with two parallel implementations of the same thing (three, if you count clang-tidy-diff). I think the more elegant way to do it is to split out this functionality from run-clang-tidy.

The two obvious ways to do this seem to be:

  1. Do the de-duplication in clang-apply-replacements itself. To me, this seems like the cleanest and most centralised approach. We could then remove the duplicate code from run-clang-tidy and clang-tidy-diff. Other people are better qualified to comment on whether this should be the default functionality, or whether we should have some -deduplicate flag,
  2. Do the de-duplication in python. We could re-factor run-clang-tidy and clang-tidy-diff into some general-purpose clang-tidy wrangler which would have an “apply-fixits” mode which does de-duplication. This seems less elegant to me because we end up with two different LLVM-sanctioned ways to apply fixits. However, it would be closer to the current approach.

Either way, wrapping this up for CMake then becomes trivial, and both projects end up pulling in the same direction.

What do you think?

1 Like