[libc] Add LLVM_LIBC_CLANG_TIDY option and allow LLVM_LIBC_ENABLE_LINTING without full build.

[lntue]

Summary

Add LLVM_LIBC_CLANG_TIDY option and allow LLVM_LIBC_ENABLE_LINTING without full build.

[sivachandra]

libc/CMakeLists.txt

56

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.

65

Same thing here?

68

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?

libc/cmake/modules/LLVMLibCObjectRules.cmake

280–281

Why is this required?

[lntue]

Add checks to make sure that clang-tidy is the same or newer than clang++ major version.

libc/CMakeLists.txt

68

I was trying to preserve the current behavior of using the HEAD clang-tidy when the new flag is not set, so that it won't break all current bots that do not have clang-tidy installed after submission.

libc/cmake/modules/LLVMLibCObjectRules.cmake

280–281

These dependencies are only needed for current behavior when clang-tidy is built in full build mode. If clang-tidy command is provided, we don't need these dependencies. Moreover, it might fail the build if these targets are not enabled with LLVM_ENABLE_PROJECTS.

[lntue]

Remove legacy behavior with respect to full build.

[sivachandra]

libc/CMakeLists.txt

75

Should this be EQUAL instead of LESS? For, we want clang-tidy version to match the clang-version to avoid header-mismatches.

82–84

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.

88

Because this is a FATAL_ERROR, do we really have to set LLVM_LIBC_ENABLE_LINTING to OFF?

libc/cmake/modules/LLVMLibCObjectRules.cmake

230

Why is this required?

[lntue]

Address comments.

libc/CMakeLists.txt

else()
   set(COMPILER_RESOURCE_DIR OFF)
   message(STATUS "COMPILER_RESOURCE_DIR not set
                   --print-resource-dir not supported by host compiler")
 endif()
 
 option(LLVM_LIBC_FULL_BUILD "Build and test LLVM libc as if it is the full libc" OFF)
 
 option(LLVM_LIBC_ENABLE_LINTING "Enables linting of libc source files" OFF)
-if(LLVM_LIBC_ENABLE_LINTING AND (NOT LLVM_LIBC_FULL_BUILD))
-  message(FATAL_ERROR "Cannot enable linting when full libc build is not enabled.")
+
+if(LLVM_LIBC_CLANG_TIDY)
+  set(LLVM_LIBC_ENABLE_LINTING ON)
 endif()
+
 if(LLVM_LIBC_ENABLE_LINTING)
-  if("clang-tools-extra" IN_LIST LLVM_ENABLE_PROJECTS
-             AND "clang" IN_LIST LLVM_ENABLE_PROJECTS)
-    add_custom_target(lint-libc)
-    file(COPY ${LIBC_SOURCE_DIR}/.clang-tidy DESTINATION ${LIBC_BUILD_DIR})
+  if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
+    set(LLVM_LIBC_ENABLE_LINTING OFF)
+    message(WARNING "C++ compiler is not clang++, linting with be disabled.")
+  else()
+    if (NOT LLVM_LIBC_CLANG_TIDY)
+      find_program(LLVM_LIBC_CLANG_TIDY NAMES clang-tidy)
+    endif()
+
+    if(LLVM_LIBC_CLANG_TIDY)
+      # Check clang-tidy major version.
+      execute_process(COMMAND ${LLVM_LIBC_CLANG_TIDY} "--version"
+                      OUTPUT_VARIABLE CLANG_TIDY_OUTPUT)
+      string(REGEX MATCH "[0-9]+" CLANG_TIDY_VERSION "${CLANG_TIDY_OUTPUT}")
+      string(REGEX MATCH "[0-9]+" CLANG_MAJOR_VERSION
+             "${CMAKE_CXX_COMPILER_VERSION}")
+      if(NOT CLANG_TIDY_VERSION EQUAL CLANG_MAJOR_VERSION)
+        set(LLVM_LIBC_ENABLE_LINTING OFF)
+        message(WARNING "
+          'clang-tidy' (version ${CLANG_TIDY_VERSION}) is not the same as
+          'clang' (version ${CLANG_MAJOR_VERSION}).  Linting will
+          be disabled.
+
+          The path to the clang-tidy binary can be set manually by passing
+          -DLLVM_LIBC_CLANG_TIDY=<path/to/clang-tidy> to CMake.")
+      endif()
     else()
       message(FATAL_ERROR "
-      'clang' and 'clang-tools-extra' are required in LLVM_ENABLE_PROJECTS to
-      lint llvm-libc. The linting step performs important checks to help prevent
-      the introduction of subtle bugs, but it may increase build times.
+        Linting is enabled but 'clang-tidy' is not found!
+
+        The path to the clang-tidy binary can be set manually by passing
+        -DLLVM_LIBC_CLANG_TIDY=<path/to/clang-tidy> to CMake.
 
         To disable linting set LLVM_LIBC_ENABLE_LINTING to OFF
         (pass -DLLVM_LIBC_ENABLE_LINTING=OFF to cmake).")
     endif()
-elseif(LLVM_LIBC_FULL_BUILD)
-  message(WARNING "
-    Linting for libc is currently disabled.
-
-    This is not recommended, to enable set LLVM_LIBC_ENABLE_LINTING to ON
-    (pass -DLLVM_LIBC_ENABLE_LINTING=ON to cmake).")
+  endif()
 endif()
 
 option(LLVM_LIBC_INCLUDE_SCUDO "Include the SCUDO standalone as the allocator for LLVM libc" OFF)
 if(LLVM_LIBC_INCLUDE_SCUDO)
   if (NOT "compiler-rt" IN_LIST LLVM_ENABLE_PROJECTS)
     message(FATAL_ERROR "SCUDO cannot be included without adding compiler-rt to LLVM_ENABLE_PROJECTS")
   endif()
 endif()

libc/cmake/modules/LLVMLibCObjectRules.cmake

PROPERTIES
       "OBJECT_FILE" $<TARGET_OBJECTS:${fq_target_name}>
       # TODO: We don't need to list internal object files if the internal
       # target is a normal static library.
       "OBJECT_FILE_RAW" $<TARGET_OBJECTS:${internal_target_name}>
       "DEPS" "${fq_deps_list}"
   )
 
   if(LLVM_LIBC_ENABLE_LINTING)
+    if(NOT LLVM_LIBC_CLANG_TIDY)
+      message(FATAL_ERROR "Something is wrong!  LLVM_LIBC_ENABLE_LINTING is "
+              "ON but LLVM_LIBC_CLANG_TIDY is not set.")
+    endif()
 
     # We only want a second invocation of clang-tidy to run
     # restrict-system-libc-headers if the compiler-resource-dir was set in
     # order to prevent false-positives due to a mismatch between the host
     # compiler and the compiled clang-tidy.
     if(COMPILER_RESOURCE_DIR)
       # We run restrict-system-libc-headers with --system-headers to prevent
       # transitive inclusion through compler provided headers.
       set(restrict_system_headers_check_invocation
-        COMMAND $<TARGET_FILE:clang-tidy> --system-headers
+        COMMAND ${LLVM_LIBC_CLANG_TIDY} --system-headers
         --checks="-*,llvmlibc-restrict-system-libc-headers"
         # We explicitly set the resource dir here to match the
         # resource dir of the host compiler.
         "--extra-arg=-resource-dir=${COMPILER_RESOURCE_DIR}"
         --quiet
         -p ${PROJECT_BINARY_DIR}
         ${ADD_ENTRYPOINT_OBJ_SRCS}
       )
     else()
       set(restrict_system_headers_check_invocation
         COMMAND ${CMAKE_COMMAND} -E echo "Header file check skipped")
     endif()
 
     set(lint_timestamp "${CMAKE_CURRENT_BINARY_DIR}/.${target_name}.__lint_timestamp__")
     add_custom_command(
       OUTPUT ${lint_timestamp}
       # --quiet is used to surpress warning statistics from clang-tidy like:
       #     Suppressed X warnings (X in non-user code).
       # There seems to be a bug in clang-tidy where by even with --quiet some
       # messages from clang's own diagnostics engine leak through:
       #     X warnings generated.
       # Until this is fixed upstream, we use -fno-caret-diagnostics to surpress
       # these.
-      COMMAND $<TARGET_FILE:clang-tidy>
+      COMMAND ${LLVM_LIBC_CLANG_TIDY}
               "--extra-arg=-fno-caret-diagnostics" --quiet
               # Path to directory containing compile_commands.json
               -p ${PROJECT_BINARY_DIR}
               ${ADD_ENTRYPOINT_OBJ_SRCS}
       # See above: this might be a second invocation of clang-tidy depending on
       # the conditions above.
       ${restrict_system_headers_check_invocation}
       # We have two options for running commands, add_custom_command and
       # add_custom_target. We don't want to run the linter unless source files
       # have changed. add_custom_target explicitly runs everytime therefore we
       # use add_custom_command. This function requires an output file and since
       # linting doesn't produce a file, we create a dummy file using a
       # crossplatform touch.
       COMMAND "${CMAKE_COMMAND}" -E touch ${lint_timestamp}
       COMMENT "Linting... ${target_name}"
       DEPENDS clang-tidy ${internal_target_name} ${ADD_ENTRYPOINT_OBJ_SRCS}
       WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
     )
-
-    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__)
   endif()
 
 endfunction(add_entrypoint_object)
 
 set(ENTRYPOINT_EXT_TARGET_TYPE "ENTRYPOINT_EXT")
 
 # A rule for external entrypoint targets.
 # Usage:
 #     add_entrypoint_external(