Skip to content

Commit

Permalink
[libc++][CMake] Removes LIBCXX_ENABLE_CLANG_TIDY.
Browse files Browse the repository at this point in the history
The clang-tidy selection in CMake was refactored in
#81362. During review it was
suggested to remove this CMake option.
  • Loading branch information
mordante committed Apr 12, 2024
1 parent a11a432 commit 2474f6c
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 37 deletions.
5 changes: 0 additions & 5 deletions libcxx/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ option(LIBCXX_ENABLE_VENDOR_AVAILABILITY_ANNOTATIONS
to provide compile-time errors when using features unavailable on some version of
the shared library they shipped should turn this on and see `include/__availability`
for more details." OFF)
option(LIBCXX_ENABLE_CLANG_TIDY "Whether to compile and run clang-tidy checks" OFF)

if (CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
set(LIBCXX_DEFAULT_TEST_CONFIG "llvm-libc++-shared-gcc.cfg.in")
Expand Down Expand Up @@ -863,10 +862,6 @@ add_subdirectory(modules)

set(LIBCXX_TEST_DEPS "cxx_experimental")

if (LIBCXX_ENABLE_CLANG_TIDY)
list(APPEND LIBCXX_TEST_DEPS cxx-tidy)
endif()

list(APPEND LIBCXX_TEST_DEPS generate-cxx-modules)

if (LIBCXX_INCLUDE_BENCHMARKS)
Expand Down
3 changes: 3 additions & 0 deletions libcxx/docs/ReleaseNotes/19.rst
Original file line number Diff line number Diff line change
Expand Up @@ -132,3 +132,6 @@ Build System Changes

- The ``LIBCXX_EXECUTOR`` and ``LIBCXXABI_EXECUTOR`` CMake variables have been removed. Please
set ``LIBCXX_TEST_PARAMS`` to ``executor=<...>`` instead.

- The Cmake variable ``LIBCXX_ENABLE_CLANG_TIDY`` has been removed. The build system has been changed
to automatically detect the presence of ``clang-tidy`` and the required ``Clang`` libraries.
12 changes: 4 additions & 8 deletions libcxx/test/tools/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@

set(LIBCXX_TEST_TOOLS_PATH ${CMAKE_CURRENT_BINARY_DIR} PARENT_SCOPE)

# TODO: Remove LIBCXX_ENABLE_CLANG_TIDY
if(LIBCXX_ENABLE_CLANG_TIDY)
if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
message(STATUS "Clang-tidy can only be used when building libc++ with "
"a clang compiler.")
return()
endif()
add_subdirectory(clang_tidy_checks)
if(NOT CMAKE_CXX_COMPILER_ID STREQUAL "Clang")
message(STATUS "Clang-tidy tests are disabled due to non-clang based compiler.")
return()
endif()
add_subdirectory(clang_tidy_checks)
56 changes: 41 additions & 15 deletions libcxx/test/tools/clang_tidy_checks/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,14 @@ set(Clang_DIR_SAVE ${Clang_DIR})
# versions must match. Otherwise there likely will be ODR-violations. This had
# led to crashes and incorrect output of the clang-tidy based checks.
find_package(Clang ${CMAKE_CXX_COMPILER_VERSION})

set(SOURCES
abi_tag_on_virtual.cpp
header_exportable_declarations.cpp
hide_from_abi.cpp
proper_version_checks.cpp
qualify_declval.cpp
robust_against_adl.cpp
uglify_attributes.cpp

libcpp_module.cpp
)

if(NOT Clang_FOUND)
message(STATUS "Could not find a suitable version of the Clang development package;
custom libc++ clang-tidy checks will not be available.")
message(STATUS "Clang-tidy tests are disabled since the "
"Clang development package is unavailable.")
return()
endif()
if(NOT TARGET clangTidy)
message(STATUS "Clang-tidy tests are disabled since the "
"Clang development package has no clangTidy target.")
return()
endif()

Expand Down Expand Up @@ -54,6 +46,38 @@ if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
)
endif()

# In some cases even with the clangTidy target present the headers appear not to
# be on the system. Run a short test to see whether the header is present.
file(WRITE "${CMAKE_CURRENT_BINARY_DIR}/test.cpp" "
#if !__has_include(\"clang-tidy/ClangTidyCheck.h\")
# error No clang-tidy headers
#endif
int main(){}
")
try_compile(HAS_CLANG_TIDY_HEADERS
"${CMAKE_CURRENT_BINARY_DIR}"
"${CMAKE_CURRENT_BINARY_DIR}/test.cpp"
LINK_LIBRARIES clangTidy)

if(NOT HAS_CLANG_TIDY_HEADERS)
message(STATUS "Clang-tidy tests are disabled since the "
"clang-tidy headers are not present.")
return()
endif()
message(STATUS "Clang-tidy tests are enabled.")

set(SOURCES
abi_tag_on_virtual.cpp
header_exportable_declarations.cpp
hide_from_abi.cpp
proper_version_checks.cpp
qualify_declval.cpp
robust_against_adl.cpp
uglify_attributes.cpp

libcpp_module.cpp
)

add_library(cxx-tidy MODULE ${SOURCES})
target_link_libraries(cxx-tidy clangTidy)

Expand All @@ -64,3 +88,5 @@ set_target_properties(cxx-tidy PROPERTIES

set_target_properties(cxx-tidy PROPERTIES LIBRARY_OUTPUT_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR})
set(CMAKE_SHARED_MODULE_SUFFIX_CXX .plugin) # Use a portable suffix to simplify how we can find it from Lit

list(APPEND LIBCXX_TEST_DEPS cxx-tidy)
1 change: 0 additions & 1 deletion libcxx/utils/ci/buildkite-pipeline.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,6 @@ definitions:

environment_definitions:
_common_env: &common_env
ENABLE_CLANG_TIDY: "On"
LLVM_SYMBOLIZER_PATH: "/usr/bin/llvm-symbolizer-${LLVM_HEAD_VERSION}"
CLANG_CRASH_DIAGNOSTICS_DIR: "crash_diagnostics"
CC: clang-${LLVM_HEAD_VERSION}
Expand Down
8 changes: 0 additions & 8 deletions libcxx/utils/ci/run-buildbot
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,6 @@ CMAKE The CMake binary to use. This variable is optional.
CLANG_FORMAT The clang-format binary to use when generating the format
ignore list.
ENABLE_CLANG_TIDY Whether to compile and run clang-tidy checks. This variable
is optional.
EOF
}

Expand Down Expand Up @@ -111,10 +108,6 @@ function clean() {
rm -rf "${BUILD_DIR}"
}

if [ -z "${ENABLE_CLANG_TIDY}" ]; then
ENABLE_CLANG_TIDY=Off
fi

function generate-cmake-base() {
echo "--- Generating CMake"
${CMAKE} \
Expand All @@ -126,7 +119,6 @@ function generate-cmake-base() {
-DLIBCXX_ENABLE_WERROR=YES \
-DLIBCXXABI_ENABLE_WERROR=YES \
-DLIBUNWIND_ENABLE_WERROR=YES \
-DLIBCXX_ENABLE_CLANG_TIDY=${ENABLE_CLANG_TIDY} \
-DLLVM_LIT_ARGS="-sv --xunit-xml-output test-results.xml --timeout=1500 --time-tests" \
"${@}"
}
Expand Down

0 comments on commit 2474f6c

Please sign in to comment.