[libcxx-commits] [PATCH] D131963: [libc++] Add custom clang-tidy checks

[Louis Dionne via Phabricator via libcxx-commits]

ldionne added inline comments.


================
Comment at: libcxx/test/tools/clang_tidy_checks/CMakeLists.txt:13-15
+  set(CMAKE_CXX_STANDARD 20)
+
+  project(cxx-tidy)
----------------
philnik wrote:
> ldionne wrote:
> > I'm not sure why we'd need those. We already set that at the top-level.
> We use
> ```
> set_target_properties(${target} PROPERTIES
>     CXX_STANDARD 20
>     CXX_STANDARD_REQUIRED YES
>     CXX_EXTENSIONS NO)
> ```
> at the top level. Should I do the same here?
Yes, that seems like the correct thing to do to me.


================
Comment at: libcxx/test/tools/clang_tidy_checks/CMakeLists.txt:2
+
+find_package(Clang)
+
----------------
You should be able to skip the version check below that way.

We'll also have to fix the ClangConfig.cmake file, I don't see a good way of moving forward without that.


================
Comment at: libcxx/test/tools/clang_tidy_checks/CMakeLists.txt:32
+
+project(cxx-tidy)
+add_library(cxx-tidy MODULE ${SOURCES})
----------------



================
Comment at: libcxx/utils/ci/run-buildbot:507
+          -DLLVM_ENABLE_ASSERTIONS=ON \
+          -DLIBCXX_ENABLE_CUSTOM_CLANG_TIDY_CHECKS=ON
 
----------------
This looks like a leftover.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131963