Libc++ uses two clang tools in its pre-commit CI, clang-format and clang-tidy.
We’ve been looking at which version of these tools we want contributors to use.
Initially we settled on the latest stable LLVM release for both tools. However
recently we ran into some issues with the tools.
The libc++ CI uses a Docker image that is updated every few weeks. This is a
manual process and there is no fixed schedule. This image uses Ubuntu as its
base image. It uses the nightly builds of apt.llvm.org for its LLVM based
dependencies.
Clang-format is used to validate whether a subset of the libc++ files are
formatted. By default files need to be formatted. Files that are not fully
formatted are listed in /data/src/llvm/libcxx/utils/data/ignore_format.txt.
This avoids regressions in the formatted state. Recently we ran into some
issues where a newer clang-format minor release caused some minimal formatting
changes. However in my experience clang-format seems to strive to keep its
output stable. Updating to the latest development version will probably be more
efforts for some (new) contributors. So I think we should keep this to the
latest released stable version.
Clang-tidy this is an optional dependency in CMake. Libc++ uses both the
default clang-tidy tests and a plugin with libc++ specific tests. Here
too we opted to use the latest stable version However I ran into some issues
with that.
The libc++ CI assumes the compiler flags supported by the compiler are also
supported by clang-tidy. Since GCC supports different flags than clang(-tidy)
these clang-tidy tests are disabled when using GCC as a compiler.
Recently Clang-17 stated to support -std=c++23 and -std=c++26, both are
not supported by clang-tidy 16. The same holds true for the compiler flag
-Wno-reserved-module-identifier. (This flag is needed to build the C++23
std module in libc++, which I intend to land in the near future.)
Due to these issues I would like let libc++ depends on the latest development
version of clang-tidy. This makes it easier to:
- Always use the same compiler flags as Clang in clang-tidy.
- Be able to use the latest Clang language features in libc++. This is useful
when the C++ language adds new keywords or new ways to use existing
keywords. For example to support coroutines or modules.
The disadvantage is that updating clang-tidy may be harder for some
contributors. Still they can opt-out of checking clang-tidy locally and rely
on the CI. Our pre-commit CI runs about 60 jobs using 5 different operating
systems. So using the output of the CI to fix issues is not uncommon in libc++.
Summary, I propose to:
- keep using the latest stable release of clang-format
- change the requirement of clang-tidy to use the latest development version
Note these changes do not affect developers using the installed libc++ on their
system.
1 Like
I don’t really see why we would want to use the latest dev version. The only case that really matters AFAICT is the change to -std=c++23 and the addition of -std=c++26. For any warning flags, we can add -Wno-unknown-warning-option if required. We can’t use the latest language features anyways, because we support the last two Clang versions. If we still want to use some new language feature, we have to guard it behind an #if _LIBCPP_CLANG_VERSION >= whatever anyways, which also disables this portion of the code for clang-tidy (although I can’t recall ever doing this). Also note that another (IMO major) disadvantage is that we rely on compiler internals for the clang-tidy checks. These internals may change at any point, resulting in potentially quite a bit of work that not everybody is willing to do, especially when updating the image. This isn’t a huge problem if it only happens for major updates, especially since some people (at least I) run on the latest clang-tidy and see source breaks before the CI does, allowing me (or others) to fix it before it becomes a problem. This isn’t the case for the latest nightly build, since the code might be so fresh that nobody tried to build against it yet.
I am mostly neutral on the clang-tidy aspect, however I would like to be cautious about introducing anything that can suddenly start failing without us doing any deliberate action to update things. That’s the case for using a tip-of-trunk clang-tidy (or any other tip-of-trunk tool). The problem this creates is that the CI can suddenly start failing, which becomes a high priority issue for us to fix to unblock everyone’s work. However, this might not happen at a time when it is convenient for us to fix it (for example maybe there’s some other urgent problem being fixed at the same time).
So my slight preference would be to use the latest stable version as well, even though it does mean that we can’t use the latest and greatest features in some cases. Whatever we end up doing, I think we should document which clang-format version is currently supported so that contributors can match that version locally to format their code. This has been a recurring complaint by contributors that they can’t reliably format their code locally and then it fails in the CI, which is kind of counterproductive for something as basic as code formatting.
I’m quite sure we had features in the past that can only be used with a specific Clang version. It is indeed rare that we need this, most C++ features are language or library only. For example coroutines added both new keywords and library support, the same with modules. This is against our general policy that we want to support the latest two Clang versions. But when the compiler marks code as ill-formed it’s not easy to add a library only work-around.
We have used _LIBCPP_CLANG_VER in the past, but it’s quite rare. So I’m not objecting against using that way, we already need to do that anyway.
I’m not too worried about CI breakage since the number of people that uploads a new Docker image is quite small. I tend to test several configurations before uploading a new image and on rare occasions this finds new issues.
Mainly curious, have you ever seen breaking changes to clang-tidy internals?
I feel there is a big difference between clang-format which is mandatory and clang-tidy which is optional for contributors. Hence my proposal to only update the optional dependency.