[libcxx-commits] [PATCH] D143333: [libc++] Run clang-tidy in all configurations that are run in the Docker container

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/utils/ci/buildkite-pipeline.yml:318
           CXX: "clang++-${LLVM_HEAD_VERSION}"
+          ENABLE_CLANG_TIDY: "On"
       agents:
----------------
philnik wrote:
> Mordante wrote:
> > The clang-tidy tests don't seem to be very cheap. I expect them to become more expensive in the future, since we probably want to add more tests. (I have some ideas on my own todo list.) So I wonder, does it have real benefit to enable them in all configurations? Especially this one. If it does then I think it would be good write the rationale in the commit message.
> Most configurations have some code that's specific to it.
> 
> For me running clang-tidy takes ~40s. While that's not quick, it's also not exactly unbearable. We have other tests that take a lot longer. By far the slowest check is `readability-identifier-naming`, removing it brings the time down to ~8s. So I don't expect running clang-tidy to take that much longer in the future, since `nasty_macros.compile.pass.cpp` takes ~2s which is effectively just running the front-end. Since we run clang-tidy two times, all the checks except `readability-identifier-naming` take ~4s.
At the risk of asking a dumb question, what's the benefit of running the checks in all configurations?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143333/new/

https://reviews.llvm.org/D143333