[libc-commits] [PATCH] D116126: [libc] add test formatting rules

Wed Dec 22 10:41:18 PST 2021

michaelrj added inline comments.

================
Comment at: libc/test/src/string/memory_utils/.clang-tidy:1
+# temporary, this folder was crashing clang-tidy. I'm working on reporting the bug.
+InheritParentConfig: false
----------------
sivachandra wrote:
> michaelrj wrote:
> > sivachandra wrote:
> > > I would actually vote for not adding any of these `.clang-tidy` files in this round. Just the CMake change to run clang-tidy for the tests should be sufficient as it will then use the main `.clang-tidy` file which is good enough to start with. WDYT?
> > The problem is that each of these `.clang-tidy` files have reasons behind them. The other two are there because those files are matching external interfaces that we can't change, and so reformatting them is unhelpful. This file is because clang-tidy will fully crash if it tries to format `utils_test.cpp`, for which I filed a bug here: https://github.com/llvm/llvm-project/issues/52825. I can remove the other two files, although it will cause a lot of log spam from the formatter being confused about functions with names that don't match our formatting, but are defined by gtest or MPFR (e.g. `addTest`). On the other hand, there's already a lot of log spam from the files that also implement the gtest interface, but are not in that folder (e.g. TmMatcher).
> Lets do it this way then. Can you land it https://reviews.llvm.org/D116127 as a separate cleanup patch? We will play with this change after that to better understand how to proceed.
SGTM

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116126