[libc-commits] [PATCH] D119180: [libc] Add LLVM_LIBC_CLANG_TIDY option and allow LLVM_LIBC_ENABLE_LINTING without full build.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/CMakeLists.txt:56
+if(LLVM_LIBC_CLANG_TIDY)
+  set(LLVM_LIBC_ENABLE_LINTING ON)
 endif()
----------------
Check whether that clang-tidy has the checks LLVM libc needs. This could be as simple as checking its version that it is good enough, and then matching it with the version of clang.


================
Comment at: libc/CMakeLists.txt:61
+  if (NOT LLVM_LIBC_CLANG_TIDY)
+    find_program(LLVM_LIBC_CLANG_TIDY NAMES clang-tidy)
+  endif()
----------------
Same thing here?


================
Comment at: libc/CMakeLists.txt:64
+
+  if (NOT LLVM_LIBC_CLANG_TIDY)
+    if (LLVM_LIBC_FULL_BUILD
----------------
We should probably not have this fallback at all? For, there is no guarantee that the compilers are in sync with the HEAD clang-tidy?


================
Comment at: libc/cmake/modules/LLVMLibCObjectRules.cmake:274
 
-    add_custom_target(${fq_target_name}.__lint__
-      DEPENDS ${lint_timestamp})
-    add_dependencies(lint-libc ${fq_target_name}.__lint__)
-    add_dependencies(${fq_target_name} ${fq_target_name}.__lint__)
+    if(NOT LLVM_LIBC_CLANG_TIDY)
+      add_custom_target(${fq_target_name}.__lint__
----------------
Why is this required?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119180