[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 accepted this revision.
sivachandra added inline comments.
This revision is now accepted and ready to land.


================
Comment at: libc/CMakeLists.txt:75
+             "${CMAKE_CXX_COMPILER_VERSION}")
+      if(CLANG_TIDY_VERSION LESS CLANG_MAJOR_VERSION)
+        set(LLVM_LIBC_ENABLE_LINTING OFF)
----------------
Should this be `EQUAL` instead of `LESS`? For, we want clang-tidy version to match the clang-version to avoid header-mismatches.


================
Comment at: libc/CMakeLists.txt:82-84
+          To set 'clang-tidy' target manually, set it to
+          LLVM_LIBC_CLANG_TIDY (pass DLLVM_LIBC_CLANG_TIDY=<clang-tidy target>
+          to cmake).")
----------------
```
The path to the clang-tidy binary can be set manually by passing
-DLLVM_LIBC_CLANG_TIDY=<path/to/clang-tidy> to CMake.
```

Similarly below as well.


================
Comment at: libc/CMakeLists.txt:88
+      set(LLVM_LIBC_ENABLE_LINTING OFF)
+      message(FATAL_ERROR "
+        Linting is enabled but 'clang-tidy' is not found!
----------------
Because this is a FATAL_ERROR, do we really have to set LLVM_LIBC_ENABLE_LINTING to OFF?


================
Comment at: libc/cmake/modules/LLVMLibCObjectRules.cmake:230
+
+    set(CLANG_TIDY_COMMAND ${LLVM_LIBC_CLANG_TIDY})
 
----------------
Why is this required?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119180